Closed Bug 1386533 Opened 7 years ago Closed 7 years ago

HttpObserverManager.canModify throws breaking WaybackMachine add-on

Categories

(WebExtensions :: Request Handling, defect, P1)

defect

Tracking

(firefox56+ verified, firefox57 verified)

VERIFIED FIXED
mozilla57
Tracking Status
firefox56 + verified
firefox57 --- verified

People

(Reporter: past, Assigned: mixedpuppy)

Details

Attachments

(2 files)

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: nobody → mixedpuppy
Flags: needinfo?(mixedpuppy)
Priority: -- → P1
[Tracking Requested - why for this release]: A small fix for an issue that could affect a lot of extensions.
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.
Pushed by mixedpuppy@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/28f22d3e5ce1
fix host permitted matching, r=kmag
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?
https://hg.mozilla.org/mozilla-central/rev/28f22d3e5ce1
Status: NEW → RESOLVED
Closed: 7 years ago
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+
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.
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
Flags: needinfo?(mixedpuppy)
Attached image 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.
Status: RESOLVED → VERIFIED
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.