986241 - Wrong PSL entries: domains with spaces inside

Status: RESOLVED FIXED

(Core Graveyard :: Networking: Domain Lists, defect)

Description

[Ángel]

The current Public Suffix list at http://publicsuffix.org/list/effective_tld_names.dat (sha1 00401fac245e7da20280e9bef889fd9a650b764a) contain two domains with spaces:
bergamo .it
potenza .it

which means top-level domains "bergamo" and "potenza", with a comment of ".it".

Expected:
bergamo.it
potenza.it


Additionally, there are other 361 domains which have a trailing space (all of them .it ones), which are harmless but probably unintended.

Comment 1

[Ángel]

Attachment: bug986241.patch

The wrong behavior can be easily confirmed by looking at prepare_tlds output:
$ python2 ./prepare_tlds.py effective_tld_names.dat | egrep "bergamo|potenza"
ETLD_ENTRY("bergamo", false, false)
ETLD_ENTRY("potenza", false, false)

I am attaching a patch covering the two issues -wrong entries and trailing whitespace- (as two commits).


After applying the generated file, as expected, only differ in that the wrong domain entries are now correct:
--- a	2014-03-21 18:10:59.059821079 +0100
+++ b	2014-03-21 18:37:14.829816330 +0100
@@ -886,3 +886,3 @@
 ETLD_ENTRY("benevento.it", false, false)
-ETLD_ENTRY("bergamo", false, false)
+ETLD_ENTRY("bergamo.it", false, false)
 ETLD_ENTRY("bg.it", false, false)
@@ -1041,3 +1041,3 @@
 ETLD_ENTRY("pordenone.it", false, false)
-ETLD_ENTRY("potenza", false, false)
+ETLD_ENTRY("potenza.it", false, false)
 ETLD_ENTRY("pr.it", false, false)

Comment 2

[Gervase Markham [:gerv]]

Comment on attachment 8394928 [details] [diff] [review]
bug986241.patch

Review of attachment 8394928 [details] [diff] [review]:
-----------------------------------------------------------------

r=gerv. Although part of me thinks we should keep some trailing whitespace, just to make sure people's parsers can handle it.

Gerv

Comment 3

[Ángel]

I think it's more important to have a comment separated from the domain by whitespace. If they get it right, trailing whitespace should be no problem.
Maybe add some of this cases in a test TLD, like suggested in bug 943800

Comment 4

[Ángel]

Attachment: bug986241 r=gerv.patch

Rebased patch with the appropiate r=gerv in the commit msg.

Comment 5

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/212730695575

Comment 7

[Ángel]

Thanks Wes. Is it the interface or did it create a single commit instead of two?

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/212730695575