1287264 - add content sniffer (nsIContentSniffer) support to WebExtensions

Status: RESOLVED FIXED

(WebExtensions :: Untriaged, defect)

Description

[kernp25]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:50.0) Gecko/20100101 Firefox/50.0
Build ID: 20160714030208

Comment 1

[kernp25]

I can work on that!

I have built firefox following these instructions:
https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Simple_Firefox_build

Comment 2

[Andy McKay]

Would this be part of webRequest, webNavigation, or something else? We'd like to avoid a direct port of a underlying technology and focus on a high level API, because the underlying technology might change.

Also the ability to sniff a piece of content and return the mimetype feels like it can be done in JavaScript using a library, I bet there's a bunch of libraries for doing this already, could we use one of those?

Comment 3

[kernp25]

Maybe it should work like this:
browser.webRequest.onResponseStarted.addListener(function(details) {
  //let body = details.responseBody; // body will be the converted data using nsScriptableUnicodeConverter::ConvertFromByteArray
  return {
    contentType: "foo/bar" // just an example
  };
}, {urls: ["<all_urls>"]}, ["responseBody"]);

Comment 4

[Caitlin Neiman [:caitmuenster]]

To be discussed at Nov. 29, 2016 WE triage meeting. 

Agenda: https://docs.google.com/document/d/1IMBFXHNpg_A-15VdJM1Hh8DUUXF1xNFy87W1w8ZEOBk/edit?usp=sharing

Comment 5

[Andy McKay]

Can you provide more information on the use case? Is it to be able to determine the content type of a request, or to alter the content type? Could it be duped to bug 1255894?

Comment 6

[kernp25]

(In reply to Andy McKay [:andym] from comment #5)
> Is it to be able to determine the content type of a request
Yes, to determine the content type. Because sometimes the content type headers send by the server are not correct.

Comment 7

[Caitlin Neiman [:caitmuenster]]

Bug to be discussed at Dec. 13, 2016 WE triage meeting. 

Agenda: https://docs.google.com/document/d/1S1QrBK1hrulE7dlLiQzjMupHUUSwDYRYAOXiqtMHe-k/edit

Comment 8

[Luca Greco [:rpl] [:luca] [:lgreco]]

We discussed this issue during the WE triage meeting and from Comment 3 it looks like the real goal is to be able to ready the responseBody in a webRequest.onBeforeRequest listener and to change the "Content-Type" header, which is going to be possible once Bug 1305162 is fixed (actually it should be already possible but only on non-release channels and it is behind a flag, as described by Bug 1201979)

Comment 9

[Luca Greco [:rpl] [:luca] [:lgreco]]

Can you confirm that Bug 1305162 is going to provide what you needed from this issue?

In that case I'm going to tweak the bug summary (to be simpler to identify it as the goal described by Comment 3) and set this as a duplicate of Bug 1305162.

Comment 10

[Andy McKay]

Closing based on comment 8 and 9 and its been a few months since we asked for more info in comment 9. If this is isn't the case, please let me know.

Comment 11

[kernp25]

Thinking about this again (i was waiting for this bug https://bugzilla.mozilla.org/show_bug.cgi?id=1255894)
Because, the browser already does content sniffing, so my above example was bad.

It should look like this now:
// onContentTypeDetermined is this good name?
// or maybe onContentTypeSniffed?
// or which is a good name for this event?
browser.webRequest.onContentTypeDetermined.addListener(function(details) {
  // details.contentType (or details.mimeType? or just details.mime?) will be the type what the browser sniffed (e.g. application/pdf)
  // We should be able to cancel/redirect the request in this event
  // and it should also have the responseHeaders (like with onResponseStarted [3] event).
  // It should also be possible to change response headers in this event
}, {urls: ["<all_urls>"]}, ["blocking", "responseHeaders"]);

This new event gets called only after the built-in content sniffer [1] was called (e.g. nsUnknownDecoder.cpp [2]), that is, if there was no content type response header for this request.

[1] https://dxr.mozilla.org/mozilla-central/search?q=%3A%3AGetMIMETypeFromContent+ext%3Acpp&redirect=false
[2] https://dxr.mozilla.org/mozilla-central/source/netwerk/streamconv/converters/nsUnknownDecoder.cpp#355
[3] https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/webRequest/onResponseStarted

What you think? Is this doable?

Comment 12

[Andy McKay]

Bouncing over to rpl because he had good answers in comment 8 and 9.

Comment 13

[Luca Greco [:rpl] [:luca] [:lgreco]]

Comment 11 seems an additional feature from what Bug 1305162 provided, I'm redirecting the needinfo to Shane to evaluate if it is something that could be doable and reasonable to evaluate (it should probably go through the design-decision-needed if it is going to need a new webRequest event, but I would like to hear Shane opinion first).

Comment 14

[kernp25]

Or what do you think of this idea?

My idea is to add a new API called "contentSniffer" to the  browser.* namespace (e.g. browser.contentSniffer) and add a new event *maybe* called "onMimeTypeSniffed".

It should look like this:
browser.contentSniffer.onMimeTypeSniffed.addListener(function(details) {
  // details.mimeType will be the type what the browser sniffed (e.g. application/pdf)
  // We should be able to cancel/redirect the request in this event
  // and it should also have the responseHeaders (read-only).
}, {urls: ["<all_urls>"]}, ["blocking", "responseHeaders"]);
});

