Closed Bug 1473194 Opened Last year Closed 11 months ago

Incorrect/Misleading "More Information" link in "Add-ons may be causing problems" dialog

Categories

(Toolkit :: Blocklist Implementation, defect)

58 Branch
defect
Not set

Tracking

()

RESOLVED WORKSFORME

People

(Reporter: dominikschmid93, Unassigned)

Details

Attachments

(1 file)

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:58.0) Gecko/20100101 Firefox/58.0
Build ID: 20180219150722

Steps to reproduce:

1.) Install the "Stylish" add-on
2.) Receive Notification popup caused by https://bugzilla.mozilla.org/show_bug.cgi?id=1472948

3.) Click the "More info"


Actual results:

Link takes me to https://blocked.cdn.mozilla.net/i19.html which is a block from 2011 for different reasons.


Expected results:

Link takes me to https://blocked.cdn.mozilla.net/c635229f-7aa0-44c5-914f-80c590949071.html
Component: Untriaged → Blocklisting
Product: Firefox → Toolkit
I briefly looked into this and it seems that the reason for the wrong url is actually in the content of the blocklist xml data, which have both the newly added version range and the old one into the same blockID "i19":

```
<emItem blockID="i19" id="ADDON_ID">
    <prefs/>
    <versionRange minVersion="1.1b1" maxVersion="1.1b1" severity="1" />
    <versionRange minVersion="3.0.0" maxVersion="3.1.1" severity="1" />
</emItem>
```

Firefox generates the blocked url using the internal helper `Blocklist._createBlocklistURL(blItem.blockID)`:

- https://searchfox.org/mozilla-central/rev/403038737ba75af3842ba6b43b6e2fb47eb06609/toolkit/mozapps/extensions/Blocklist.jsm#455
- https://searchfox.org/mozilla-central/rev/403038737ba75af3842ba6b43b6e2fb47eb06609/toolkit/mozapps/extensions/Blocklist.jsm#565-568

And so, Firefox is going to generate the right blocklist url if the new versionRange is in a blocklist with the expected blockID "c635229f-7aa0-44c5-914f-80c590949071", but that blockID doesn't seem to be currently part of the generate blocklist xml data.

Hey Mathieu, 
can you take a look or redirect us to someone who can?
Flags: needinfo?(mathieu)
The raw data looks OK, it contains an entry for the addon of Bug 1472948:
https://firefox.settings.services.mozilla.com/v1/buckets/blocklists/collections/addons/records?_limit=10

The problem seems to come from the server part that takes these entries and generates the XML. The implementation was reversed engineered from the old Olympia app, and is relatively delicate.

You spotted it: the problem happens when several entries are defined for the same addon ID.

When looking at this code:
https://github.com/mozilla-services/amo2kinto/blob/0a77c360fffec18453287d9c35858d6024ef9c17/amo2kinto/exporter.py#L110-L120

It seems that if the first entry has a `blockID` attribute and the second does not, then the XML node ID is not overriden (it keeps the id of the first one).

The blockID attribute (ie. i19) is the legacy identifier attribute from Olympia, and should not be mandatory in the Kinto stack since we use the `id` field as the main one (ie. c635229f-7aa0-44c5-914f-80c590949071).

But exceptionally, as a workaround to overcome the piece of python code that collapses entries together, I would suggest to manually set a blockID field on the latest entry to a value that is superior to the previous entry (eg. i1900).

Is the blocking affected or just the «more info» link?
Flags: needinfo?(mathieu)
(In reply to Mathieu Leplatre (:leplatrem) from comment #2)
> Is the blocking affected or just the «more info» link?

The blocking worked successfully as I got the notification on multiple devices today and clicking the "restart firefox" button on the dialog has correctly disabled the addon.
Thanks, Mathieu, that worked great. I fixed it for this entry, but keep this bug open to see whether we need a general fix in order to prevent this from happening in the future.
Component: Blocklist Policy Requests → Blocklist Implementation
(In reply to Andreas Wagner [:TheOne] [BACK Sept 24] [use NI] from comment #4)
> Thanks, Mathieu, that worked great. I fixed it for this entry, but keep this
> bug open to see whether we need a general fix in order to prevent this from
> happening in the future.

Based on comment #2 this sounds like an issue on the server-side that should be fixed there, so I'm tempted to close this as WFM now that it's been worked around on the AMO side. I'm not sure we can do something about this client-side, unless we already have access to the guid IDs in the XML file, and can use those? Mathieu, is there something to be done client-side before we switch to kinto?
Flags: needinfo?(mathieu)
The fix was deployed on the server already. So yes, we can mark this as resolved.
Flags: needinfo?(mathieu)
Alright, let's close this out then. Thanks!
Status: UNCONFIRMED → RESOLVED
Closed: 11 months ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.