790524 - The sniffing logic can overwrite sniffed content-type by application/octet-stream

Status: RESOLVED FIXED

(Core :: Networking: HTTP, defect)

Description

[Paul Adenot (:padenot)]

If we successfully sniff a file in [1], but we set a nsUnknownDecoder as mListener in [2], we will end up overwriting what we sniffed when OnDataAvailable for the nsUnknownDecoder is called, because the nsUnknownDecoder does not check if we already have a content-type in the request, but only in its mContentType.

[1]: http://mxr.mozilla.org/mozilla-central/source/netwerk/protocol/http/nsHttpChannel.cpp
[2]: http://mxr.mozilla.org/mozilla-central/source/netwerk/streamconv/converters/nsUnknownDecoder.cpp#305

Comment 1

[Boris Zbarsky [:bzbarsky]]

And the point is that we should be setting up the unknown decoder, if we do it, _after_ calling content sniffers, I think.

Christian, Jason, does that make sense?

Comment 2

[Jason Duell]

I'm probably missing something here, but 

   http://mxr.mozilla.org/mozilla-central/source/netwerk/protocol/http/nsHttpChannel.cpp#885

seems to set mContentType when the sniffers are run.  So why isn't nsUnknownDecoder::DetermineContentType failing or returning right away?

  http://mxr.mozilla.org/mozilla-central/source/netwerk/streamconv/converters/nsUnknownDecoder.cpp#307

But that's just from looking at this briefly, without an actual test case to follow.  An xpcshell test or some other steps to repro would be nice.

Comment 3

[Boris Zbarsky [:bzbarsky]]

> So why isn't nsUnknownDecoder::DetermineContentType failing or returning right away?

nsUnknownDecoder is set up as a stream converter in this case, not a content sniffer.  See the beginning of nsHttpChannel::CallOnStartRequest.

Comment 4

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

hm, that sounds reasonable

Comment 5

[Paul Adenot (:padenot)]

Attachment: Avoid overwriting the content-type by application/octet-stream in the sniffing logic. r=

This is what comment 1 describes and is green on try.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 682543 [details] [diff] [review]
Avoid overwriting the content-type by application/octet-stream in the sniffing logic. r=

Perfect, thanks!

Comment 7

[Paul Adenot (:padenot)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/25831c0daacd

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/25831c0daacd