Make CORB MIME type classification consistent with the web specs.

https://tools.ietf.org/html/rfc7303 says that if "new media type is
introduced for an XML-based format, the name of the media type SHOULD
end with '+xml'".

https://tools.ietf.org/html/rfc6839 covers '+xml' and '+json' suffixes.

https://mimesniff.spec.whatwg.org/#xml-mime-type says "An XML MIME type
is any MIME type whose subtype ends in '+xml' or whose essence is
'text/xml' or 'application/xml'. [RFC7303]".

https://mimesniff.spec.whatwg.org/#json-mime-type says "A JSON MIME type
is any MIME type whose subtype ends in '+json' or whose essence is
'application/json' or 'text/json'."

There are no occurences of "application/xml+", "text/xml+",
"application/json+", "text/json+" or "text/x-json" in the specs above
and on various lists of MIME types like:
-
https://developer.mozilla.org/en-US/docs/Web/HTTP/Basics_of_HTTP/MIME_types/Complete_list_of_MIME_types
- https://en.wikipedia.org/wiki/Media_type
- https://www.freeformatter.com/mime-types-list.html
- https://www.sitepoint.com/mime-types-complete-list/

Bug: 826756
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: Ied30f9728bd4f082bb620fea150f342457ea4833
Reviewed-on: https://chromium-review.googlesource.com/985211
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: Nick Carter <nick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547565}

content/browser/loader/cross_site_document_resource_handler_unittest.cc

