Closed Bug 1538974 Opened 1 year ago Closed 1 year ago

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

Categories

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

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla68
Tracking Status
firefox-esr60 --- unaffected
firefox66 --- wontfix
firefox67 + fixed
firefox68 + fixed

People

(Reporter: englehardt, Assigned: ehsan)

References

(Regression)

Details

(Keywords: regression)

Attachments

(1 file)

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.

Flags: needinfo?(ehsan)

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.

Assignee: nobody → ehsan
Flags: needinfo?(ehsan)
Keywords: regression
Regressed by: 1511436
Summary: The `urlclassifier.trackingAnnotationSkipURLs` pref do not accept some rules with paths → The `urlclassifier.trackingAnnotationSkipURLs` pref do not accept some rules with uppercase characters
Type: enhancement → defect
Pushed by eakhgari@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6bd4f2846097
Ensure that we accept uppercase characters for url-classifier annotation skip URLs; r=johannh

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
Attachment #9055636 - Flags: approval-mozilla-beta?
Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
Has Regression Range: --- → yes

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!

Attachment #9055636 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.