717430 - Run fake bulk validation to detect SDK add-ons that will break with the 1.4 update

Status: RESOLVED FIXED

(addons.mozilla.org Graveyard :: Compatibility Tools, defect, P4)

Description

[Jorge Villalobos [:jorgev] (he/him)]

SDK-based add-ons that hard-code absolute paths to files in their packages will break with the 1.4 update, due to changes in the way files are handled. More details here: https://wiki.mozilla.org/Labs/Jetpack/Release_Notes/1.4#Known_Issues

The Jetpack team want to identify these add-ons so that their developers can be contacted about it. What we need is a validator compatibility test that detects these cases and generates an error. Then we can run a fake Firefox 11 bump where only the developers with failing cases will be contacted. We would need to later remove this test when the real Firefox 11 validation is run.

The error message can be something like "This add-on references absolute resource URIs, which will break in the 1.4 SDK". Jeff should be able to provide an example of an add-on that wouldn't pass this test.

Comment 1

[Matt Basta [:basta]]

How should this be detected? From the link, I'd say we should flag any JavaScript string literals and xul/html/xml elements with src/href attributes that start with "resource://" in Jetpack add-ons. Does this sound reasonable?

Comment 2

[Jeff Griffiths (:canuckistani) (:⚡︎)]

Matt: a slightly more refined pattern would be:

/\"resource:\/\/[\w\-]+?\-data\//

eg, all uris would *also* include the string '-data/' in the string, as the files being referenced are all contained in the 'data' directory of the Addon.

It would also be great if the script would ignore the addon-kit and api-utils directories in a given addon as these will produce some false positives.

Comment 3

[Matt Basta [:basta]]

(In reply to Jeff Griffiths from comment #2)
> It would also be great if the script would ignore the addon-kit and
> api-utils directories in a given addon as these will produce some false
> positives.

Can you give an example of one of these URIs?

Comment 4

[Wil Clouser [:clouserw]]

How many add-ons are we talking about?  Will MXR not work for this?

Comment 5

[Jorge Villalobos [:jorgev] (he/him)]

(In reply to Wil Clouser [:clouserw] from comment #4)
> How many add-ons are we talking about?  Will MXR not work for this?

We tried using MXR, but it wasn't sufficient since there are too many matches coming from SDK library files and its regex features are very limited.

Comment 6

[Jeff Griffiths (:canuckistani) (:⚡︎)]

Here are two static uris that are generated using SDK 1.3:

resource://jid0-odikjs9b4it3h1nylpkr0ndtlue-at-jetpack-copy-shorturl-data/js/find-short-url.js

resource://jid0-odikjs9b4it3h1nylpkr0ndtlue-at-jetpack-copy-shorturl-data/img/ruler.png

We would want to find string literals like this in JavaScript and html files in addons. When scanning a given addon if possible it would be good to skip the directories in the addon that store the SDK's own libraries, for example given an addon with an id 'jid0-odikjs9b4it3h1nylpkr0ndtlue-at-jetpack'...

Scan inside these directories:

resources/jid0-odikjs9b4it3h1nylpkr0ndtlue-at-jetpack-copy-shorturl-lib/
resources/jid0-odikjs9b4it3h1nylpkr0ndtlue-at-jetpack-copy-shorturl-data/

DO NOT scan into these directories:

resources/jid0-odikjs9b4it3h1nylpkr0ndtlue-at-jetpack-api-utils-lib/
resources/jid0-odikjs9b4it3h1nylpkr0ndtlue-at-jetpack-api-utils-data/

resources/jid0-odikjs9b4it3h1nylpkr0ndtlue-at-jetpack-addon-kit-lib/
resources/jid0-odikjs9b4it3h1nylpkr0ndtlue-at-jetpack-addon-kit-data/

Comment 7

[Andrew Williamson [:eviljeff]]

https://addons.mozilla.org/en-US/firefox/addon/read-later-fast/(In reply to Matt Basta from comment #3)
> (In reply to Jeff Griffiths from comment #2)
> > It would also be great if the script would ignore the addon-kit and
> > api-utils directories in a given addon as these will produce some false
> > positives.
> 
> Can you give an example of one of these URIs?

resources/jid0-ayilkqe9scrtfyorhwez2iaxcro-api-utils-lib/self.js

[https://mxr.mozilla.org/addons/source/303585/]

Comment 8

[Matt Basta [:basta]]

> DO NOT scan into these directories:

Keep in mind that we currently don't scan any files in an add-on if it matches a hash from a file in the SDK.

Comment 9

[Wil Clouser [:clouserw]]

Leveraging the bulk validation for this is kind of scary and not particularly related.  We should just make a command that selects the right add-ons, runs validation on them, and prints the output.

I'm assigning to mbasta for the logic in the validator for now.

Comment 10

[Matt Basta [:basta]]

Done

https://github.com/mozilla/amo-validator/pull/118

So here's how this works: any Jetpack add-on that contains the issue in this bug will throw a compatibility error, even if no versions to test for compatibility have been selected. This means that a bare compatibility run (with no "forced" version, like we'd use to test for compatibility with Firefox) will yield all add-ons that aren't compatible with the SDK 1.4.

If this is going to muck with things, I can get rid of the compatibility error and the error can be detected by iterating the messages like this:

any(m["id"][-1] == "jetpack_abs_uri" for m in messages)

Comment 11

[Matt Basta [:basta]]

Merged:

https://github.com/mattbasta/amo-validator/commit/fb2c57b9d63f8becdaa155ce592fc8cfe9cd4bb9