560392 - net_ParseContentType accepts comma-separated list of content types

Status: NEW

(Core :: Networking, defect, P5)

Description

[Zack Weinberg (:zwol)]

net_ParseContentType claims that the syntax of the Content-Type header is

   header-value = media-type *( LWS "," LWS media-type )
   media-type   = type "/" subtype *( LWS ";" LWS parameter )

but neither RFC 2045 (core MIME; <http://www.faqs.org/rfcs/rfc2045.html> -- see section 5.1] nor RFC 2616 (HTTP 1.1; <http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.17>) permit more than one media-type to appear in a content-type header.

If we do get a comma-separated list on the wire, the parser appears to use the last base type and subtype, and take the last well-formed charset parameter it can find; this behavior frankly doesn't make a whole lot of sense to me.

I suggest we throw away the code currently in net_ParseContentType and have it just do what net_ParseMediaType currently does.

Comment 1

[Boris Zbarsky [:bzbarsky]]

I want to say that we have existing bugs on this, but in any case...

Doing what you suggest would affect not just comma-separated types but also responses that specify multiple Content-Type headers (which we internally convert to the equivalent comma-separated version).  The spec on what to do with that case is in flux, so we've sort of been avoiding messing with this code until that gets sorted out.

Comment 2

[Zack Weinberg (:zwol)]

(In reply to comment #1)
> I want to say that we have existing bugs on this, but in any case...

I did look, but didn't find any.

> Doing what you suggest would affect not just comma-separated types but also
> responses that specify multiple Content-Type headers (which we internally
> convert to the equivalent comma-separated version).  The spec on what to do
> with that case is in flux, so we've sort of been avoiding messing with this
> code until that gets sorted out.

... I don't see how that's a useful thing to allow any more than comma-separated lists are?  Although I can see how that might happen more often in real life, and if we have to do some specific thing for web compat, I guess I can hold my nose.  Still, couldn't we internally pass nsTArray<nsString> around?

Which spec should I be looking at for this stuff?  Is there a new HTTP draft in the works?

Comment 3

[Boris Zbarsky [:bzbarsky]]

> ... I don't see how that's a useful thing to allow 

Sites do it.  The only question is whether the first header or the last one should be used in that situation.

> Still, couldn't we internally pass nsTArray<nsString> around

We could rewrite how HTTP headers work in general, sure...  (well, except for all the frozen interfaces involved).

> Is there a new HTTP draft in the works?

http://datatracker.ietf.org/wg/httpbis/charter/ has the info on that.  I'm not sure whether the current verbiage is in there or in HTML5, nor whether it's gotten to the point where different browsers agree on it...

Comment 4

[Zack Weinberg (:zwol)]

http://tools.ietf.org/html/draft-abarth-mime-sniff-04 seems to be the current draft verbiage.  It doesn't say anything about cross-browser agreement, though, it just says "you do this".  Where, in this case, "this" is "the last header wins".

That *appears* to be what we do, although the algorithm is messy enough that I don't want to swear that we never pick up data from earlier headers (in particular, I'm not sure what would happen with

Content-Type: text/plain; charset=iso-8859-1
Content-Type: text/html

)

Comment 5

[Boris Zbarsky [:bzbarsky]]

Yeah; I have no problem with changing this (quite messy) code to mske it clearer and align with whatever the end thing ends up that's decided on.  I just don't know that we need to thrash it until we're ready to rewrite it for good.

Comment 6

[Firefox Bug Husbandry Bot]

Bulk change to priority: https://bugzilla.mozilla.org/show_bug.cgi?id=1399258

Comment 7

[Anne (:annevk)]

FWIW, I'm sorting out parsing of Content-Type over in https://github.com/whatwg/mimesniff/issues/30. Feedback welcome.