New issue
Advanced search Search tips

Issue 772343 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 11
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 3
Type: Bug



Sign in to add a comment

Remove comment handling from media type parser

Project Member Reported by annevank...@gmail.com, Oct 6 2017

Issue description

In particular the scanning for "(" in the media type parser.

Safari does not have this behavior and Edge does not have it for parameter values (and barely for media types). Let's remove it.

See https://github.com/whatwg/mimesniff/issues/33 for further analysis. (I'm planning to refine the media/MIME type parser in that standard to get browsers more closely aligned. Eventually there will be tests too, once I know what I think the details should be like. Feedback appreciated.)
 
Cc: b...@chromium.org
Components: -Internals>Network Internals>Network>HTTP
bnc:  Not sure if you want to tackle this one, too.  Since it's just Chrome and FF (Rather than Chrome and Safari, which might indicate mobile sites depend on this), I think we're probably on pretty safe ground here, too.

Comment 2 by b...@chromium.org, Oct 6 2017

Cc: -b...@chromium.org
Owner: b...@chromium.org
Status: Assigned (was: Unconfirmed)
I can take this one.  It would be nice to see what Firefox developers think about it before making a change though: https://bugzilla.mozilla.org/show_bug.cgi?id=1406337.
Components: Internals>Network
Components: -Internals>Network>HTTP
FWIW, I'm pretty sure Firefox is in support of this given that Firefox already implements the new generic MIME type parser and the bug mentioned above wasn't closed. All that's left is integrating the new generic MIME type parser throughout the code base.
Owner: mmenke@chromium.org
-> mmenke for https://chromium-review.googlesource.com/c/chromium/src/+/1257443
Project Member

Comment 7 by bugdroid1@chromium.org, Oct 5

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5518c1a563526f51674feb28bfedd527e1c94072

commit 5518c1a563526f51674feb28bfedd527e1c94072
Author: Matt Menke <mmenke@chromium.org>
Date: Fri Oct 05 22:03:19 2018

Content-Type parser: Update to more closely match latest WHATWG spec.

The latest spec can be found at https://mimesniff.spec.whatwg.org/

In particular, this CL makes the parser:
* Trim whitespace within quotes.
* Not ignore first "(" in a charset or anything after it.
* Allow quoted empty strings.
* Prefer values closed to the beginning of the string.

This makes a number of previously failing WPT layout tests pass,
though there still remain some deviations from the spec:
* When there are multiple Content-Type lines, we use the last.
* We don't restrict the characters we allow in Content-Type fields.
* We trim whitespace before the first quote characters (The spec
  indicates that if there's a space after the "=", it's treated as a
  non-quoted string).

Bug:  788491 ,  772343 
Change-Id: Ieeb11eb76d824a9df5130d8c5dc0be3b442ca366
Reviewed-on: https://chromium-review.googlesource.com/c/1257443
Commit-Queue: Matt Menke <mmenke@chromium.org>
Reviewed-by: Asanka Herath <asanka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#597331}
[modify] https://crrev.com/5518c1a563526f51674feb28bfedd527e1c94072/net/http/http_response_headers_unittest.cc
[modify] https://crrev.com/5518c1a563526f51674feb28bfedd527e1c94072/net/http/http_util.cc
[modify] https://crrev.com/5518c1a563526f51674feb28bfedd527e1c94072/net/http/http_util_unittest.cc
[modify] https://crrev.com/5518c1a563526f51674feb28bfedd527e1c94072/third_party/WebKit/LayoutTests/external/wpt/mimesniff/mime-types/charset-parameter.window-expected.txt

Status: Fixed (was: Assigned)

Sign in to add a comment