273306 - nsHttpChannel overriding contentType doesn't work as documented (apparent regression)

Status: RESOLVED FIXED

(Core Graveyard :: File Handling, defect, P1)

Description

[Raefer Gabriel]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.5) Gecko/20041107 Firefox/1.0 (ax)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.5) Gecko/20041107 Firefox/1.0 (ax)

I have been working on porting and cleaning up the ForceContentType tool for
Mozilla to work with Firefox 1.0 as an extension, and I got everything
installing and attaching to an nsHttpChannel and receiving
http-on-examine-response events just fine, but overriding the contentType works
only for certain original content types.  If a server sends text/html or several
other well-defined Content Types, I can successfully override it with the
setting of my choice, but if the server sends text/plain (and possibly some
other types), my override doesn't take effect (it seems to get modified again
before the page is displayed.

According to the documentation on nsIChannel::contentType, what I am doing seems
to be permissible.  Christian Biesinger has suggested this is likely a
regression caused by some of the hacks implemented to deal with <a
href="https://bugzilla.mozilla.org/show_bug.cgi?id=220807">Bug 220807</a>. 
Unfortunately, these are exactly the cases (text/plain content types) where it
is most useful to be able to manually override the content type, thus this bug
essentially renders my extension useless.

Any workaround suggestions would also be greatly appreciated to force rendering
of a page with a certain MIME type, perhaps a way to inject the correct Content
Type into a page load earlier/later in the process I can override it some way. 
This bug is quite significant because it means we have sacrificed any
flexibility or ability for extension writers to empower users with alternative
ways to handle misconfigured server Content Types in favor of a few Content Type
hacks that work somewhat unreliably much of the time (if they worked perfectly,
people wouldn't be asking for an extension like this all the time on the
MozillaZine forums and I wouldn't be writing this in the first place).

Reproducible: Always
Steps to Reproduce:
1. Attach an observer to http-on-examine response events.
2. When page is of content type plain/text, try to change
nsIChannel::contentType variable.

Actual Results:  
Content type of loaded page is still text/plain, displays in browser as text.

Expected Results:  
Behaved as if page was of the Content-Type that I set it to (either handled with
appopriate plugin, displayed in browser as HTML for text/html etc.)

Comment 1

[Boris Zbarsky [:bzbarsky]]

> Any workaround suggestions would also be greatly appreciated 

The simplest is probably to call setResponseHeader() on the content-type header
and change it to the type you want, before setting contentType.  You may want to
save the contentCharset first and reset it after calling setResponseHeader....

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: Patch, sorta

This fixes the case described in comment 0.  There are two remaining problems:

1)  When a content-type hint is set to "text/plain" and then the server
delivers
    a text/plain response, we will sniff for the "real" type.
2)  When someone forces contentType to text/plain (eg in an extension such as
    Raefer's), we will still sniff for the "real" type.

I don't really see a good way to deal with these short of either moving this
code to necko or introducing more apis to enable the URILoader to know more
about the state of the channel.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Nevermind, I can tell that you can't change the Content-Type header via
setResponseHeader....

In which case, I'm not sure there's a simple workaround here....  The other
thing you could do is to register your own object for the
"@mozilla.org/streamconv;1?from=application/x-vnd.mozilla.maybe-text&to=*/*"
contractid... It'd need to implement the nsIStreamConverter (and
nsIStreamListener) interface.  It'd mostly be a no-op -- it would hold a ref to
the aListener passed to asyncConvertData and would forward all nsIStreamListener
calls to it.  The nsIStreamConverter::convert() method is not really used and
wouldn't need to be implenented.

That would basically override the existing content sniffer that overrides the
content type on the channel with an implementation that doesn't change the
channel at all.

Comment 4

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 167977 [details] [diff] [review]
Patch, sorta

for that problem, we could maybe introduce a text/x-real-plain or something...
hm... that may not be worth it...

Comment 5

[Raefer Gabriel]

Boris, thanks for the suggestion, I'll see if I'm able to get the desired effect
with an nsIStreamConverter and nsIStreamListener (oh boy, more APIs to learn!).

I just have to figure out how to pass off pages that don't have overrides set to
the current content-sniffing code, which I don't want to break if possible.  It
looks like currently an instance of nsBinaryDetector (which extends
nsUnknownDecoder) sits on that channel - hopefully I can figure out how to
instantiate an object of that type myself and pass non-overridden requests to it.

What's strange is that you'd think with all that HTML sniffing code that I see
in nsUnknownDecoder.cpp I wouldn't still be writing this extension, but that
HTML sniffing code just doesn't always seem to work and I still see posts every
few days on MozillaZine with sites that are broken due to sending text/plain as
their content type (perhaps due to other issues with this sniffing code that are
out of scope in this bug).

Thanks again for the swift response on this bug.

Comment 6

[Boris Zbarsky [:bzbarsky]]

You should be able to instantiate an nsBinaryDetector by CID, even if you've
overridden its contractid.  See
http://www.mozilla.org/scriptable/components_object.html#_classesByID and the
definition of the nsBinaryDetector classid at
http://lxr.mozilla.org/seamonkey/source/netwerk/streamconv/converters/nsUnknownDecoder.h#141

As for nsUnknownDecoder, applying it to all incoming requests would be a major
violation of the HTTP specification... we apply it to HTTP/0.9 requests (no
Content-Type header) and to local files and the like.

Comment 7

[Christian :Biesinger (don't email me, ping me on IRC)]

we don't usually apply it to local files... see
http://www.mozilla.org/docs/web-developer/mimetypes.html#file ;)
(hm... that's incomplete... if EHAS doesn't give us a type, we do use the
unknowndecoder, iirc)

Comment 8

[Boris Zbarsky [:bzbarsky]]

Fixed on trunk for 1.8a6.