@@ -564,7 +564,7 @@ const TestScenario kScenarios[] = {
         RESOURCE_TYPE_XHR,                      // resource_type
         "http://www.a.com/",                    // initiator_origin
         OriginHeader::kOmit,                    // cors_request
-        "text/x-json",                          // response_mime_type
+        "text/json",                            // response_mime_type
         MimeType::kJson,                        // canonical_mime_type
         false,                                  // include_no_sniff_header
         false,                                  // simulate_range_response

services/network/cross_origin_read_blocking.cc

@@ -31,8 +31,10 @@ const char kAppXml[] = "application/xml";
 const char kAppJson[] = "application/json";
 const char kImageSvg[] = "image/svg+xml";
 const char kTextJson[] = "text/json";
-const char kTextXjson[] = "text/x-json";
 const char kTextPlain[] = "text/plain";
+// TODO(lukasza): Remove kJsonProtobuf once this MIME type is not used in
+// practice.  See also https://crbug.com/826756#c3
+const char kJsonProtobuf[] = "application/json+protobuf";
 
 // MIME type suffixes
 const char kJsonSuffix[] = "+json";
@@ -77,39 +79,6 @@ CrossOriginReadBlocking::SniffingResult MatchesSignature(
   return CrossOriginReadBlocking::kNo;
 }
 
-// Returns true if |mime_type == prefix| or if |mime_type| starts with
-// |prefix + '+'|.  Returns false otherwise.
-//
-// For example:
-// - MatchesMimeTypePrefix("application/json", "application/json") -> true
-// - MatchesMimeTypePrefix("application/json+foo", "application/json") -> true
-// - MatchesMimeTypePrefix("application/jsonp", "application/json") -> false
-// - MatchesMimeTypePrefix("application/foo", "application/json") -> false
-bool MatchesMimeTypePrefix(base::StringPiece mime_type,
-                           base::StringPiece prefix) {
-  constexpr auto kCaseInsensitive = base::CompareCase::INSENSITIVE_ASCII;
-  if (!base::StartsWith(mime_type, prefix, kCaseInsensitive))
-    return false;
-  DCHECK_GE(mime_type.length(), prefix.length());
-
-  if (mime_type.length() == prefix.length()) {
-    // Given StartsWith results above, the above condition is our O(1) check if
-    // |base::LowerCaseEqualsASCII(mime_type, prefix)|.
-    DCHECK(base::LowerCaseEqualsASCII(mime_type, prefix));
-    return true;
-  }
-
-  if (mime_type[prefix.length()] == '+') {
-    // Given StartsWith results above, the above condition is our O(1) check if
-    // |base::StartsWith(mime_type, prefix + '+', kCaseInsensitive)|.
-    DCHECK(base::StartsWith(mime_type, prefix.as_string() + '+',
-                            kCaseInsensitive));
-    return true;
-  }
-
-  return false;
-}
-
 }  // namespace
 
 CrossOriginReadBlocking::MimeType CrossOriginReadBlocking::GetCanonicalMimeType(
@@ -120,29 +89,29 @@ CrossOriginReadBlocking::MimeType CrossOriginReadBlocking::GetCanonicalMimeType(
   if (base::LowerCaseEqualsASCII(mime_type, kImageSvg))
     return CrossOriginReadBlocking::MimeType::kOthers;
 
+  // See also https://mimesniff.spec.whatwg.org/#html-mime-type
   if (base::LowerCaseEqualsASCII(mime_type, kTextHtml))
     return CrossOriginReadBlocking::MimeType::kHtml;
 
-  if (base::LowerCaseEqualsASCII(mime_type, kTextPlain))
-    return CrossOriginReadBlocking::MimeType::kPlain;
-
-  // StartsWith rather than LowerCaseEqualsASCII is used to account both for
-  // mime types similar to 1) application/json and to 2)
-  // application/json+protobuf.
+  // See also https://mimesniff.spec.whatwg.org/#json-mime-type
   constexpr auto kCaseInsensitive = base::CompareCase::INSENSITIVE_ASCII;
-  if (MatchesMimeTypePrefix(mime_type, kAppJson) ||
-      MatchesMimeTypePrefix(mime_type, kTextJson) ||
-      MatchesMimeTypePrefix(mime_type, kTextXjson) ||
+  if (base::LowerCaseEqualsASCII(mime_type, kAppJson) ||
+      base::LowerCaseEqualsASCII(mime_type, kTextJson) ||
+      base::LowerCaseEqualsASCII(mime_type, kJsonProtobuf) ||
       base::EndsWith(mime_type, kJsonSuffix, kCaseInsensitive)) {
     return CrossOriginReadBlocking::MimeType::kJson;
   }
 
-  if (MatchesMimeTypePrefix(mime_type, kAppXml) ||
-      MatchesMimeTypePrefix(mime_type, kTextXml) ||
+  // See also https://mimesniff.spec.whatwg.org/#xml-mime-type
+  if (base::LowerCaseEqualsASCII(mime_type, kAppXml) ||
+      base::LowerCaseEqualsASCII(mime_type, kTextXml) ||
       base::EndsWith(mime_type, kXmlSuffix, kCaseInsensitive)) {
     return CrossOriginReadBlocking::MimeType::kXml;
   }
 
+  if (base::LowerCaseEqualsASCII(mime_type, kTextPlain))
+    return CrossOriginReadBlocking::MimeType::kPlain;
+
   return CrossOriginReadBlocking::MimeType::kOthers;
 }
 

services/network/cross_origin_read_blocking_unittest.cc

@@ -176,15 +176,12 @@ TEST(CrossOriginReadBlockingTest, GetCanonicalMimeType) {
       {"application/xml", MimeType::kXml},
       {"application/json", MimeType::kJson},
       {"text/json", MimeType::kJson},
-      {"text/x-json", MimeType::kJson},
       {"text/plain", MimeType::kPlain},
 
       // Other mime types:
       {"application/foobar", MimeType::kOthers},
 
       // Regression tests for https://crbug.com/799155 (prefix/suffix matching):
-      {"application/json+protobuf", MimeType::kJson},
-      {"text/json+protobuf", MimeType::kJson},
       {"application/activity+json", MimeType::kJson},
       {"text/foobar+xml", MimeType::kXml},
       // No match without a '+' character:
@@ -195,7 +192,6 @@ TEST(CrossOriginReadBlockingTest, GetCanonicalMimeType) {
 
       // Case-insensitive comparison:
       {"APPLICATION/JSON", MimeType::kJson},
-      {"APPLICATION/JSON+PROTOBUF", MimeType::kJson},
       {"APPLICATION/ACTIVITY+JSON", MimeType::kJson},
 
       // Images are allowed cross-site, and SVG is an image, so we should
@@ -206,6 +202,20 @@ TEST(CrossOriginReadBlockingTest, GetCanonicalMimeType) {
       // Javascript should not be blocked.
       {"application/javascript", MimeType::kOthers},
       {"application/jsonp", MimeType::kOthers},
+
+      // TODO(lukasza): Remove in the future, once this MIME type is not used in
+      // practice.  See also https://crbug.com/826756#c3
+      {"application/json+protobuf", MimeType::kJson},
+      {"APPLICATION/JSON+PROTOBUF", MimeType::kJson},
+
+      // According to specs, these types are not XML or JSON.  See also:
+      // - https://mimesniff.spec.whatwg.org/#xml-mime-type
+      // - https://mimesniff.spec.whatwg.org/#json-mime-type
+      {"text/x-json", MimeType::kOthers},
+      {"text/json+blah", MimeType::kOthers},
+      {"application/json+blah", MimeType::kOthers},
+      {"text/xml+blah", MimeType::kOthers},
+      {"application/xml+blah", MimeType::kOthers},
   };
 
   for (const auto& test : tests) {

third_party/WebKit/LayoutTests/FlagExpectations/site-per-process

@@ -3,6 +3,7 @@
 
 # TODO(lukasza): Remove these once CORB is enabled by default.
 crbug.com/802835 external/wpt/fetch/corb/img-png-mislabeled-as-html-nosniff.tentative.sub.html [ Pass ]
+crbug.com/802835 external/wpt/fetch/corb/img-mime-types-coverage.tentative.sub.html [ Pass ]
 crbug.com/802835 external/wpt/fetch/corb/script-html-correctly-labeled.tentative.sub.html [ Pass ]
 crbug.com/802835 external/wpt/fetch/corb/preload-image-png-mislabeled-as-html-nosniff.tentative.sub.html [ Pass ]
 

third_party/WebKit/LayoutTests/TestExpectations

@@ -3242,6 +3242,8 @@ crbug.com/802896 virtual/gpu/fast/canvas/canvas-ellipse-circumference-fill.html
 # TODO(lukasza): Remove these once CORB is enabled by default.
 crbug.com/802835 external/wpt/fetch/corb/img-png-mislabeled-as-html-nosniff.tentative.sub.html [ Failure ]
 crbug.com/802835 virtual/outofblink-cors/external/wpt/fetch/corb/img-png-mislabeled-as-html-nosniff.tentative.sub.html [ Failure ]
+crbug.com/802835 external/wpt/fetch/corb/img-mime-types-coverage.tentative.sub.html [ Failure ]
+crbug.com/802835 virtual/outofblink-cors/external/wpt/fetch/corb/img-mime-types-coverage.tentative.sub.html [ Failure ]
 crbug.com/802835 external/wpt/fetch/corb/script-html-correctly-labeled.tentative.sub.html [ Failure ]
 crbug.com/802835 virtual/outofblink-cors/external/wpt/fetch/corb/script-html-correctly-labeled.tentative.sub.html [ Failure ]
 crbug.com/802835 external/wpt/fetch/corb/preload-image-png-mislabeled-as-html-nosniff.tentative.sub.html [ Failure ]

third_party/WebKit/LayoutTests/external/wpt/fetch/corb/img-mime-types-coverage.tentative.sub.html

+<!-- Test verifies that cross-origin, nosniff images are 1) blocked when their
+  MIME type is covered by CORB and 2) allowed otherwise.
+
+  This test is very similar to fetch/nosniff/images.html, except that
+  1) it deals with cross-origin images (CORB ignores same-origin fetches),
+  2) it focuses on MIME types relevant to CORB.
+  There are opportunities to unify the test here with nosniff tests *if*
+  we can also start blocking same-origin (or cors-allowed) images.  We
+  should try to gather data to quantify the impact of such change.
+-->
+<script src=/resources/testharness.js></script>
+<script src=/resources/testharnessreport.js></script>
+<div id=log></div>
+<script>
+  var passes = [
+      // Empty or non-sensical MIME types
+      null, "", "x", "x/x",
+
+      // MIME-types not protected by CORB
+      "image/gif", "image/png", "image/png;blah", "image/svg+xml",
+      "application/javascript", "application/jsonp",
+
+      // MIME types that may seem to be JSON or XML, but really aren't - i.e.
+      // these MIME types are not covered by:
+      // - https://mimesniff.spec.whatwg.org/#json-mime-type
+      // - https://mimesniff.spec.whatwg.org/#xml-mime-type
+      // - https://tools.ietf.org/html/rfc6839
+      // - https://tools.ietf.org/html/rfc7303
+      "text/x-json", "text/json+blah", "application/json+blah",
+      "text/xml+blah", "application/xml+blah",
+      "application/blahjson", "text/blahxml",
+
+  var fails = [
+      // CORB-protected MIME-types - i.e. ones covered by:
+      // - https://mimesniff.spec.whatwg.org/#html-mime-type
+      // - https://mimesniff.spec.whatwg.org/#json-mime-type
+      // - https://mimesniff.spec.whatwg.org/#xml-mime-type
+      "text/html",
+      "text/json", "application/json", "text/xml", "application/xml",
+      "application/blah+json", "text/blah+json",
+      "application/blah+xml", "text/blah+xml",
+      "TEXT/HTML", "TEXT/JSON", "TEXT/BLAH+JSON", "APPLICATION/BLAH+XML"]
+
+  const get_url = (mime) => {
+    // www1 is cross-origin, so the HTTP response is CORB-eligible -->
+    url = "http://{{domains[www1]}}:{{ports[http][0]}}"
+    url = url + "/fetch/nosniff/resources/image.py"
+    if (mime != null) {
+        url += "?type=" + encodeURIComponent(mime)
+    }
+    return url
+  }
+
+  passes.forEach(function(mime) {
+    async_test(function(t) {
+      var img = document.createElement("img")
+      img.onerror = t.unreached_func("Unexpected error event")
+      img.onload = t.step_func_done(function(){
+        assert_equals(img.width, 96)
+      })
+      img.src = get_url(mime)
+      document.body.appendChild(img)
+    }, "CORB should allow the response if Content-Type is: '" + mime + "'.  ")
+  })
+
+  fails.forEach(function(mime) {
+    async_test(function(t) {
+      var img = document.createElement("img")
+      img.onerror = t.step_func_done()
+      img.onload = t.unreached_func("Unexpected load event")
+      img.src = get_url(mime)
+      document.body.appendChild(img)
+    }, "CORB should block the response if Content-Type is: '" + mime + "'.  ")
+  })
+</script>

third_party/WebKit/LayoutTests/external/wpt/fetch/nosniff/image.html

@@ -3,7 +3,19 @@
 <div id=log></div>
 <script>
   // Note: images get always sniffed, nosniff doesn't do anything
-  var passes = [null, "", "x", "x/x", "image/gif", "image/png", "image/png;blah"]
+  // (but note the tentative Cross-Origin Read Blocking (CORB) tests
+  // - for example wpt/fetch/corb/img-mime-types-coverage.tentative.sub.html).
+  var passes = [
+      // Empty or non-sensical MIME types
+      null, "", "x", "x/x",
+
+      // Image MIME types
+      "image/gif", "image/png", "image/png;blah", "image/svg+xml",
+
+      // CORB-protected MIME types (but note that CORB doesn't apply here,
+      // because CORB ignores same-origin requests).
+      "text/html", "application/xml", "application/blah+xml"
+  ]
 
   const get_url = (mime) => {
     let url = "resources/image.py"

third_party/WebKit/LayoutTests/external/wpt/fetch/nosniff/resources/image.py

@@ -3,7 +3,13 @@ import os.path
 def main(request, response):
     type = request.GET.first("type", None)
 
-    body = open(os.path.join(os.path.dirname(__file__), "../../../images/blue96x96.png"), "rb").read()
+    if type != None and "svg" in type:
+      filename = "green-96x96.svg"
+    else:
+      filename = "blue96x96.png"
+
+    path = os.path.join(os.path.dirname(__file__), "../../../images", filename)
+    body = open(path, "rb").read()
 
     response.add_required_headers = False
     response.writer.write_status(200)

third_party/WebKit/LayoutTests/external/wpt/images/green-96x96.svg

+<svg xmlns="http://www.w3.org/2000/svg" width="96" height="96">
+  <rect fill="lime" width="96" height="96"/>
+</svg>