Closed Bug 1101625 Opened 5 years ago Closed 5 years ago

make UnEscapeURIForUI escape blacklisted chars only

Categories

(Core :: Internationalization, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla37

People

(Reporter: mats, Assigned: mats)

Details

Attachments

(2 files)

Currently we (re-)escape the whole string when it contains a forbidden char:
http://mxr.mozilla.org/mozilla-central/source/intl/uconv/nsTextToSubURI.cpp#266
It would be better if we only (re-)escape the forbidden chars and
leave the rest alone.  Also, FindCharInSet(unsafeChars) is rather slow but
since we know 'unsafeChars' is very likely sorted we can optimize this
by doing a binary search.
Comment on attachment 8527771 [details] [diff] [review]
part 1 - Don't escape all non-ASCII characters when unsafe characters are found, just escape the unsafe characters.

Review of attachment 8527771 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM

::: xpcom/io/nsEscape.h
@@ +170,5 @@
> + * Percent-escapes all characters in aStr that occurs in aForbidden.
> + * @param aStr the input URL string
> + * @param aForbidden the characters that should be escaped if found in aStr
> + * @note that aForbidden MUST be sorted (low to high)
> + * @param aResult the result if some some characters were escaped

Nit: remove the extra "some"
Attachment #8527771 - Flags: review?(smontagu) → review+
Attachment #8527772 - Flags: review?(smontagu) → review+
(In reply to Simon Montagu :smontagu from comment #3)
> Nit: remove the extra "some"

Fixed.

https://hg.mozilla.org/integration/mozilla-inbound/rev/be23ef5730dc
https://hg.mozilla.org/integration/mozilla-inbound/rev/c2301fd8921b
Flags: in-testsuite?
https://hg.mozilla.org/mozilla-central/rev/be23ef5730dc
https://hg.mozilla.org/mozilla-central/rev/c2301fd8921b
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
I landed a trivial fixup for mingw (nsString using char16ptr_t causing ambiguous call):

https://hg.mozilla.org/integration/mozilla-inbound/rev/c4a4eceb85f2
You need to log in before you can comment on or make changes to this bug.