Closed Bug 368702 Opened 14 years ago Closed 14 years ago

Effective TLD Service should treat trailing dot properly


(Core :: Networking, defect)

Not set





(Reporter: ecfbugzilla, Assigned: ecfbugzilla)




(1 file, 3 obsolete files)


var tld 
    = Components.classes[";1"]


Currently this shows 0 though 4 is the correct value. I should have a patch soon.
Assignee: nobody → trev
Attachment #253346 - Flags: review?(darin.moz)
Attached file mochitest compatible testcase (obsolete) —
Attachment #253348 - Flags: review?(darin.moz)
Should we really count the trailing dot? There's already some confusion about what the length means because an IDN domain would be converted to punycode and the length returned based on the new length. This only affects effectiveTLD's that are not pure ascii, but we do in fact have a couple of those already in our domain list.

Then again if Necko isn't stripping the trailing dot then maybe it does count. I'd like this made explicit in comments in the interface file either way.
If we get a domain with a trailing dot passed we certainly need to count this dot in our result, otherwise you won't be able to take a substring of given length at the end of the domain name and say that it is the effective TLD. And yes, Necko doesn't strip the trailing dot. It actually treats "" and "" as different domains (which makes sense since some servers do in fact treat them as different).
Comment on attachment 253348 [details]
mochitest compatible testcase

why not make this an xpcshell testcase, like the stuff in netwerk/test/unit?
Attached patch Unit test (obsolete) — Splinter Review
No good reason not to make an xpcshell testcase but I don't know this test harness and it doesn't seem to be documented. The script to run the tests doesn't work on Windows so I had to do it manually but this seems to work.
Attachment #253348 - Attachment is obsolete: true
Attachment #253990 - Flags: review?(darin.moz)
Attachment #253348 - Flags: review?(darin.moz)
docs are at

make check is supposed to work on windows, I hope it still does...
Thanks for pointing me to the docs and thanks for telling about "make check" - yes, it works and even makes things quite a lot easier.
Attachment #253346 - Flags: review?(darin.moz) → review?(cbiesinger)
Attachment #253990 - Flags: review?(darin.moz) → review?(cbiesinger)
Flags: wanted1.8.1.x+
Attachment #253346 - Flags: review?(cbiesinger) → review+
Comment on attachment 253990 [details] [diff] [review]
Unit test

I'm somewhat surprised that for a host w/o TLD the length is the string length instead of 0, but ok - this was clearly the case before this patch too.

I'd make Cc and Ci constants
Attachment #253990 - Flags: review?(cbiesinger) → review+
Comment on attachment 253346 [details] [diff] [review]
Make sure to skip trailing dot for TLD checking

The comment will need to be fixed before checking in. "ignore the trailing if one is present" should be "ignore the trailing *dot* if one is present" of course.
Attachment #253346 - Flags: superreview?(dveditz)
Attachment #253990 - Flags: superreview?(dveditz)
Comment on attachment 253346 [details] [diff] [review]
Make sure to skip trailing dot for TLD checking

Attachment #253346 - Flags: superreview?(dveditz) → superreview+
Attachment #253990 - Flags: superreview?(dveditz) → superreview+
This is both the fix and the unit test in one patch. The comment is fixed, also Cc and Ci in the unit test are made constants.
Whiteboard: [checkin needed]
mozilla/netwerk/dns/src/nsEffectiveTLDService.cpp  1.2
mozilla/netwerk/test/unit/test_bug368702.js        1.1
Closed: 14 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [checkin needed]
Target Milestone: --- → mozilla1.9alpha5
Attachment #253346 - Attachment is obsolete: true
Attachment #253990 - Attachment is obsolete: true
Blocks: abp
You need to log in before you can comment on or make changes to this bug.