Closed Bug 880625 Opened 7 years ago Closed 4 years ago

Updates to the PSL

Categories

(Core :: Networking: Domain Lists, defect)

x86
All
defect
Not set

Tracking

()

RESOLVED INCOMPLETE

People

(Reporter: weppos, Assigned: weppos)

Details

Attachments

(1 file, 3 obsolete files)

We received several amendments from Tobias Sattler and Aaron Gable.
They need to be checked and merged.
Attached patch Patch from Tobias (obsolete) — Splinter Review
Attached patch Patch from Aaron (obsolete) — Splinter Review
weppos: do you need to do more here? If not, please set a review flag for me.

Gerv
Yes, I actually still need to validate these changes. I posted the patches here as a reminder.
Attached patch 880625.patch (obsolete) — Splinter Review
First batch, TLDs from A-C.

Whenever possible, I validated the list using the registry information. Otherwise, Wikipedia.
Attachment #8336226 - Flags: review?(gerv)
Did Tobias and Aaron not specify their sources? Are they just two members of the public, or do they have special knowledge in some way? I notice in some cases they suggest different changes for the same TLD, so it would be useful to understand where they got the info from. One may have a better source than the other.

Gerv
No, they didn't specify the sources. You may be able to find their email and domains in your email archive. Aaron is from Google, Tobias from United Domains.

I'm not sure why they ended up with such differences. So far, their proposal were quite similar.

I'm going to each TLD one by one, that's why it's taking time.
Attachment #8336226 - Flags: review?(gerv) → review+
See bug 943800 for my plan to prevent this happening in future.

In the mean time, weppos: can you also produce an update to the PSL test suite file?

Gerv
Attached patch 880625-AC.patchSplinter Review
Here's the new patch that includes the changes to the PSL and test suite.
Attachment #8336226 - Attachment is obsolete: true
Attachment #8339281 - Flags: review?(gerv)
Comment on attachment 8339281 [details] [diff] [review]
880625-AC.patch

r=gerv.

Gerv
Attachment #8339281 - Flags: review?(gerv) → review+
Attachment #759668 - Attachment is obsolete: true
Attachment #759669 - Attachment is obsolete: true
@Ryan

The patch I provided was the first step (TLDs from A to C). The attachment you flagged as obsolete are still under review.
https://hg.mozilla.org/integration/mozilla-inbound/rev/13c1b3a2d135

Not sure if this is supposed to be [leave open] or not. Please add it to the whiteboard if there's more work to be done here.
Keywords: checkin-needed
Backed out for xpcshell failures (again). Please run this through Try before requesting checkin again.
https://hg.mozilla.org/integration/mozilla-inbound/rev/e5ca4a98aa68

https://tbpl.mozilla.org/php/getParsedLog.php?id=31324689&tree=Mozilla-Inbound
I didn't notice .ck was also in use in the test. The previous time it failed before and never reported it.

@Ryan, Gerv
Is there any way for me to run that single spec suite locally on my machine without running all specs?
> I didn't notice .ck was also in use in the test.

This is why it would be useful to have a big comment before the .ck block which said "This is used in tests" :-)

You can run a single test. But you need to build your own copy of Firefox first.

Then:
./mach xpcshell-test netwerk/test/unit/test_psl.js

Gerv
@gerv

I propose to close this ticket, as we made several changes in the PSL since then and the list now is at https://github.com/publicsuffix/list

I'm quite sure that if any of the proposed changes are still valid in the new PSL, we'll discover them soon (given also the recent adoption by LE).

Do you agree?
OK.

Gerv
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → INCOMPLETE
You need to log in before you can comment on or make changes to this bug.