This new event gets called only after the built-in content sniffer [1] was called (e.g. nsUnknownDecoder.cpp [2]), that is, if there was no content type response header for this request.

[1] https://dxr.mozilla.org/mozilla-central/search?q=%3A%3AGetMIMETypeFromContent+ext%3Acpp&redirect=false
[2] https://dxr.mozilla.org/mozilla-central/source/netwerk/streamconv/converters/nsUnknownDecoder.cpp#355

Comment 15

[Shane Caraveo (:mixedpuppy)]

So one thing we're not doing is exposing nsIChannel.contentType.  This is set prior to onResponseStarted[1], and uses the content sniffers to determine the content type, rather than the header.  Given that, I don't think we need any additional event or api, and we should just expose it.

There might still be a general usefulness in having content sniffers available, but without specific use cases[3] that can only be accomplished with access to that, I'm not inclined to add new APIs for it.

So to resolve this bug:

1. Create a new bug to expose nsIChannel.contentType in webRequest, no need for design-decision on that.  That should solve getting reasonably reliable content types from webRequest for the response[2].

2. Flesh out whether there is a real need[3] for content sniffer access in this bug.  


[1] See nsHttpChannel::CallOnStartRequest, HttpBaseChannel::CallTypeSniffers and nsBaseChannel::OnStartRequest
[2] there is some confusion from comments whether we're talking about response type or request type, or both.
[3] comment 6 is not a use case, it's a limitation in the current implementation

Comment 16

[Rob Wu [:robwu]]

The RESOLVED FIXED status of this bug is likely inaccurate (should be WONTFIX?):
- Comment 9 and comment 10 support the decision to close the bug because bug 1305162 was fixed. That bug offers the ability to read the REQUEST body.
- This bug is about content sniffing, which is about the RESPONSE body.
  More specifically, the response body of responses without a specified content type.


Based on comment 14, the use case here is be able to discover when Firefox's content sniffer detects a specific MIME type, and replace (redirect/cancel) the response. For this, there is no real need for accessing the actual content sniffer, it suffices to expose the content type through the webRequest API.

I will file a new bug with more details, since I have encountered actual use cases as an extension developer.

Comment 17

[kernp25]

(In reply to Shane Caraveo (:mixedpuppy) from comment #15)
> 1. Create a new bug to expose nsIChannel.contentType in webRequest, no need
> for design-decision on that.  That should solve getting reasonably reliable
> content types from webRequest for the response[2].

Can you tell me how it would look like?
The nsIChannel.contentType will be available in webRequest.onResponseStarted[1] or webRequest.onHeadersReceived[2]?

[1] https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/webRequest/onResponseStarted
[2] https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/webRequest/onHeadersReceived

Comment 18

[kernp25]

(In reply to Shane Caraveo (:mixedpuppy) from comment #15)
> 2. Flesh out whether there is a real need[3] for content sniffer access in
> this bug.  

I think, a content handler [1] makes more sense here :)

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1356397

Comment 19

[Shane Caraveo (:mixedpuppy)]

(In reply to kernp25 from comment #17)

> Can you tell me how it would look like?
> The nsIChannel.contentType will be available in
> webRequest.onResponseStarted[1] or webRequest.onHeadersReceived[2]?
> 
> [1]
> https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/webRequest/
> onResponseStarted
> [2]
> https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/webRequest/
> onHeadersReceived

It would be on the details object in the listeners, and if it has been set at the time of those calls, then you would be able to access the value.

Really this is just a hint that is available.  Given StreamFilter and an ability to update channel.contentType, I wonder if this would be solved.