1538974 - The `urlclassifier.trackingAnnotationSkipURLs` pref do not accept some rules with uppercase characters

Status: RESOLVED FIXED

(Core :: Privacy: Anti-Tracking, defect)

Description

[Steven Englehardt [:englehardt]]

In Bug 1536898 we tested a temporary fix for the breakage described in Bug 1531818.

We tried using accounts.google.com/ServiceLogin as a skip url, given that the current recaptcha skip url includes a path. The breakage was due to the lack of cookies on the url: https://accounts.google.com/ServiceLogin?passive=1209600&osid=1&continue=https://smartlock.google.com/client?callback%3DonGoogleYoloLoad&followup=https://smartlock.google.com/client?callback%3DonGoogleYoloLoad&authuser=0.

Expected result: Breakage described in Bug 1531818 was fixed.

Actual result: The breakage was not fixed, and we instead needed to use accounts.google.com as the skip url.

Although we didn't test it, we suspect this will also apply to the url-classifier-skip-urls remote settings approach to adding skip urls.

Comment 1

[(no longer active)]

This regression is caused by bug 1511436 part 3. That patch removed a call to ToLowerCase() here: https://hg.mozilla.org/mozilla-central/rev/0cc5e8d1a09d#l19.195 and didn't replace it with anything. The problem mentioned in comment 0 is caused because of the upper case characters in accounts.google.com/ServiceLogin.

Comment 2

[(no longer active)]

Attachment: Bug 1538974 - Ensure that we accept uppercase characters for url-classifier annotation skip URLs;

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6bd4f2846097
Ensure that we accept uppercase characters for url-classifier annotation skip URLs; r=johannh

Comment 4

[(no longer active)]

Comment on attachment 9055636 [details]
Bug 1538974 - Ensure that we accept uppercase characters for url-classifier annotation skip URLs;

Beta/Release Uplift Approval Request

• Feature/Bug causing the regression: Bug 1511436
• User impact if declined: We may not be able to push site-specific workarounds for Enhanced Tracking Protection to users successfully in order to address site breakage when we discover such problems. We can of course be careful to push such settings with all lower-case strings but it's easy to make human errors there and there won't be anything to stop us.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): A one liner change to a JS file.
• String changes made/needed: None

Comment 5

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/6bd4f2846097

Comment 6

[Pascal Chevrel:pascalc (PTO until April 26)]

Comment on attachment 9055636 [details]
Bug 1538974 - Ensure that we accept uppercase characters for url-classifier annotation skip URLs;

Tracked 67 regression, well understood and low-risk patch with tests, uplift approved for 67 beta 9, thanks!

Comment 7

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/releases/mozilla-beta/rev/1e5ffce08e37