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

Fixed psl implementation. #39

Closed
wants to merge 3 commits into from
Closed

Conversation

smeinecke
Copy link

Public Suffix List is NOT puny encoded, so as noted in the "publicsuffix"
module:

Note that for internationalized domains the list at http://publicsuffix.org
uses decoded names, so it is up to the caller to decode any Punycode-encoded
names.

  • Add check if host is puny encoded and decode + reencode result of psl.

Public Suffix List is NOT puny encoded, so as noted in the "publicsuffix"
module:
"""
Note that for internationalized domains the list at http://publicsuffix.org
uses decoded names, so it is up to the caller to decode any Punycode-encoded
names.
"""

- Add check if host is puny encoded and decode + reencode result of psl.
@neilmb
Copy link

neilmb commented Apr 20, 2016

Could you add a test case to test.py that exercises this code path? In the test_pld method, you could add an example with a punycoded domain and the resulting PLD.

- add 4 additional idn tld test urls in test.py
- add missing unicode prefix in test
- fix missing decode of hostname in pld
@smeinecke
Copy link
Author

There seems to be another big problem as the TLD 'xn--e1a4c' is not recognized by the publicsuffix module. It seems as the default shipped public suffix list is very out of date which is a big problem as nearly every month new TLDs get delegated.

@dlecocq
Copy link

dlecocq commented Aug 26, 2016

I think this is obviated by #45 -- @smeinecke - could you confirm it meets your needs? (I specifically grabbed your test cases with that hope)

@smeinecke
Copy link
Author

@dlecocq yes, that's great, thanks 👍

@smeinecke smeinecke closed this Aug 26, 2016
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

Successfully merging this pull request may close these issues.

3 participants