From a44776c18ffd78765568649ec912fd0a67363f6e Mon Sep 17 00:00:00 2001 From: "abarth@chromium.org" Date: Tue, 31 Jan 2012 03:25:30 +0000 Subject: 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 --- net/http/http_content_disposition.cc | 37 ++++++++++++++------------- net/http/http_content_disposition_unittest.cc | 14 +++++----- net/http/http_util.cc | 19 ++++++++++++-- net/http/http_util.h | 7 +++++ 4 files changed, 50 insertions(+), 27 deletions(-) (limited to 'net') 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 -- cgit v1.1