Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Some well formed base domains fail to check #91

Closed
przmv opened this issue Dec 24, 2015 · 6 comments
Closed

Some well formed base domains fail to check #91

przmv opened this issue Dec 24, 2015 · 6 comments

Comments

@przmv
Copy link

przmv commented Dec 24, 2015

I'm running the following test case with libpsl:

checkPublicSuffix('amber.museum', 'amber.museum');
checkPublicSuffix('aip.ee', 'aip.ee');
checkPublicSuffix('africa.com', 'africa.com');
checkPublicSuffix('amursk.ru', 'amursk.ru');
checkPublicSuffix('appspot.com', 'appspot.com');
checkPublicSuffix('ar.com', 'ar.com');
checkPublicSuffix('eu.org', 'eu.org');
checkPublicSuffix('nsk.ru', 'nsk.ru');

and it fails:

psl_registrable_domain(amber.museum)=NULL (expected amber.museum)
psl_registrable_domain(aip.ee)=NULL (expected aip.ee)
psl_registrable_domain(africa.com)=NULL (expected africa.com)
psl_registrable_domain(amursk.ru)=NULL (expected amursk.ru)
psl_registrable_domain(appspot.com)=NULL (expected appspot.com)
psl_registrable_domain(ar.com)=NULL (expected ar.com)
psl_registrable_domain(eu.org)=NULL (expected eu.org)
psl_registrable_domain(nsk.ru)=NULL (expected nsk.ru)

(full test log)

@sleevi
Copy link
Contributor

sleevi commented Dec 24, 2015

That sounds like a bug with libpsl, not with the list; it's unclear what you see the issue as.

@przmv
Copy link
Author

przmv commented Dec 24, 2015

I got the same results with tldjs: https://jsfiddle.net/d0xe4ybk/1/ and also with x/net/publicsuffix Golang package.

@sleevi
Copy link
Contributor

sleevi commented Dec 24, 2015

That doesn't really address my point; what you just said sounds like bugs in their implementation.

The test cases provided are consistent with the algorithm definition; if you're getting bad results, it's worth diving into the implementations and finding out why. For example, has a change in the format of the list produced invalid results?

Note that the test is not meant to be equivalent of psl_registerable_domain; that is, the function checkPublicSuffix is not equivalent to returning the registerable portion of the domain, but the public suffix.

The distinction is perhaps more obvious when you consider foo.amber.museum; the registerable portion of that domain is foo.amber.museum, the public suffix is amber.museum. This is similar to how .com is handled - example.com is the registerable domain, while .com is the public suffix. The function invoked by the tests is akin to checkPublicSuffix('com', 'com')

@sleevi
Copy link
Contributor

sleevi commented Dec 24, 2015

er, sorry on the remark on checkPublicSuffix('com', 'com') - we actually have a test case for that in https://raw.githubusercontent.com/publicsuffix/list/master/tests/test_psl.txt

Since appspot.com (and all the other domains) are on the list, the results would be the same as checkPublicSuffix('com' - which is to say, all should return null.

The broader question/issue seems to be whether/how gaps are handled; that's primarily a question of spec and implementation ambiguity for which @gerv @weppos and others have been unable to find a clear and complete solution. I would point you to the older Bugzilla bug - https://bugzilla.mozilla.org/show_bug.cgi?id=1139842 and https://bugzilla.mozilla.org/show_bug.cgi?id=1001410 - which tracked a lot of this discussion about how gaps are handled and what the expected results are.

@rockdaboot
Copy link
Contributor

As @sleevi says, the test cases should be like

checkPublicSuffix('amber.museum', NULL);
...

since 'amber.museum' and your other examples are public suffices.

@przmv
Copy link
Author

przmv commented Dec 24, 2015

Thanks, got it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants