1317366 - webRequest throws when a blocking listener returns undefined

Status: VERIFIED FIXED

(WebExtensions :: Untriaged, defect)

Description

[u287251]

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:49.0) Gecko/20100101 Firefox/49.0
Build ID: 20161028133011

Steps to reproduce:

Tested the WebExtensions version of uBlock Origin 1.9.17b9:
https://github.com/gorhill/uBlock/releases/tag/1.9.17b9



Actual results:

When loading any web page, the browser console is filled with entries like:

result is undefined WebRequest.jsm:752
	HttpObserverManager.applyChanges< resource://gre/modules/WebRequest.jsm:752:1
	next self-hosted:1120:9
	TaskImpl_run resource://gre/modules/Task.jsm:319:42
	bound TaskImpl_run self-hosted:957:17

This is because the code at WebRequest.jsm:752 does not 1st check whether the returned value from a webRequest.onBeforeRequest listener is undefined, it assumes an Object is always returned.

However, the documentation regarding listeners for chrome.webRequest.onBeforeRequest et al says[1]:

> the callback can return a webRequest.BlockingResponse that determines
> the further life cycle of the request.

So as per documentation, a callback CAN return a BlockingResponse object for when a request has to be canceled or redirected. However, when no cancellation of redirection is needed, it is ok to return nothing, i.e. `undefined`.

[1] https://developer.chrome.com/extensions/webRequest


Expected results:

No such error in the browser console.

Comment 1

[Kris Maglione [:kmag]]

Attachment: Bug 1317366: Handle blocking WebRequest listeners returning non-object values.

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

Comment 2

[Shane Caraveo (:mixedpuppy)]

Comment on attachment 8810721 [details]
Bug 1317366: Handle blocking WebRequest listeners returning non-object values.

https://reviewboard.mozilla.org/r/93030/#review92938

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/69cd55731647
Handle blocking WebRequest listeners returning non-object values. r=mixedpuppy

Comment 4

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/69cd55731647

Comment 5

[Kris Maglione [:kmag]]

Comment on attachment 8810721 [details]
Bug 1317366: Handle blocking WebRequest listeners returning non-object values.

Approval Request Comment
[Feature/regressing bug #]: But 1305217
[User impact if declined]: This bug prevents certain WebExtension request listeners from modifying or canceling requests.
[Describe test coverage new/current, TreeHerder]: The feature in question is extensively tested, and new tests have been added for this feature.
[Risks and why]: Very low. This patch simply adds an additional null check to promise resolution values which we previously performed only for non-promise values.
[String/UUID change made/needed]: None.

Comment 6

[Julien Cristau [:jcristau] (back April 22)]

Comment on attachment 8810721 [details]
Bug 1317366: Handle blocking WebRequest listeners returning non-object values.

let's fix this regression from a couple of days ago in 52 aurora

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/3375fcac31eb

Comment 8

[CosminB]

I can reproduce this issue on Firefox 53.0a1 (20161114043454) under Windows 7 64-bit and Ubuntu 16.04 LTS 32-bit,here is a video: http://screencast.com/t/3QbgwyDsED 

This issue is verified as fixed on Firefox 53.0a1 (20161123030208) and Firefox 52.0a2 (20161123004021) under Windows 7 64-bit and Ubuntu 16.04 LTS 32-bit, there is no WebRequest error in the Browser Console. Here is a video: http://screencast.com/t/IFHOL3TC0EAa