Closed Bug 1583479 Opened 3 months ago Closed 2 months ago

Updates of the home page blocklist whilst Firefox is running have incorrect case-insensitive comparisons

Categories

(Firefox :: New Tab Page, defect, P2, critical)

Desktop
All
defect
Points:
1

Tracking

()

VERIFIED FIXED
Firefox 71
Iteration:
71.3 - Sept 30 - Oct 13
Tracking Status
firefox69 --- disabled
firefox70 --- verified
firefox71 --- verified

People

(Reporter: danibodea, Assigned: standard8)

References

(Blocks 3 open bugs)

Details

Attachments

(2 files)

Note

  • When the blocklist string is saved in uppercase and the homepage link contains the blocklist string, but with uppercase characters, and IF the blocklist is retrieved before installing an extension that hijacks the homepage, then the homepage preferences will only be reverted after the first restart AND the set homepage will be opened at first reopen of the profile, even if it's blocklisted.

Affected versions

  • Nightly v71.0a1 from 2019-09-23 and Nightly v71.0a1 form 2019-10-03
  • Beta v70.0b8 and Beta v70.0b11

Affected platforms

  • All

Steps to reproduce

  1. Make sure that the "pc=U510" or "PC=U510" (NOT "pc=u510" or "bing") are added to the "homepage-urls" collection of the hijack-blocklists and no other strings that would block the homepage are added.
  2. Install the Remote Settings Devtools extension
    https://github.com/mozilla/remote-settings-devtools/releases/tag/1.1.0
  3. Click the "Remote Settings Devtools" extension icon from the toolbar.
  4. Change the Environment setting (from the top-right corner) to Stage.
  5. Click on "Clear ALL local data" button.
  6. Click on the "Poll Server" button (or click on the "Force Sync" button from the "main/hijack-blocklists" Collection.
  7. Open Bing Homepage and Search Engine and install the extension:
    https://addons.mozilla.org/ro/firefox/addon/bing-homepage-searchengine/?src=search
    Observation: The homepage is NOT instantly blocked and reverted to original settings.
  8. Close the browser and reopen (as a normal user).

Expected result

  • The homepage preferences set by the Bing extension are being removed, reverting the original homepage preferences and the blocked homepage is not loaded.

Actual result

  • The blocked homepage is loaded after first restart AND the extension door hanger that notifies the user that "Your homepage has been changed." at this point when it's already blocked and reverted.
Component: New Tab Page → Search

Raising the severity for this bug due to the fact that the user experience is confusing and lacking in this particular scenario:

  • we allow the extension to set the homepage and also allow user to confirm the homepage change.
  • we overwrite explicit user action on restart.
Severity: normal → critical
Blocks: 1578508

I've just tried this on today's nightly and it is working fine for me. I can't think how this would be broken either, as if it is in the ignore list, we just totally skip that section when installing the add-on.

I'm wondering if the blocklists hadn't fully synced at the time of installing the add-on.

Bodea, please could you try again?

Flags: needinfo?(daniel.bodea)

This was reproduced constantly yesterday on at least 3 different machines. It appears as it does not reproduce anymore. Any idea on how to troubleshoot it? Has anything changed serverside? It seems a bit risky to set it as WORKSFORME without knowing what caused it to reproduce.

Flags: needinfo?(daniel.bodea) → needinfo?(standard8)

Could it be that perhaps there was an infrastructure/ Remote Settings issue in play yesterday?

(In reply to Bodea Daniel [:danibodea] from comment #3)

This was reproduced constantly yesterday on at least 3 different machines. It appears as it does not reproduce anymore. Any idea on how to troubleshoot it? Has anything changed serverside? It seems a bit risky to set it as WORKSFORME without knowing what caused it to reproduce.

The only thing I saw today was that sometimes doing the "poll updates" could take a while.

I don't know if you were waiting for the in-page display to update or not after triggering the poll. If you weren't, that could explain what was happening here, as the blocklist update wouldn't have been received.

Otherwise, I don't know if we can't reproduce it.

Flags: needinfo?(standard8)

I found the culprit! It was a change in the blocklist, done by the other peer tester. I do not understand why this behaves as such, but this was certainly the cause.

  • Initially, I have added the string "PC=U510" and the bug would reproduce (the Bing homepage would be set even if it would already be blocklisted by the "PC=U510" string), but it would get blocked and reverted to default after the first restart.
  • Then, the "bing" string (that is contained in the domain of the homepage link) was added to the homepage blocklist as well and after this, the Bing homepage would not be set at all when installing the extension.
  • I had to also try it when only the "bing" string is added to the blocklist and it gives the same result as when both the "PC=U510" and "bing" strings are added together: the bug does not reproduce.

In conclusion, it appears that if the "bing" string is added to the blocklist, it behaves differently than when the "PC=U510" string is added.
The bug is still valid, but there might be another issue being uncovered by this revelation.

Mark, what do you think?

Flags: needinfo?(standard8)

The issue here is that when we initialise on startup, the codes are being treated as case-insensitive. However, when we get an update whilst we're already initialised, then we've got a bad case-comparison that means PC=U510 is not matching pc=u510 - the code is lower casing the url, but not the codes from the blocklist.

That's an easy fix that I'll put a patch up for in a moment.

Assignee: nobody → standard8
Status: NEW → ASSIGNED
Iteration: --- → 71.3 - Sept 30 - Oct 13
Points: --- → 1
Component: Search → New Tab Page
Flags: needinfo?(standard8)
Priority: -- → P2
Summary: When the blocklist is retrieved before installing an extension that hijacks the homepage, then the homepage set will be displayed at first reopen → Updates of the home page blocklist whilst Firefox is running have incorrect case-insensitive comparisons

(In reply to Mark Banner (:standard8) from comment #7)

The issue here is that when we initialise on startup, the codes are being treated as case-insensitive. However, when we get an update whilst we're already initialised, then we've got a bad case-comparison that means PC=U510 is not matching pc=u510 - the code is lower casing the url, but not the codes from the blocklist.

Slight correction: startup & in-update are working fine. It is when a new add-on is installed, or when the homepage preference is set, where we have the bad case checking.

After talking with Mark, we have found the actual cause of the issue. This being said, I will modify the bug to match the findings.

Assignee: standard8 → nobody
Status: ASSIGNED → NEW
Iteration: 71.3 - Sept 30 - Oct 13 → ---
Points: 1 → ---
Component: New Tab Page → Search
Priority: P2 → --
Summary: Updates of the home page blocklist whilst Firefox is running have incorrect case-insensitive comparisons → When the blocklist string is saved in uppercase and the homepage link contains the blocklist string, but with uppercase characters, then the homepage will only be blocked after the first restart
Assignee: nobody → standard8
Status: NEW → ASSIGNED
Iteration: --- → 71.3 - Sept 30 - Oct 13
Points: --- → 1
Component: Search → New Tab Page
Priority: -- → P2

Updating the title to the one chosen by the dev.

Summary: When the blocklist string is saved in uppercase and the homepage link contains the blocklist string, but with uppercase characters, then the homepage will only be blocked after the first restart → Updates of the home page blocklist whilst Firefox is running have incorrect case-insensitive comparisons
Pushed by mbanner@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/fc32527395e2
Fix case-insensitive comparison when comparing against the home page ignore list when setting the home page value. r=mikedeboer
Status: ASSIGNED → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 71

Comment on attachment 9098797 [details]
Bug 1583479 - Fix case-insensitive comparison when comparing against the home page ignore list when setting the home page value. r?mikedeboer

Beta/Release Uplift Approval Request

  • User impact if declined: Fixes a minor issue in the ignorelist where the list should be compared via case insensitive rather than case sensitive. This could mean we don't block correctly if we rolled out an ignore list update where we had upper case strings.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: See comment 0
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Small correction, covered by unit tests.
  • String changes made/needed: None
Attachment #9098797 - Flags: approval-mozilla-beta?
Flags: qe-verify+
QA Whiteboard: [qa-triaged]

I have verified this fix in Nightly v71.0a1 from (2019-10-06) and it still reproduces in beta v71.0b12. Awaiting uplift.

QA Whiteboard: [qa-triaged]
QA Whiteboard: [qa-triaged]

Comment on attachment 9098797 [details]
Bug 1583479 - Fix case-insensitive comparison when comparing against the home page ignore list when setting the home page value. r?mikedeboer

Blocklist fix, looks simple, verified in nightly, OK for uplift for beta 13.

Attachment #9098797 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Thi fix was also verified in Beta v71.0b13, build ID: 20191007220302 on Windows 10 and Mac OS 10.14.

Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.