HttpObserverManager.canModify throws breaking WaybackMachine add-on

VERIFIED FIXED in Firefox 56

Status

P1
normal
VERIFIED FIXED
a year ago
6 months ago

People

(Reporter: past, Assigned: mixedpuppy)

Tracking

unspecified
mozilla57

Firefox Tracking Flags

(firefox56+ verified, firefox57 verified)

Details

Attachments

(2 attachments)

With the WaybackMachine extension enabled (1.8.1) latest nightly breaks opening a new tab with the following log in the console:

[Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIURI.host]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: resource://gre/modules/WebRequest.jsm :: canModify :: line 852"  data: no] (unknown)
	canModify resource://gre/modules/WebRequest.jsm:852:38
	runChannelListener resource://gre/modules/WebRequest.jsm:883:23
	applyChanges resource://gre/modules/WebRequest.jsm:1002:15
	next self-hosted:1183:9
tab is undefined  background.js:49
Unchecked lastError value: Error: Invalid tab ID: -1  ExtensionCommon.jsm:304

My guess is that this code here needs to put a try/catch around loadingPrincipal.URI.host which throws in about: pages:

http://searchfox.org/mozilla-central/rev/bbc1c59e460a27b20929b56489e2e55438de81fa/toolkit/modules/addons/WebRequest.jsm#852
Flags: needinfo?(mixedpuppy)
(Assignee)

Updated

a year ago
Assignee: nobody → mixedpuppy
Flags: needinfo?(mixedpuppy)
Priority: -- → P1
Comment hidden (mozreview-request)
(Assignee)

Comment 2

a year ago
[Tracking Requested - why for this release]: A small fix for an issue that could affect a lot of extensions.
status-firefox56: --- → affected
status-firefox57: --- → affected
tracking-firefox56: --- → ?

Comment 3

a year ago
mozreview-review
Comment on attachment 8893127 [details]
Bug 1386533 fix host permitted matching,

https://reviewboard.mozilla.org/r/164126/#review169528
Attachment #8893127 - Flags: review?(kmaglione+bmo) → review+
Track 56+ as the impact on lots of extensions.
tracking-firefox56: ? → +
(Assignee)

Comment 6

a year ago
Comment on attachment 8893127 [details]
Bug 1386533 fix host permitted matching,

Making an early request, this is a simple fix that can be uplifted.

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1308688
[User impact if declined]: some request monitoring fails
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: just landed
[Needs manual test from QE? If yes, steps to reproduce]: install wayback machine addon and use to view recent versions of a webpage.
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: no
[Why is the change risky/not risky?]: It's just a catch around nsIURI.host access which throws for non-host URIs such as "about:"
[String changes made/needed]: none
Attachment #8893127 - Flags: approval-mozilla-beta?

Comment 7

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/28f22d3e5ce1
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox57: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Comment on attachment 8893127 [details]
Bug 1386533 fix host permitted matching,

Small fix that may have a good impact on many webextensions.
Attachment #8893127 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Comment 10

a year ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/25aa08f84e10
status-firefox56: affected → fixed
I had to back this out from Beta for causing frequent NS_ERROR_SOCKET_ADDRESS_IN_USE failures on ASAN mochitest-bc runs. I confirmed this patch was the culprit after running the backout through Try and doing mass retriggers to confirm.

https://hg.mozilla.org/releases/mozilla-beta/rev/667773e6e8c1

Here's a sampling of what the failures looked like:
https://treeherder.mozilla.org/logviewer.html#?job_id=122289586&repo=try
https://treeherder.mozilla.org/logviewer.html#?job_id=122291164&repo=try
https://treeherder.mozilla.org/logviewer.html#?job_id=122289321&repo=try

I had initially reported them under bug 1389063, but the frequency was high enough that I didn't feel comfortable leaving the patch in without further investigation.
status-firefox56: fixed → affected
Flags: needinfo?(mixedpuppy)
*sigh* 160 green runs on Try, but still hitting the failure on Beta :(. Re-landed and apologies for the churn.

https://hg.mozilla.org/releases/mozilla-beta/rev/2638feb177dd9104a3887fae2ce05931a6b41d6c
status-firefox56: affected → fixed
Flags: needinfo?(mixedpuppy)

Comment 13

a year ago
Created attachment 8918903 [details]
version check.gif

This bug is verified on Firefox 56.0 (20170926190823) and Firefox 57.0b8 (20171013042429) under Windows 10 64bit/Mac OS X 10.12.3.

Please see the attached video.

Updated

a year ago
Status: RESOLVED → VERIFIED
status-firefox56: fixed → verified
status-firefox57: fixed → verified

Updated

6 months ago
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.