Closed Bug 1343498 Opened 3 years ago Closed 3 years ago

WebExtensions with invalid match pattern are not installed

Categories

(Toolkit :: Add-ons Manager, defect, P2)

defect

Tracking

()

VERIFIED FIXED
mozilla55
Tracking Status
firefox51 --- unaffected
firefox52 --- unaffected
firefox53 --- wontfix
firefox54 --- disabled
firefox55 --- verified

People

(Reporter: cbadescu, Assigned: aswan)

References

Details

(Whiteboard: triaged)

Attachments

(2 files)

[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
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
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.
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 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+
Pushed by aswan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1c7dacc277a7
Don't fail on unparseable host permissions r=kmag
https://hg.mozilla.org/mozilla-central/rev/1c7dacc277a7
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
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.
Status: RESOLVED → VERIFIED
Should this fix be uplifted to 54? Or should it stay in 55?
Flags: needinfo?(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.
Flags: needinfo?(aswan)
You need to log in before you can comment on or make changes to this bug.