python-publicsuffix2 icon indicating copy to clipboard operation
python-publicsuffix2 copied to clipboard

make get_tld return only eTLDs in the publicsuffix list

Open hiratara opened this issue 5 years ago • 4 comments

See: #18

To solve the problem, I initialize the trie with -1. -1 means that there are no definitions in the publicsuffix list.

This is WIP PR because I don't really understand the exact semantics of wildcard=False .

hiratara avatar Jul 19 '20 12:07 hiratara

@KnitCode I fix the behavior of wildcard=False by c6575d52c0d48ba7f981f2c015f31c4c90c05a73 . What do you think?

hiratara avatar Jul 24 '20 00:07 hiratara

@hiratara I'm sorry somehow I lost track of this -- and a pandemic. I'm really sorry. Happy to check again. I checked in today because I realized that the code is not handling the private suffixes as expected... and we have tests for them so I'm super confused. Do you want to revisit and merge this PR? there is another one on Nov 3rd from someone else.

The bug i see is that we should have

get_sld('this.that.internal') = 'that.internal'

as the documentation and tests show, but instead we have

get_sld('this.that.internal') = 'internal'

KnitCode avatar Dec 03 '21 01:12 KnitCode

Hello @KnitCode,

as the documentation and tests show, but instead we have get_sld('this.that.internal') = 'internal'

Unfortunately, it has always behaved that way. I tried to check out https://github.com/nexB/python-publicsuffix2/releases/tag/release-2.2019-08-12 (before I contributed this repository) and found that get_sld('this.that.internal') was already returning 'internal'. The document was also the same as now.

https://github.com/nexB/python-publicsuffix2/blob/b7fae069079a79cdb9ad7525c1fb0005f131faf4/src/publicsuffix2/init.py#L244-L246

I also tried 9cd5417ebb3a47f2c771870da8d909fb3d5aef45, the commit on which the document was added, but I got the same result. I think the documentation is wrong.

I understand it is a bug, but it is not related this PR. We should open a new issue.

hiratara avatar Jan 08 '22 04:01 hiratara

@hiratara oh interesting. I checked my old old versions as well. I think i wrote that part of the documentation, so it must have been a mistake in the documentation. I would like to see it return this.local but in any case... where are we at on this PR? there is another one waiting behind it.

I think we need @pombredanne to merge anything.

KnitCode avatar Jan 21 '22 23:01 KnitCode