This patch tunes our parsing of the Content-Disposition header to match other

browsers and the specs slightly better.

There are two changes in this patch:

1) We now require disposition-type to match the RFC 2616 token production in
   order to treat the response as an attachment (i.e., a download).

2) When there are multiple filename parameters, we now take the first one,
   which maches our behavior prior to the new parser and matches what other
   browsers do.

Review URL: http://codereview.chromium.org/9225046

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@119833 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 50 insertions, 27 deletions

diff --git a/net/http/http_content_disposition.cc b/net/http/http_content_disposition.cc
index 456e6d4..5ec09c9 100644
--- a/net/http/http_content_disposition.cc
+++ b/net/http/http_content_disposition.cc
@@ -23,20 +23,21 @@ HttpContentDisposition::~HttpContentDisposition() {
 std::string::const_iterator HttpContentDisposition::ConsumeDispositionType(
     std::string::const_iterator begin, std::string::const_iterator end) {
   DCHECK(type_ == INLINE);
-
   std::string::const_iterator delimiter = std::find(begin, end, ';');
 
-  // If there's an '=' in before the first ';', then the Content-Disposition
-  // header is malformed, and we treat the first bytes as a parameter rather
-  // than a disposition-type.
-  if (std::find(begin, delimiter, '=') != delimiter)
-    return begin;
-
   std::string::const_iterator type_begin = begin;
   std::string::const_iterator type_end = delimiter;
   HttpUtil::TrimLWS(&type_begin, &type_end);
-  if (type_begin != type_end &&
-      !LowerCaseEqualsASCII(type_begin, type_end, "inline"))
+
+  // If the disposition-type isn't a valid token the then the
+  // Content-Disposition header is malformed, and we treat the first bytes as
+  // a parameter rather than a disposition-type.
+  if (!HttpUtil::IsToken(type_begin, type_end))
+    return begin;
+
+  DCHECK(std::find(type_begin, type_end, '=') == type_end);
+
+  if (!LowerCaseEqualsASCII(type_begin, type_end, "inline"))
     type_ = ATTACHMENT;
   return delimiter;
 }
@@ -73,17 +74,17 @@ void HttpContentDisposition::Parse(const std::string& header,
 
   HttpUtil::NameValuePairsIterator iter(pos, end, ';');
   while (iter.GetNext()) {
-    if (LowerCaseEqualsASCII(iter.name_begin(),
-                             iter.name_end(),
-                             "filename")) {
+    if (filename.empty() && LowerCaseEqualsASCII(iter.name_begin(),
+                                                 iter.name_end(),
+                                                 "filename")) {
       DecodeFilenameValue(iter.value(), referrer_charset, &filename);
-    } else if (LowerCaseEqualsASCII(iter.name_begin(),
-                             iter.name_end(),
-                             "name")) {
+    } else if (filename.empty() && LowerCaseEqualsASCII(iter.name_begin(),
+                                                        iter.name_end(),
+                                                        "name")) {
       DecodeFilenameValue(iter.value(), referrer_charset, &filename);
-    } else if (LowerCaseEqualsASCII(iter.name_begin(),
-                                    iter.name_end(),
-                                    "filename*")) {
+    } else if (ext_filename.empty() && LowerCaseEqualsASCII(iter.name_begin(),
+                                                            iter.name_end(),
+                                                            "filename*")) {
       DecodeExtValue(iter.raw_value(), &ext_filename);
     }
   }
diff --git a/net/http/http_content_disposition_unittest.cc b/net/http/http_content_disposition_unittest.cc
index 0bba0b6..7587c3e 100644
--- a/net/http/http_content_disposition_unittest.cc
+++ b/net/http/http_content_disposition_unittest.cc
@@ -212,7 +212,7 @@ TEST(HttpContentDispositionTest, tc2231) {
     },
     // http://greenbytes.de/tech/tc2231/#inlonlyquoted
     { "\"inline\"",
-      net::HttpContentDisposition::ATTACHMENT,
+      net::HttpContentDisposition::INLINE,
       L""
     },
     // http://greenbytes.de/tech/tc2231/#inlwithasciifilename
@@ -237,7 +237,7 @@ TEST(HttpContentDispositionTest, tc2231) {
     },
     // http://greenbytes.de/tech/tc2231/#attonlyquoted
     { "\"attachment\"",
-      net::HttpContentDisposition::ATTACHMENT,
+      net::HttpContentDisposition::INLINE,
       L""
     },
     // http://greenbytes.de/tech/tc2231/#attonly403
@@ -364,7 +364,7 @@ TEST(HttpContentDispositionTest, tc2231) {
     // Note: tc2231 says we should fail to parse this header.
     { "attachment; filename=\"foo.html\"; filename=\"bar.html\"",
       net::HttpContentDisposition::ATTACHMENT,
-      L"bar.html"  // Probably should be foo.html to match other browsers.
+      L"foo.html"
     },
     // http://greenbytes.de/tech/tc2231/#attfnbrokentoken
     // Note: tc2231 says we should fail to parse this header.
@@ -401,8 +401,8 @@ TEST(HttpContentDispositionTest, tc2231) {
     // http://greenbytes.de/tech/tc2231/#attmissingdisposition3
     // Note: tc2231 says we should fail to parse this header.
     { "\"foo; filename=bar;baz\"; filename=qux",
-      net::HttpContentDisposition::ATTACHMENT,
-      L"bar"  // Firefox gets qux
+      net::HttpContentDisposition::INLINE,
+      L""  // Firefox gets qux
     },
     // http://greenbytes.de/tech/tc2231/#attmissingdisposition4
     // Note: tc2231 says we should fail to parse this header.
@@ -447,10 +447,10 @@ TEST(HttpContentDispositionTest, tc2231) {
       L"foo\"bar;baz\"qux"
     },
     // http://greenbytes.de/tech/tc2231/#attmultinstances
-    { "attachment; filename=foo.html, attachment; filename=bar.html",
     // Note: tc2231 says we should fail to parse this header.
+    { "attachment; filename=foo.html, attachment; filename=bar.html",
       net::HttpContentDisposition::ATTACHMENT,
-      L"bar.html"
+      L"foo.html, attachment"
     },
     // http://greenbytes.de/tech/tc2231/#attmissingdelim
     { "attachment; foo=foo filename=bar",
diff --git a/net/http/http_util.cc b/net/http/http_util.cc
index db37dfb..4096ac8 100644
--- a/net/http/http_util.cc
+++ b/net/http/http_util.cc
@@ -413,14 +413,29 @@ void HttpUtil::TrimLWS(string::const_iterator* begin,
     --(*end);
 }
 
-// static
 bool HttpUtil::IsQuote(char c) {
   // Single quote mark isn't actually part of quoted-text production,
   // but apparently some servers rely on this.
   return c == '"' || c == '\'';
 }
 
-// static
+// See RFC 2616 Sec 2.2 for the definition of |token|.
+bool HttpUtil::IsToken(string::const_iterator begin,
+                       string::const_iterator end) {
+  if (begin == end)
+    return false;
+  for (std::string::const_iterator iter = begin; iter != end; ++iter) {
+    unsigned char c = *iter;
+    if (c >= 0x80 || c <= 0x1F || c == 0x7F ||
+        c == '(' || c == ')' || c == '<' || c == '>' || c == '@' ||
+        c == ',' || c == ';' || c == ':' || c == '\\' || c == '"' ||
+        c == '/' || c == '[' || c == ']' || c == '?' || c == '=' ||
+        c == '{' || c == '}' || c == ' ' || c == '\t')
+      return false;
+  }
+  return true;
+}
+
 std::string HttpUtil::Unquote(std::string::const_iterator begin,
                               std::string::const_iterator end) {
   // Empty string
diff --git a/net/http/http_util.h b/net/http/http_util.h
index 41f2713..57aa0d0 100644
--- a/net/http/http_util.h
+++ b/net/http/http_util.h
@@ -104,6 +104,13 @@ class NET_EXPORT HttpUtil {
   // Whether the character is the start of a quotation mark.
   static bool IsQuote(char c);
 
+  // Whether the string is a valid |token| as defined in RFC 2616 Sec 2.2.
+  static bool IsToken(std::string::const_iterator begin,
+                      std::string::const_iterator end);
+  static bool IsToken(const std::string& str) {
+    return IsToken(str.begin(), str.end());
+  }
+
   // RFC 2616 Sec 2.2:
   // quoted-string = ( <"> *(qdtext | quoted-pair ) <"> )
   // Unquote() strips the surrounding quotemarks off a string, and unescapes