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)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla45
Tracking Status
firefox45 --- fixed

People

(Reporter: jya, Assigned: jya)

References

Details

Attachments

(3 files)

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
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: nobody → jyavenard
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 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-

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 :)
> 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.  ;)
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)
type and subtype in media mimetype are case-insensitive

Carrying r+ from bug 1191833
Attachment #8695685 - Flags: review+
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+
(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...
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.