1343498 - WebExtensions with invalid match pattern are not installed

Status: VERIFIED FIXED

Description

[CosminB]

[Affected versions]:
- Firefox 54.0a1 (2017-02-28) 
- Firefox 53.0a2 (20170228004003)

[Affected platforms]:
- Windows 7 64-bit
- Ubuntu 16.04 32-bit

[Steps to reproduce]:
1.Launch Firefox with clean profile.
2.Create extensions.webextPermissionPrompts and set it to true.
3.Create xpinstall.signatures.dev-root and set it to true.
4.Restart the browser
5.Install the following WebExtension: https://addons-dev.allizom.org/en-US/firefox/addon/permissionsall2/ 

[Expected results]:
- The corrupt message is displayed.

[Actual results]:
- No corrupt message is displayed and the installation is canceled.
- The following errors are displayed in the browser console: 
  Invalid match pattern: 'history$%^'  MatchPattern.jsm:53
  Error: Unparseable host permission  ExtensionsUI.jsm:248:15

[Additional notes]:
- This issue reproduces constantly.
- The permissions that have an invalid match pattern passes the submission without any warnings. 
- Here is a video of this issue https://www.screencast.com/t/KhupkORUc

Comment 1

[:shell escalante]

believe the linter with full schema will check this before signing.  Andy file matching github bug

in AMO code trying to figure out what is host permissions versus add-on permissions.  andrew taking bug

Comment 2

[Andrew Swan [:aswan]]

We should tighten up this code so this isn't fatal.  The patches for bug 1197420 add a "classifyPermissions()" function to ExtensionUtils, I'll put up a patch for this bug once that one lands.

Comment 3

[CosminB]

I already filed a bug for this issue in GitHub on addons-linter, here is the bug https://github.com/mozilla/addons-linter/issues/1150

Comment 4

[Andrew Swan [:aswan]]

Created attachment 8858449 [details]
Bug 1343498: Don't fail on unparseable host permissions

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

Comment 5

[Kris Maglione [:kmag] (long backlog; ping on IRC if you're blocked)]

Comment on attachment 8858449 [details]
Bug 1343498: Don't fail on unparseable host permissions

https://reviewboard.mozilla.org/r/130414/#review133868

::: browser/modules/ExtensionsUI.jsm:250
(Diff revision 1)
>          allUrls = true;
>          break;
>        }
>        let match = /^[htps*]+:\/\/([^/]+)\//.exec(permission);
>        if (!match) {
> -        throw new Error("Unparseable host permission");
> +        Cu.reportError(`Unparseable host permission ${permission}`);

Hm. We should really change the schema code to drop invalid permissions... but I guess this is OK for now.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1c7dacc277a7
Don't fail on unparseable host permissions r=kmag

Comment 7

[Wes Kocher (:KWierso)]

https://hg.mozilla.org/mozilla-central/rev/1c7dacc277a7

Comment 8

[CosminB]

Created attachment 8860938 [details]
UnparseablePermissions.xpi

This issue is verified as fixed on Firefox 55.0a1 (2017-04-24) under Wind 7 64-bit, Ubuntu 16.04 32-bit and Mac OS X 10.10.4

The following error is displayed for an unparseable host permissions: “Unparseable host permission (permission)”
Please see the attached file.

Comment 9

[Liz Henry (:lizzard) (needinfo? me)]

Should this fix be uplifted to 54? Or should it stay in 55?

Comment 10

[Andrew Swan [:aswan]]

(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #9)
> Should this fix be uplifted to 54? Or should it stay in 55?

This code was present in 54 but behind a preference that was off by default (it was enabled in bug 1342142 which landed in 55).
So I don't think an uplift is necessary.