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

Status: VERIFIED FIXED

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

Description

[Daniel Bodea [:danibodea]]

Attachment: first reopen blocked homepage shown plus extension doorhanger.png

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.

Comment 1

[Adrian Florinescu [:aflorinescu]]

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.

Comment 2

[Mark Banner (:standard8)]

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?

Comment 3

[Daniel Bodea [:danibodea]]

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.

Comment 4

[Mike de Boer [:mikedeboer]]

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

Comment 5

[Mark Banner (:standard8)]

(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.

Comment 6

[Daniel Bodea [:danibodea]]

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?

Comment 7

[Mark Banner (: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.

Comment 8

[Mark Banner (:standard8)]

(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.

Comment 9

[Mark Banner (:standard8)]

Attachment: Bug 1583479 - Fix case-insensitive comparison when comparing against the home page ignore list when setting the home page value. r?mikedeboer

Comment 10

[Daniel Bodea [:danibodea]]

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.

Comment 11

[Daniel Bodea [:danibodea]]

Updating the title to the one chosen by the dev.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
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

Comment 13

[Natalia Csoregi [:nataliaCs]]

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

Comment 14

[Mark Banner (:standard8)]

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

Comment 15

[Daniel Bodea [:danibodea]]

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

Comment 16

[Liz Henry (:lizzard) (relman/hg->git project)]

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.

Comment 17

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-beta/rev/baa92051f2a0

Comment 18

[Daniel Bodea [:danibodea]]

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