Closed
Bug 402013
Opened 17 years ago
Closed 17 years ago
improve normalization behavior of nsIEffectiveTLDService
Categories
(Core :: Networking, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: dwitte, Assigned: dwitte)
References
()
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
Biesinger
:
review+
Biesinger
:
superreview+
mconnor
:
approval1.9+
|
Details | Diff | Splinter Review |
followup from https://bugzilla.mozilla.org/show_bug.cgi?id=368989#c47. nsIEffectiveTLDService normalizes hosts to UTF8, in order to perform comparisons with the eTLD host file. this means that what it returns isn't always compatible with nsIURI::GetHost(), or GetAsciiHost() either, without conversion. this places somewhat of a burden on consumers. it also means eTLD has to re-normalize URI's to get them into UTF8, which is a little ugly. given that the normalization of the host and the eTLD host file must match, there are three possible solutions:
1) make eTLD normalize the host file internally to "display IDN", i.e. compatible with nsIURI::GetHost(). this could be done by exposing the normalization method on a public interface (bug 402008). however, given that this normalization can change depending on a pref (i.e. at runtime!) and the IDN whitelist, this seems like a bad idea.
2) make eTLD use nsIURI::GetAsciiHost(), normalize the host file internally to ASCII/ACE, and state as such in the interface documentation. if consumers want to compare the string they get back from eTLD with their URI, they can then use GetAsciiHost() and things should work. (incidentally, this would make nsIEffectiveTLDService::GetPublicSuffic(nsIURI*) the "fast path", and GetPublicSuffixFromHost(nsACString&) do the extra host normalization.)
3) WONTFIX this bug and stick with UTF8, letting the consumer convert if they need to.
option 2) makes the most sense to me. if a consumer wants to convert the resulting host back to display IDN, (maybe for... display!), then fixing bug 402008 will take care of that.
biesi, dveditz, what do you think?
Assignee | ||
Comment 1•17 years ago
|
||
here's a patch that implements the second option, just to get things rolling.
Attachment #287058 -
Flags: superreview?(dveditz)
Attachment #287058 -
Flags: review?(dveditz)
Assignee | ||
Updated•17 years ago
|
Attachment #287058 -
Flags: superreview?(dveditz)
Attachment #287058 -
Flags: review?(dveditz)
Assignee | ||
Comment 2•17 years ago
|
||
updated patch, include rv check of GetAsciiHost() since this can fail under ordinary circumstances for certain schemes (thanks Mardak!).
dveditz, this is blocking the etld-in-cookies bug from landing, any chance you can get to this soon? (thanks!)
Attachment #287058 -
Attachment is obsolete: true
Attachment #290945 -
Flags: superreview?(dveditz)
Attachment #290945 -
Flags: review?(dveditz)
Assignee | ||
Comment 3•17 years ago
|
||
Comment on attachment 290945 [details] [diff] [review]
patch v1.1
switching request to biesi, in the hopes of landing this + the cookie etld patch for b2.
Attachment #290945 -
Flags: superreview?(dveditz)
Attachment #290945 -
Flags: superreview?(cbiesinger)
Attachment #290945 -
Flags: review?(dveditz)
Attachment #290945 -
Flags: review?(cbiesinger)
Updated•17 years ago
|
Attachment #290945 -
Flags: superreview?(cbiesinger)
Attachment #290945 -
Flags: superreview+
Attachment #290945 -
Flags: review?(cbiesinger)
Attachment #290945 -
Flags: review+
Assignee | ||
Comment 4•17 years ago
|
||
Comment on attachment 290945 [details] [diff] [review]
patch v1.1
requesting approval1.9 - need this patch to land blocker bug 385299.
Attachment #290945 -
Flags: approval1.9?
Comment 5•17 years ago
|
||
Comment on attachment 290945 [details] [diff] [review]
patch v1.1
a=mconnor on behalf of drivers for checkin _today_
Attachment #290945 -
Flags: approval1.9? → approval1.9+
Assignee | ||
Comment 6•17 years ago
|
||
landed!
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 7•17 years ago
|
||
added unit test to cover new behavior.
Checking in test_bug368702.js;
/cvsroot/mozilla/netwerk/test/unit/test_bug368702.js,v <-- test_bug368702.js
new revision: 1.4; previous revision: 1.3
done
Flags: in-testsuite+
You need to log in
before you can comment on or make changes to this bug.
Description
•