1304331 - Content-Type header changed in Web Extension is not taken into account

Status: VERIFIED FIXED

(WebExtensions :: Request Handling, defect)

Description

[user6770522]

Attachment: content-type-issue.zip

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

Steps to reproduce:

1. Create a simple web server that serves HTML files with Content-Type "application/octet-stream". See attached "server.py" and "test.html".
2. Create a Web Extension and override the Content-Type using "chrome.webRequest.onHeadersReceived.addListener". See attached manifest.json and "background.js".
3. Activate the Web Extension in Firefox (via about:debugging).
4. Start the simple server and issue a request (e.g. http://localhost:8080/test.html).


Actual results:

The browser prompts the user to download the HTML file.


Expected results:

The browser should display the HTML page as a normal web page.

Comment 1

[Kris Maglione [:kmag]]

Attachment: Bug 1304331: Support setting the Content-Type header from WebRequest observers.

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

Comment 2

[Giorgio Maone [:ma1]]

Comment on attachment 8793306 [details]
Bug 1304331: Support setting the Content-Type header from WebRequest observers.

https://reviewboard.mozilla.org/r/80078/#review78812

Thanks for the fix. Just out of curiosity, why did you add console in Extension.jsm?

Comment 3

[Kris Maglione [:kmag]]

Comment on attachment 8793306 [details]
Bug 1304331: Support setting the Content-Type header from WebRequest observers.

https://reviewboard.mozilla.org/r/80078/#review78812

I went to copy the line that added it into WebRequest.jsm so I could use it for debugging, and realized that it had been removed by the patches for bug 1287010. I meant to fix that in a separate patch, though.

Comment 4

[Kris Maglione [:kmag]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/83a0cb757881b0eb3e0339794b22f0b4c6390e30
Bug 1304331: Support setting the Content-Type header from WebRequest observers. r=mao

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/83a0cb757881

Comment 6

[Liz Henry (:lizzard) (relman/hg->git project)]

Kris, is this a fix you are looking to uplift?

Comment 7

[Kris Maglione [:kmag]]

I'm considering it. I want to see how often it's used and how it goes over on nightly first, though.

Comment 8

[Kris Maglione [:kmag]]

I could only find two extensions in the Chrome store that do this:

https://chrome.google.com/webstore/detail/modify-content-type/jnfofbopfpaoeojgieggflbpcblhfhka
https://chrome.google.com/webstore/detail/l%C3%B6veliness/mompnkcmpbopandjnddeecgeeojegohc

with less than 4k users total. There aren't any WebExtensions for Firefox that do, yet.

It might be a good idea to uplift this to Aurora once it's had a week or two to bake. I'm not sure it should go to beta. Either way, we should document the limitation and the versions it affects.

Comment 9

[Kris Maglione [:kmag]]

Comment on attachment 8793306 [details]
Bug 1304331: Support setting the Content-Type header from WebRequest observers.

Approval Request Comment
[Feature/regressing bug #]: Bug 1232849
[User impact if declined]: This prevents extensions from modifying a common header, which may be necessary for their functionality, and which they should reasonably expect to work. Those extensions will likely unexpectedly fail in versions of Firefox without this fix.
[Describe test coverage new/current, TreeHerder]: The functionality in question has relatively good automated test coverage, and additional tests have been added for this bug.
[Risks and why]: Low. The changed sections of code are all wrapped in try-catch blocks so that any error is treated as a simple inability to modify the header, and will therefore silently revert to the previous behavior. Any issues should simply manifest as an incomplete fix, rather than causing problems we wouldn't otherwise have seen.
[String/UUID change made/needed]: None.

Comment 10

[Gerry Chang [:gchang]]

Hi Florin, can we have some QA's help to verify this?

Comment 11

[Gerry Chang [:gchang]]

Comment on attachment 8793306 [details]
Bug 1304331: Support setting the Content-Type header from WebRequest observers.

Fix a Web Extension related issue which will be in 51. Take it in 51 aurora.

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/a13ac8658440

Comment 13

[Victor Carciu]

Verified as fixed in :
Win7x64 - FF Nightly(52.0a1) 2016-10-06
        - FF Aurora(51.0a2)  2016-10-06

Win10x32 - FF Nightly(52.0a1) 2016-10-06
         - FF Aurora(51.0a2) 2016-10-06

Comment 14

[Victor Carciu]

Attachment: Postfix Video

Comment 15

[Will Bamberg [:wbamberg]]

I've added a browser compat note about "Content-Type":
https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/webRequest/onHeadersReceived#Browser_compatibility

Does this cover it?

Comment 16

[Will Bamberg [:wbamberg]]

Marking as dev-doc-complete, but let me know if I need anything else here.

Comment 17

[Kris Maglione [:kmag]]

I think that probably covers it.