Closed Bug 779845 Opened 7 years ago Closed 7 years ago

Inconsistencies in the PSL test data and Example

Categories

(Core :: Networking: Domain Lists, defect)

defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: rob, Assigned: gerv)

Details

Attachments

(3 files, 3 obsolete files)

The changes to the .jp PSL rules in Bug 763208 invalidated some of the test data [1] and parts of the Example [2].

1. I suggest (as mentioned in Bug 763208 comment 9) replacing the .jp section of the test data as follows:
# More complex TLD.
checkPublicSuffix('jp', NULL);
checkPublicSuffix('test.jp', 'test.jp');
checkPublicSuffix('www.test.jp', 'test.jp');
checkPublicSuffix('ac.jp', NULL);
checkPublicSuffix('test.ac.jp', 'test.ac.jp');
checkPublicSuffix('www.test.ac.jp', 'test.ac.jp');
checkPublicSuffix('kyoto.jp', NULL);
checkPublicSuffix('test.kyoto.jp', 'test.kyoto.jp');
checkPublicSuffix('ide.kyoto.jp', NULL);
checkPublicSuffix('b.ide.kyoto.jp', 'b.ide.kyoto.jp');
checkPublicSuffix('a.b.ide.kyoto.jp', 'b.ide.kyoto.jp');
checkPublicSuffix('c.kobe.jp', NULL);
checkPublicSuffix('b.c.kobe.jp', 'b.c.kobe.jp');
checkPublicSuffix('a.b.c.kobe.jp', 'b.c.kobe.jp');
checkPublicSuffix('city.kobe.jp', 'city.kobe.jp');
checkPublicSuffix('www.city.kobe.jp', 'city.kobe.jp');

2. Petar Bogdanovic noticed (in Bug 763208 comment 10) that the "Unlisted TLD" test data is actually invalid, because 'If no rules match, the prevailing rule is "*" '.  So the following test data should be either removed or fixed:
# Unlisted TLD.
checkPublicSuffix('example', NULL);
checkPublicSuffix('example.example', NULL);
checkPublicSuffix('b.example.example', NULL);
checkPublicSuffix('a.b.example.example', NULL);

3. Also, I think the following test data could also be removed:
# Listed, but non-Internet, TLD.
#checkPublicSuffix('local', NULL);
#checkPublicSuffix('example.local', NULL);
#checkPublicSuffix('b.example.local', NULL);
#checkPublicSuffix('a.b.example.local', NULL);

4. I'll defer to Gerv to come up with a fix for the .jp parts of the Example.


[1] http://publicsuffix.org/list/test.txt
[2] http://publicsuffix.org/list/
Assignee: nobody → gerv
I'll incorporate the test into our tree to ensure the test is up-to-date.
Attachment #651320 - Flags: review?(gerv)
Attachment #651320 - Flags: feedback?(rob)
I set the user name of these two patches to Rob Stradling because they do not contain my creative part at all. Rob, please verify the test is imported correctly and your name and mail address is correct. They will be displayed on the commit log.
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml
Attachment #651321 - Flags: review?(gerv)
Attachment #651321 - Flags: feedback?(rob)
Gerv, please forward to a appropriate reviewer if you are not.
Attachment #651322 - Flags: review?(gerv)
Comment on attachment 651320 [details] [diff] [review]
Part 1: Import PSL test from http://publicsuffix.org/list/test.txt

r=gerv on the idea of having these tests as part of our build tree.

Gerv
Attachment #651320 - Flags: review?(gerv) → review+
Comment on attachment 651321 [details] [diff] [review]
Part 2: Update the PSL test to catch up with the .jp PSL update and fix test's bug about unlisted TLD

r=gerv on fixes.

Gerv
Attachment #651321 - Flags: review?(gerv) → review+
Comment on attachment 651322 [details] [diff] [review]
Part 3: Incorporate the PSL test into our test system

Brian: as a member of the networking team, can you rubber-stamp this?

Thanks :-)

Gerv
Attachment #651322 - Flags: review?(gerv) → review?(bsmith)
Attachment #651320 - Flags: feedback?(rob) → feedback+
Attachment #651321 - Flags: feedback?(rob) → feedback+
(In reply to Gervase Markham [:gerv] from comment #4)
> Comment on attachment 651320 [details] [diff] [review]
> Part 1: Import PSL test from http://publicsuffix.org/list/test.txt
> 
> r=gerv on the idea of having these tests as part of our build tree.

+1, although I suggest renaming test.txt as part of the import.  How about netwerk/test/unit/data/test_psl.txt ?
(In reply to Rob Stradling from comment #7)
> +1, although I suggest renaming test.txt as part of the import.  How about
> netwerk/test/unit/data/test_psl.txt ?
Fair enough. I'll attach updated patches.
Attachment #651320 - Attachment is obsolete: true
Attachment #651430 - Flags: review+
Attachment #651322 - Attachment is obsolete: true
Attachment #651322 - Flags: review?(bsmith)
ping?
Comment on attachment 651433 [details] [diff] [review]
Part 3: Incorporate the PSL test into our test system

LGTM.
Attachment #651433 - Flags: review?(bsmith) → review+
Looks like this is my screw-up; I forgot to hg add a file. :-|

Gerv
Shouldn't be closed until http://publicsuffix.org/list/ is updated so that it points the test data on our tree.
Whiteboard: [leave open]
emk: Done.

Gerv
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Whiteboard: [leave open]
You need to log in before you can comment on or make changes to this bug.