Closed Bug 1304331 Opened 7 years ago Closed 7 years ago

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

Categories

(WebExtensions :: Request Handling, defect)

47 Branch
defect
Not set
normal

Tracking

(firefox49 wontfix, firefox-esr45 unaffected, firefox50 affected, firefox51 verified, firefox52 verified)

VERIFIED FIXED
mozilla52
Tracking Status
firefox49 --- wontfix
firefox-esr45 --- unaffected
firefox50 --- affected
firefox51 --- verified
firefox52 --- verified

People

(Reporter: user6770522, Assigned: kmag)

Details

(Keywords: dev-doc-complete)

Attachments

(3 files)

Attached file 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.
Assignee: nobody → kmaglione+bmo
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
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?
Attachment #8793306 - Flags: review?(g.maone) → review+
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.
https://hg.mozilla.org/integration/mozilla-inbound/rev/83a0cb757881b0eb3e0339794b22f0b4c6390e30
Bug 1304331: Support setting the Content-Type header from WebRequest observers. r=mao
https://hg.mozilla.org/mozilla-central/rev/83a0cb757881
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Kris, is this a fix you are looking to uplift?
Flags: needinfo?(kmaglione+bmo)
I'm considering it. I want to see how often it's used and how it goes over on nightly first, though.
Flags: needinfo?(kmaglione+bmo)
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.
Keywords: dev-doc-needed
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.
Attachment #8793306 - Flags: approval-mozilla-aurora?
OS: Unspecified → All
Hardware: Unspecified → All
Version: 48 Branch → 47 Branch
Hi Florin, can we have some QA's help to verify this?
Flags: qe-verify+
Flags: needinfo?(florin.mezei)
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.
Attachment #8793306 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
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
Status: RESOLVED → VERIFIED
Flags: needinfo?(florin.mezei)
Attached image Postfix Video
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?
Flags: needinfo?(kmaglione+bmo)
Marking as dev-doc-complete, but let me know if I need anything else here.
I think that probably covers it.
Flags: needinfo?(kmaglione+bmo)
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.