WebExtensions with invalid match pattern are not installed

VERIFIED FIXED in Firefox 55

Status

()

Toolkit
Add-ons Manager
P2
normal
VERIFIED FIXED
a year ago
11 months ago

People

(Reporter: CosminB, Assigned: aswan)

Tracking

Trunk
mozilla55
Points:
---

Firefox Tracking Flags

(firefox51 unaffected, firefox52 unaffected, firefox53 wontfix, firefox54 disabled, firefox55 verified)

Details

(Whiteboard: triaged)

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

(Reporter)

Description

a year ago
[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

a year ago
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
Assignee: nobody → aswan
Flags: needinfo?(amckay)
Priority: -- → P2
Whiteboard: triaged
(Assignee)

Comment 2

a year ago
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.
(Reporter)

Comment 3

a year ago
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
Flags: needinfo?(amckay)
Comment hidden (mozreview-request)
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.
Attachment #8858449 - Flags: review?(kmaglione+bmo) → review+

Comment 6

a year ago
Pushed by aswan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1c7dacc277a7
Don't fail on unparseable host permissions r=kmag

Comment 7

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/1c7dacc277a7
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
(Reporter)

Comment 8

a year ago
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.
(Reporter)

Updated

a year ago
Status: RESOLVED → VERIFIED
status-firefox55: fixed → verified
Should this fix be uplifted to 54? Or should it stay in 55?
status-firefox53: affected → wontfix
Flags: needinfo?(aswan)
(Assignee)

Comment 10

11 months ago
(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.
Flags: needinfo?(aswan)
status-firefox54: affected → disabled
You need to log in before you can comment on or make changes to this bug.