[Regression fix] [Data URI parser] Accept data URI with invalid mediatype data

tyoshino · Commit bot · commit 5469386eaa6b · 2014-10-01T04:01:46.000Z
r272022 introduced grammar check on the mediatype part in a data URI. This broke applications that depend on the behavior that our WebURLLoaderImpl and URLRequestDataJob accept data URIs with incomplete mediatype such as "data:image;base64,xxx". Accept this kind of data URI again for backward compatibility. Instead, fallback to the default mediatype when ParseMimeTypeWithoutParameter() returns false in DataURL::Parse() to avoid generating Content-type header with invalid mediatype. BUG=412479 Review URL: https://codereview.chromium.org/555383003 Cr-Commit-Position: refs/heads/master@{#297593}
diff --git a/net/base/data_url.cc b/net/base/data_url.cc
@@ -66,10 +66,16 @@ bool DataURL::Parse(const GURL& url, std::string* mime_type,
   }
 
   if (mime_type->empty()) {
-    // fallback to defaults if nothing specified in the URL:
+    // Fallback to the default if nothing specified in the mediatype part as
+    // specified in RFC2045. As specified in RFC2397, we use |charset| even if
+    // |mime_type| is empty.
     mime_type->assign("text/plain");
   } else if (!ParseMimeTypeWithoutParameter(*mime_type, NULL, NULL)) {
-    return false;
+    // Fallback to the default as recommended in RFC2045 when the mediatype
+    // value is invalid. For this case, we don't respect |charset| but force it
+    // set to "US-ASCII".
+    mime_type->assign("text/plain");
+    charset->assign("US-ASCII");
   }
   if (charset->empty())
     charset->assign("US-ASCII");
diff --git a/net/base/data_url.h b/net/base/data_url.h
@@ -35,8 +35,23 @@ class NET_EXPORT DataURL {
   // decoded data (e.g.., if the data URL specifies base64 encoding, then the
   // returned data is base64 decoded, and any %-escaped bytes are unescaped).
   //
-  // If the URL is malformed, then this method will return false, and its
-  // output variables will remain unchanged.  On success, true is returned.
+  // If the media type value doesn't match the media-type production defined in
+  // RFC 7231, mime_type will be set to the default value "text/plain". We
+  // don't simply fail for this grammar violation since Chromium had been
+  // accepting such invalid values. For example, <img> element with the src
+  // attribute set to a data URL with an invalid media type "image" (without a
+  // slash and subtype) had been displayed. However, the value this method will
+  // store in mime_type argument can be used for generating other headers, etc.
+  // This could lead to security vulnerability. We don't want to accept
+  // arbitrary value and ask each caller to validate the return value.
+  //
+  // If the charset parameter is specified but its value doesn't match the
+  // token production defined in RFC 7230, this method simply fails and returns
+  // false.
+  //
+  // If there's any other grammar violation in the URL, then this method will
+  // return false. Output variables may be changed and contain invalid data. On
+  // success, true is returned.
   //
   // OPTIONAL: If |data| is NULL, then the <data> section will not be parsed
   //           or validated.
diff --git a/net/base/data_url_unittest.cc b/net/base/data_url_unittest.cc
@@ -63,6 +63,28 @@ TEST(DataURLTest, Parse) {
       "US-ASCII",
       "hello world" },
 
+    // Allow invalid mediatype for backward compatibility but set mime_type to
+    // "text/plain" instead of the invalid mediatype.
+    { "data:foo,boo",
+      true,
+      "text/plain",
+      "US-ASCII",
+      "boo" },
+
+    // When accepting an invalid mediatype, override charset with "US-ASCII"
+    { "data:foo;charset=UTF-8,boo",
+      true,
+      "text/plain",
+      "US-ASCII",
+      "boo" },
+
+    // Invalid mediatype. Includes a slash but the type part is not a token.
+    { "data:f(oo/bar;baz=1;charset=kk,boo",
+      true,
+      "text/plain",
+      "US-ASCII",
+      "boo" },
+
     { "data:foo/bar;baz=1;charset=kk,boo",
       true,
       "foo/bar",
@@ -88,13 +110,6 @@ TEST(DataURLTest, Parse) {
       "US-ASCII",
       "<html><body><b>hello world</b></body></html>" },
 
-    // Bad mime type
-    { "data:f(oo/bar;baz=1;charset=kk,boo",
-      false,
-      "",
-      "",
-      "" },
-
     // the comma cannot be url-escaped!
     { "data:%2Cblah",
       false,
diff --git a/net/url_request/url_request_data_job_unittest.cc b/net/url_request/url_request_data_job_unittest.cc
@@ -63,12 +63,17 @@ TEST(BuildResponseTest, InvalidMimeType) {
   scoped_refptr<net::HttpResponseHeaders> headers(
       new net::HttpResponseHeaders(std::string()));
 
-  // MIME type contains delimiters. Must be rejected.
+  // MIME type contains delimiters. Must be accepted but Content-Type header
+  // should be generated as if the mediatype was text/plain.
   EXPECT_EQ(
-      net::ERR_INVALID_URL,
+      net::OK,
       URLRequestDataJob::BuildResponse(
           GURL("data:f(o/b)r,test"),
           &mime_type, &charset, &data, headers.get()));
+
+  std::string value;
+  EXPECT_TRUE(headers->GetNormalizedHeader("Content-Type", &value));
+  EXPECT_EQ(value, "text/plain;charset=US-ASCII");
 }
 
 TEST(BuildResponseTest, InvalidCharset) {
