Closed Bug 263462 Opened 20 years ago Closed 18 years ago

NS_UnescapeURL doesn't honor esc_OnlyNonASCII

Categories

(Core :: XPCOM, defect)

1.7 Branch
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: jshin1987, Assigned: jshin1987)

References

Details

(Keywords: fixed-aviary1.0, intl)

Attachments

(1 file)

NS_UnescapeURL doesn't check 'esc-OnlyNonASCII' flag. When unescaping, setting it on should get ASCII characters to be skipped according to the 'documentation' in nsEscape.h: esc_OnlyNonASCII = PR_BIT(12), /* causes _graphic_ ascii octets (0x20-0x7E) * to be skipped when escaping. causes all * ascii octets to be skipped when unescaping * I wrote that comment, but apparently I didn't implement it in nsEscape.cpp leading to the discrepancy and bug 243504 (I specified 'esc_OnlyNonASCII' in nsUTF8ConverterService.cpp expecting ASCII characters to be kept 'url-escaped' when calling NS_UnescapeURL, but the flag has no effect and ASCII characters are escaped). I'm gonna upload a patch in a moment.
Attached patch patch (deleted) — Splinter Review
implement esc_OnlyNonASCII
Comment on attachment 161462 [details] [diff] [review] patch asking for r/sr
Attachment #161462 - Flags: superreview?(jst)
Attachment #161462 - Flags: review?(darin)
plussing for the aviary branch since this fix is required to fix 243504 which is aviary1.0+
Flags: blocking-aviary1.0+
Comment on attachment 161462 [details] [diff] [review] patch sr=jst
Attachment #161462 - Flags: superreview?(jst) → superreview+
Comment on attachment 161462 [details] [diff] [review] patch looks good, r=darin
Attachment #161462 - Flags: review?(darin) → review+
I checked this into the aviary 1.0 branch
Keywords: fixed-aviary1.0
Comment on attachment 161462 [details] [diff] [review] patch asking for a1.7 it's already gotten into aviary 1.0 branch
Attachment #161462 - Flags: approval1.7.x?
Comment on attachment 161462 [details] [diff] [review] patch a=mkaply for 1.7
Attachment #161462 - Flags: approval1.7.x? → approval1.7.x+
checked into 1.7 and trunk. thanks all.
Status: NEW → RESOLVED
Closed: 20 years ago
Keywords: fixed1.7.x
Resolution: --- → FIXED
I think this change to our escaping code caused: Bug #273109
Re-open because patch for this bug is backed out on Thunderbird 1.0 release build(temporary?) to resolve more serious regression, Bug 273109. See Bug 273109 Comment #17. This probably revived problem of Bug 243504(Bug 239192 also). See Bug 243504 Comment #31.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Keywords: fixed-aviary1.0
clearing fixed 1.7.5 flag as the patch has been backed out of there as well
Keywords: fixed1.7.5
Flags: blocking1.8a6?
Doesn't look like this is going to make 1.8a6
Flags: blocking1.8b?
Flags: blocking1.8a6?
Flags: blocking1.8a6-
This was kept on trunk. This bug is only for 1.7/1.0 branches.
Version: Trunk → 1.7 Branch
Depends on: 274264
I put this back on the thunderbird 1.0 branch. re-closing the bug.
Status: REOPENED → RESOLVED
Closed: 20 years ago20 years ago
Keywords: fixed-aviary1.0
Resolution: --- → FIXED
Reopening to comfirm status on 1.7 branch. Mscott, pacth looks to be still backed-out on 1.7 branch. Is it intentional? (Aviary) > http://lxr.mozilla.org/aviarybranch/source/xpcom/io/nsEscape.cpp#437 > 437 PRBool ignoreNonAscii = (flags & esc_OnlyASCII); > 438 PRBool ignoreAscii = (flags & esc_OnlyNonASCII); > 439 PRBool writing = (flags & esc_AlwaysCopy); (Mozilla 1.7 branch) > http://lxr.mozilla.org/mozilla1.7/source/xpcom/io/nsEscape.cpp#437 > 437 PRBool ignoreNonAscii = (flags & esc_OnlyASCII); > 438 PRBool writing = (flags & esc_AlwaysCopy);
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(In reply to comment #16) > Reopening to comfirm status on 1.7 branch. > > Mscott, pacth looks to be still backed-out on 1.7 branch. > Is it intentional? It cannot be relaned until I get approval for landing the patch for bug 274264 and land it in 1.7 branch.
Oh, delay by waiting for approval problem again... If so, I think setting "blocking 1.7.6" flag is better. Jshin, what do you think?
Flags: blocking1.7.6?
Not a blocker. blocking1.7.6-
Flags: blocking1.7.6? → blocking1.7.6-
Is this fixed on the trunk? If so, should it be marked as such?
Flags: blocking1.8b? → blocking1.8b-
I made some updates to bug 243504 that I think contains some additional information regarding the issue. Apparently it has been solved in the user interface but not from the command line.
QA Contact: xpcom
The patch is apparently checked in a long time ago. http://lxr.mozilla.org/mozilla/source/xpcom/io/nsEscape.cpp#384 Marking FIXED.
Status: REOPENED → RESOLVED
Closed: 20 years ago18 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: