Closed
Bug 1230353
Opened 8 years ago
Closed 8 years ago
MIME type and subtype should be case-insensitive
Categories
(Core :: Audio/Video: Playback, defect)
Core
Audio/Video: Playback
Tracking
()
RESOLVED
FIXED
mozilla45
Tracking | Status | |
---|---|---|
firefox45 | --- | fixed |
People
(Reporter: jya, Assigned: jya)
References
Details
Attachments
(3 files)
8.38 KB,
patch
|
bzbarsky
:
review-
|
Details | Diff | Splinter Review |
1.83 KB,
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
1.23 KB,
patch
|
jya
:
review+
|
Details | Diff | Splinter Review |
Forking from bug 1191833. Per HTML5 spec: https://html.spec.whatwg.org/multipage/infrastructure.html#mime-type "A string is a valid MIME type if it matches the media-type rule defined in section 3.1.1.1 "Media Type" of RFC 7231. In particular, a valid MIME type may include MIME type parameters. [HTTP]" Per RFC 2616 section 3.7, which states "The type, subtype, and parameter attribute names are case-insensitive". We should ensure that all tests, search and comparison of mimetype should be case insensitive. Alternatively, ensure it's always handled as a lowercase string
Assignee | ||
Comment 1•8 years ago
|
||
As per RFC 2616 section 3.7, which states "The type, subtype, and parameter attribute names are case-insensitive". So ensure the type and subtype are always lower-case as all our comparisons assume that they are. This version limits the change to nsContentTypeParser::GetType() which is exclusively used to request mimetype as defined by RFC 2616 due to the caller context. The reason I took this approach is because I found that working on the nsAString directly was far more cumbersome, and would be greatly slower than working on the low level C-string. Not that this routine is used in critical loops as fas as I can tell. It also helps abstract the retrieval of the MIME's type in one place.
Attachment #8695624 -
Flags: review?(bzbarsky)
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → jyavenard
Assignee | ||
Comment 2•8 years ago
|
||
Note I purposefully left the trailing space issued from copy/paste ; as I didn't want to get started cleaning those up (there are plenty everywhere)
Comment 3•8 years ago
|
||
Comment on attachment 8695624 [details] [diff] [review] Ensure type and subtype of MIME are lowercase. > The reason I took this approach is because I found that working on the > nsAString directly was far more cumbersome I'm not sure I follow. Is this: nsresult rv = GetParameter(nullptr, aResult); NS_ENSURE_SUCCESS(rv, rv); nsContentUtils::ASCIIToLower(aResult); return NS_OK; more cumbersome than adding new IDL API surface and so forth? But in any case, the API surface being added doesn't make sense: getMIMEType doesn't make sense for arbitrary MIME headers, which are what nsIMIMEHeaderParam works with. Please just handle this inside nsContentTypeParser.
Attachment #8695624 -
Flags: review?(bzbarsky) → review-
Assignee | ||
Comment 4•8 years ago
|
||
In French we have a say: "Pourquoi faire simple quand on peut faire compliqué" getMIMEType was only used by nsContentTypeParser, but yeah, I guess should I had known about ASCIIToLower it would have affected my decision :)
Comment 5•8 years ago
|
||
> In French we have a say: "Pourquoi faire simple quand on peut faire compliqué"
Hey, it's the story of web browsers and the web in a nutshell. ;)
Assignee | ||
Comment 6•8 years ago
|
||
As per RFC 2616 section 3.7, which states "The type, subtype, and parameter attribute names are case-insensitive". So ensure the type and subtype are always lower-case as all our comparisons assume that they are. I was trying to stay in the long tradition of the web :)
Attachment #8695683 -
Flags: review?(bzbarsky)
Assignee | ||
Comment 7•8 years ago
|
||
type and subtype in media mimetype are case-insensitive Carrying r+ from bug 1191833
Attachment #8695685 -
Flags: review+
Comment 8•8 years ago
|
||
Comment on attachment 8695683 [details] [diff] [review] P1. Ensure type and subtype of MIME are lowercase. r=me. Thanks for bearing with me here, and I'm sorry you ever had to spelunk into the mime header param code...
Attachment #8695683 -
Flags: review?(bzbarsky) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/042dff507cdc https://hg.mozilla.org/integration/mozilla-inbound/rev/f9246b7ca284
Assignee | ||
Comment 10•8 years ago
|
||
(In reply to Boris Zbarsky [:bz] from comment #8) > Comment on attachment 8695683 [details] [diff] [review] > P1. Ensure type and subtype of MIME are lowercase. > > r=me. Thanks for bearing with me here, and I'm sorry you ever had to > spelunk into the mime header param code... no worries. you were 100% right, it was the best place to do it...
Comment 11•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/042dff507cdc https://hg.mozilla.org/mozilla-central/rev/f9246b7ca284
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
You need to log in
before you can comment on or make changes to this bug.
Description
•