1298923 - Prevent installation of an extension when "*" is used as part of strict_min_version in a WebExtension manifest.json

Status: VERIFIED FIXED

(WebExtensions :: General, defect, P1)

Description

[Bob Silverberg [:bsilverberg]]

This is a follow-up to bug #1297752 to change the warning to an error. The intent is to uplift bug #1297752, which only generates a warning, to beta, and then to let this bug, which will generate an error, preventing installation, to ride the trains.

Comment 1

[Bob Silverberg [:bsilverberg]]

Attachment: Bug 1298923 - Prevent installation of an extension when "*" is used as part of strict_min_version in a WebExtension manifest.json,

Review commit: https://reviewboard.mozilla.org/r/76130/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/76130/

Comment 2

[Kris Maglione [:kmag]]

Comment on attachment 8787333 [details]
Bug 1298923 - Prevent installation of an extension when "*" is used as part of strict_min_version in a WebExtension manifest.json,

https://reviewboard.mozilla.org/r/76130/#review75242

::: toolkit/mozapps/extensions/internal/XPIProvider.jsm:934
(Diff revision 1)
>      logger.warn("Ignoring applications property in manifest");
>    }
>  
>    // A * is illegal in strict_min_version
>    if (bss.strict_min_version && bss.strict_min_version.split(".").some(part => part == "*")) {
> -    logger.warn("The use of '*' in strict_min_version is deprecated");
> +    throw new Error("The use of '*' in strict_min_version is forbidden");

s/forbidden/invalid/

::: toolkit/mozapps/extensions/test/xpcshell/test_webextension_install.js:385
(Diff revision 1)
> +    try {
>        yield AddonManager.installTemporaryAddon(addonDir);
> -    });
> -    ok(messages.some(msg => msg.message.includes("The use of '*' in strict_min_version is deprecated")),
> -       "Deprecation warning for strict_min_version with '*' was generated");
> -
> -    let addon = yield promiseAddonByID(newId);
> +    } catch (err) {
> +      if (err.message == "The use of '*' in strict_min_version is forbidden") {
> +        expectedExceptionThrown = true;
> +      }
> +    }

`Assert.throws`

Comment 3

[Bob Silverberg [:bsilverberg]]

Comment on attachment 8787333 [details]
Bug 1298923 - Prevent installation of an extension when "*" is used as part of strict_min_version in a WebExtension manifest.json,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/76130/diff/1-2/

Comment 4

[Bob Silverberg [:bsilverberg]]

Comment on attachment 8787333 [details]
Bug 1298923 - Prevent installation of an extension when "*" is used as part of strict_min_version in a WebExtension manifest.json,

https://reviewboard.mozilla.org/r/76130/#review75242

> `Assert.throws`

I've tried converting this to `Assert.throws` a number of ways, but none of them work. I end up with `Missing expected exception.` I've left one example in the latest commit. I've also tried `yield`ing inside the function passed to `Assert.throws` but that doesn't help. Can you suggest a syntax that would work?

Comment 5

[Kris Maglione [:kmag]]

Comment on attachment 8787333 [details]
Bug 1298923 - Prevent installation of an extension when "*" is used as part of strict_min_version in a WebExtension manifest.json,

https://reviewboard.mozilla.org/r/76130/#review75242

> I've tried converting this to `Assert.throws` a number of ways, but none of them work. I end up with `Missing expected exception.` I've left one example in the latest commit. I've also tried `yield`ing inside the function passed to `Assert.throws` but that doesn't help. Can you suggest a syntax that would work?

Sorry, it needs to be `yield Assert.rejects(...)` in this case.

Comment 6

[Bob Silverberg [:bsilverberg]]

Comment on attachment 8787333 [details]
Bug 1298923 - Prevent installation of an extension when "*" is used as part of strict_min_version in a WebExtension manifest.json,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/76130/diff/2-3/

Comment 7

[Bob Silverberg [:bsilverberg]]

Comment on attachment 8787333 [details]
Bug 1298923 - Prevent installation of an extension when "*" is used as part of strict_min_version in a WebExtension manifest.json,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/76130/diff/3-4/

Comment 8

[Kris Maglione [:kmag]]

Comment on attachment 8787333 [details]
Bug 1298923 - Prevent installation of an extension when "*" is used as part of strict_min_version in a WebExtension manifest.json,

https://reviewboard.mozilla.org/r/76130/#review76724

Comment 9

[Bob Silverberg [:bsilverberg]]

Try looks good. Requesting check-in.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e2707a19e5cb
Prevent installation of an extension when "*" is used as part of strict_min_version in a WebExtension manifest.json, r=kmag

Comment 11

[Carsten Book [:Tomcat]]

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

Comment 12

[Vasilica Tamas, QA [:vtamas]]

Confirm that there is generated an error while trying to install a webextension which uses "*" as part of strict_min_version on Firefox 51.0a2 (2016-09-29) under Windows 10 64-bit and Ubuntu 16.04 32-bit.

   - error thrown while installing via drag&drop/install from file: http://pastebin.com/Fgcxvtzd
   - error thrown while installing via about:debugging: http://pastebin.com/iA8GdCDT , http://screencast.com/t/QHgsihoWrt