Header parser: Don't allow single quotes.
No HTTP header specs allow single quotes to be used instead of double
quotes, so this CL better aligns Chrome with the spec.
Testing current behavior, using
http://test.greenbytes.de/tech/tc2231/attwithfntokensq.asis:
FireFox doesn't support single quotes, but Chrome and Edge do.
Bug: 896233, 179825
Change-Id: I29f034180d3dec06d767e6dff0ce938f48e47147
Reviewed-on: https://chromium-review.googlesource.com/c/1286733
Reviewed-by: Min Qin <qinmin@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Reviewed-by: Asanka Herath <asanka@chromium.org>
Commit-Queue: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602768}diff --git a/components/download/internal/common/download_stats.cc b/components/download/internal/common/download_stats.cc
index b6fd99e..91f6c99 100644
--- a/components/download/internal/common/download_stats.cc
+++ b/components/download/internal/common/download_stats.cc
@@ -58,6 +58,8 @@
CONTENT_DISPOSITION_HAS_NAME_ONLY, // Obsolete; kept for UMA compatiblity.
+ CONTENT_DISPOSITION_HAS_SINGLE_QUOTED_FILENAME,
+
CONTENT_DISPOSITION_LAST_ENTRY
};
@@ -779,6 +781,9 @@
RecordContentDispositionCountFlag(
CONTENT_DISPOSITION_HAS_RFC2047_ENCODED_STRINGS, result,
net::HttpContentDisposition::HAS_RFC2047_ENCODED_STRINGS);
+ RecordContentDispositionCountFlag(
+ CONTENT_DISPOSITION_HAS_SINGLE_QUOTED_FILENAME, result,
+ net::HttpContentDisposition::HAS_SINGLE_QUOTED_FILENAME);
}
void RecordOpen(const base::Time& end) {
diff --git a/components/gcm_driver/crypto/encryption_header_parsers_unittest.cc b/components/gcm_driver/crypto/encryption_header_parsers_unittest.cc
index b7142cee..021a079 100644
--- a/components/gcm_driver/crypto/encryption_header_parsers_unittest.cc
+++ b/components/gcm_driver/crypto/encryption_header_parsers_unittest.cc
@@ -37,7 +37,6 @@
{ "keyid=foo", "foo", "", kDefaultRecordSize },
{ "keyid=foo;", "foo", "", kDefaultRecordSize },
{ "keyid=\"foo\"", "foo", "", kDefaultRecordSize },
- { "keyid='foo'", "foo", "", kDefaultRecordSize },
{ "salt=c2l4dGVlbmNvb2xieXRlcw",
"", "sixteencoolbytes", kDefaultRecordSize },
{ "rs=2048", "", "", 2048 },
@@ -186,7 +185,6 @@
{ "keyid=foo", "foo", "", "" },
{ "aesgcm128=c2l4dGVlbmNvb2xieXRlcw", "", "sixteencoolbytes", "" },
{ "aesgcm128=\"c2l4dGVlbmNvb2xieXRlcw\"", "", "sixteencoolbytes", "" },
- { "aesgcm128='c2l4dGVlbmNvb2xieXRlcw'", "", "sixteencoolbytes", "" },
{ "dh=dHdlbHZlY29vbGJ5dGVz", "", "", "twelvecoolbytes" },
{ "keyid=foo;someothervalue=bar;aesgcm128=dHdlbHZlY29vbGJ5dGVz",
"foo", "twelvecoolbytes", "" },
diff --git a/net/http/http_content_disposition.cc b/net/http/http_content_disposition.cc
index aba9e10..bdf8ab44 100644
--- a/net/http/http_content_disposition.cc
+++ b/net/http/http_content_disposition.cc
@@ -407,8 +407,11 @@
"filename")) {
DecodeFilenameValue(iter.value(), referrer_charset, &filename,
&parse_result_flags_);
- if (!filename.empty())
+ if (!filename.empty()) {
parse_result_flags_ |= HAS_FILENAME;
+ if (filename[0] == '\'')
+ parse_result_flags_ |= HAS_SINGLE_QUOTED_FILENAME;
+ }
} else if (ext_filename.empty() &&
base::LowerCaseEqualsASCII(
base::StringPiece(iter.name_begin(), iter.name_end()),
@@ -423,6 +426,9 @@
filename_ = ext_filename;
else
filename_ = filename;
+
+ if (!filename.empty() && filename[0] == '\'')
+ parse_result_flags_ |= HAS_SINGLE_QUOTED_FILENAME;
}
} // namespace net
diff --git a/net/http/http_content_disposition.h b/net/http/http_content_disposition.h
index 0f1b5ac0..7ae1bee 100644
--- a/net/http/http_content_disposition.h
+++ b/net/http/http_content_disposition.h
@@ -23,30 +23,33 @@
// report download metrics in UMA. This enum isn't directly used in UMA but
// mapped to another one for binary compatiblity; ie. changes are OK.
enum ParseResultFlags {
- INVALID = 0,
+ INVALID = 0,
// A valid disposition-type is present.
- HAS_DISPOSITION_TYPE = 1 << 0,
+ HAS_DISPOSITION_TYPE = 1 << 0,
// The disposition-type is not 'inline' or 'attachment'.
HAS_UNKNOWN_DISPOSITION_TYPE = 1 << 1,
// Has a valid non-empty 'filename' attribute.
- HAS_FILENAME = 1 << 2,
+ HAS_FILENAME = 1 << 2,
// Has a valid non-empty 'filename*' attribute.
- HAS_EXT_FILENAME = 1 << 3,
+ HAS_EXT_FILENAME = 1 << 3,
// The following fields are properties of the 'filename' attribute:
// Quoted-string contains non-ASCII characters.
- HAS_NON_ASCII_STRINGS = 1 << 4,
+ HAS_NON_ASCII_STRINGS = 1 << 4,
// Quoted-string contains percent-encoding.
- HAS_PERCENT_ENCODED_STRINGS = 1 << 5,
+ HAS_PERCENT_ENCODED_STRINGS = 1 << 5,
// Quoted-string contains RFC 2047 encoded words.
- HAS_RFC2047_ENCODED_STRINGS = 1 << 6
+ HAS_RFC2047_ENCODED_STRINGS = 1 << 6,
+
+ // Has a filename that starts with a single quote.
+ HAS_SINGLE_QUOTED_FILENAME = 1 << 7,
};
HttpContentDisposition(const std::string& header,
diff --git a/net/http/http_content_disposition_unittest.cc b/net/http/http_content_disposition_unittest.cc
index f4db044..f283ce9 100644
--- a/net/http/http_content_disposition_unittest.cc
+++ b/net/http/http_content_disposition_unittest.cc
@@ -221,16 +221,13 @@
// http://greenbytes.de/tech/tc2231/#inlonlyquoted
{"\"inline\"", HttpContentDisposition::INLINE, L""},
// http://greenbytes.de/tech/tc2231/#inlwithasciifilename
- {"inline; filename=\"foo.html\"",
- HttpContentDisposition::INLINE,
+ {"inline; filename=\"foo.html\"", HttpContentDisposition::INLINE,
L"foo.html"},
// http://greenbytes.de/tech/tc2231/#inlwithfnattach
{"inline; filename=\"Not an attachment!\"",
- HttpContentDisposition::INLINE,
- L"Not an attachment!"},
+ HttpContentDisposition::INLINE, L"Not an attachment!"},
// http://greenbytes.de/tech/tc2231/#inlwithasciifilenamepdf
- {"inline; filename=\"foo.pdf\"",
- HttpContentDisposition::INLINE,
+ {"inline; filename=\"foo.pdf\"", HttpContentDisposition::INLINE,
L"foo.pdf"},
// http://greenbytes.de/tech/tc2231/#attonly
{"attachment", HttpContentDisposition::ATTACHMENT, L""},
@@ -241,132 +238,109 @@
// http://greenbytes.de/tech/tc2231/#attonlyucase
{"ATTACHMENT", HttpContentDisposition::ATTACHMENT, L""},
// http://greenbytes.de/tech/tc2231/#attwithasciifilename
- {"attachment; filename=\"foo.html\"",
- HttpContentDisposition::ATTACHMENT,
+ {"attachment; filename=\"foo.html\"", HttpContentDisposition::ATTACHMENT,
L"foo.html"},
// http://greenbytes.de/tech/tc2231/#attwithasciifnescapedchar
{"attachment; filename=\"f\\oo.html\"",
- HttpContentDisposition::ATTACHMENT,
- L"foo.html"},
+ HttpContentDisposition::ATTACHMENT, L"foo.html"},
// http://greenbytes.de/tech/tc2231/#attwithasciifnescapedquote
{"attachment; filename=\"\\\"quoting\\\" tested.html\"",
- HttpContentDisposition::ATTACHMENT,
- L"\"quoting\" tested.html"},
+ HttpContentDisposition::ATTACHMENT, L"\"quoting\" tested.html"},
// http://greenbytes.de/tech/tc2231/#attwithquotedsemicolon
{"attachment; filename=\"Here's a semicolon;.html\"",
- HttpContentDisposition::ATTACHMENT,
- L"Here's a semicolon;.html"},
+ HttpContentDisposition::ATTACHMENT, L"Here's a semicolon;.html"},
// http://greenbytes.de/tech/tc2231/#attwithfilenameandextparam
{"attachment; foo=\"bar\"; filename=\"foo.html\"",
- HttpContentDisposition::ATTACHMENT,
- L"foo.html"},
+ HttpContentDisposition::ATTACHMENT, L"foo.html"},
// http://greenbytes.de/tech/tc2231/#attwithfilenameandextparamescaped
{"attachment; foo=\"\\\"\\\\\";filename=\"foo.html\"",
- HttpContentDisposition::ATTACHMENT,
- L"foo.html"},
+ HttpContentDisposition::ATTACHMENT, L"foo.html"},
// http://greenbytes.de/tech/tc2231/#attwithasciifilenameucase
- {"attachment; FILENAME=\"foo.html\"",
- HttpContentDisposition::ATTACHMENT,
+ {"attachment; FILENAME=\"foo.html\"", HttpContentDisposition::ATTACHMENT,
L"foo.html"},
// http://greenbytes.de/tech/tc2231/#attwithasciifilenamenq
- {"attachment; filename=foo.html",
- HttpContentDisposition::ATTACHMENT,
+ {"attachment; filename=foo.html", HttpContentDisposition::ATTACHMENT,
L"foo.html"},
// http://greenbytes.de/tech/tc2231/#attwithasciifilenamenqs
// Note: tc2231 says we should fail to parse this header.
- {"attachment; filename=foo.html ;",
- HttpContentDisposition::ATTACHMENT,
+ {"attachment; filename=foo.html ;", HttpContentDisposition::ATTACHMENT,
L"foo.html"},
// http://greenbytes.de/tech/tc2231/#attemptyparam
// Note: tc2231 says we should fail to parse this header.
{"attachment; ;filename=foo", HttpContentDisposition::ATTACHMENT, L"foo"},
// http://greenbytes.de/tech/tc2231/#attwithasciifilenamenqws
// Note: tc2231 says we should fail to parse this header.
- {"attachment; filename=foo bar.html",
- HttpContentDisposition::ATTACHMENT,
+ {"attachment; filename=foo bar.html", HttpContentDisposition::ATTACHMENT,
L"foo bar.html"},
// http://greenbytes.de/tech/tc2231/#attwithfntokensq
- {
- "attachment; filename='foo.bar'",
- HttpContentDisposition::ATTACHMENT,
- L"foo.bar" // Should be L"'foo.bar'"
- },
+ {"attachment; filename='foo.bar'", HttpContentDisposition::ATTACHMENT,
+ L"'foo.bar'"},
#ifdef ICU_SHOULD_FAIL_CONVERSION_ON_INVALID_CHARACTER
// http://greenbytes.de/tech/tc2231/#attwithisofnplain
{
- "attachment; filename=\"foo-\xE4html\"",
- HttpContentDisposition::ATTACHMENT,
- L"" // Should be L"foo-\xE4.html"
+ "attachment; filename=\"foo-\xE4html\"",
+ HttpContentDisposition::ATTACHMENT,
+ L"" // Should be L"foo-\xE4.html"
},
#endif
// http://greenbytes.de/tech/tc2231/#attwithutf8fnplain
// Note: We'll UTF-8 decode the file name, even though tc2231 says not to.
{"attachment; filename=\"foo-\xC3\xA4.html\"",
- HttpContentDisposition::ATTACHMENT,
- L"foo-\xE4.html"},
+ HttpContentDisposition::ATTACHMENT, L"foo-\xE4.html"},
// http://greenbytes.de/tech/tc2231/#attwithfnrawpctenca
{
- "attachment; filename=\"foo-%41.html\"",
- HttpContentDisposition::ATTACHMENT,
- L"foo-A.html" // Should be L"foo-%41.html"
+ "attachment; filename=\"foo-%41.html\"",
+ HttpContentDisposition::ATTACHMENT,
+ L"foo-A.html" // Should be L"foo-%41.html"
},
// http://greenbytes.de/tech/tc2231/#attwithfnusingpct
- {"attachment; filename=\"50%.html\"",
- HttpContentDisposition::ATTACHMENT,
+ {"attachment; filename=\"50%.html\"", HttpContentDisposition::ATTACHMENT,
L"50%.html"},
// http://greenbytes.de/tech/tc2231/#attwithfnrawpctencaq
{
- "attachment; filename=\"foo-%\\41.html\"",
- HttpContentDisposition::ATTACHMENT,
- L"foo-A.html" // Should be L"foo-%41.html"
+ "attachment; filename=\"foo-%\\41.html\"",
+ HttpContentDisposition::ATTACHMENT,
+ L"foo-A.html" // Should be L"foo-%41.html"
},
// http://greenbytes.de/tech/tc2231/#attwithnamepct
// Value is skipped like other UAs.
- {
- "attachment; name=\"foo-%41.html\"",
- HttpContentDisposition::ATTACHMENT,
- L""
- },
+ {"attachment; name=\"foo-%41.html\"", HttpContentDisposition::ATTACHMENT,
+ L""},
#ifdef ICU_SHOULD_FAIL_CONVERSION_ON_INVALID_CHARACTER
// http://greenbytes.de/tech/tc2231/#attwithfilenamepctandiso
{
- "attachment; filename=\"\xE4-%41.html\"",
- HttpContentDisposition::ATTACHMENT,
- L"" // Should be L"\xE4-%41.htm"
+ "attachment; filename=\"\xE4-%41.html\"",
+ HttpContentDisposition::ATTACHMENT,
+ L"" // Should be L"\xE4-%41.htm"
},
#endif
// http://greenbytes.de/tech/tc2231/#attwithfnrawpctenclong
{
- "attachment; filename=\"foo-%c3%a4-%e2%82%ac.html\"",
- HttpContentDisposition::ATTACHMENT,
- L"foo-\xE4-\u20AC.html" // Should be L"foo-%c3%a4-%e2%82%ac.html"
+ "attachment; filename=\"foo-%c3%a4-%e2%82%ac.html\"",
+ HttpContentDisposition::ATTACHMENT,
+ L"foo-\xE4-\u20AC.html" // Should be L"foo-%c3%a4-%e2%82%ac.html"
},
// http://greenbytes.de/tech/tc2231/#attwithasciifilenamews1
- {"attachment; filename =\"foo.html\"",
- HttpContentDisposition::ATTACHMENT,
+ {"attachment; filename =\"foo.html\"", HttpContentDisposition::ATTACHMENT,
L"foo.html"},
// http://greenbytes.de/tech/tc2231/#attwith2filenames
// Note: tc2231 says we should fail to parse this header.
{"attachment; filename=\"foo.html\"; filename=\"bar.html\"",
- HttpContentDisposition::ATTACHMENT,
- L"foo.html"},
+ HttpContentDisposition::ATTACHMENT, L"foo.html"},
// http://greenbytes.de/tech/tc2231/#attfnbrokentoken
// Note: tc2231 says we should fail to parse this header.
{"attachment; filename=foo[1](2).html",
- HttpContentDisposition::ATTACHMENT,
- L"foo[1](2).html"},
+ HttpContentDisposition::ATTACHMENT, L"foo[1](2).html"},
#ifdef ICU_SHOULD_FAIL_CONVERSION_ON_INVALID_CHARACTER
// http://greenbytes.de/tech/tc2231/#attfnbrokentokeniso
// Note: tc2231 says we should fail to parse this header.
- {"attachment; filename=foo-\xE4.html",
- HttpContentDisposition::ATTACHMENT,
+ {"attachment; filename=foo-\xE4.html", HttpContentDisposition::ATTACHMENT,
L""},
#endif
// http://greenbytes.de/tech/tc2231/#attfnbrokentokenutf
// Note: tc2231 says we should fail to parse this header.
{"attachment; filename=foo-\xC3\xA4.html",
- HttpContentDisposition::ATTACHMENT,
- L"foo-\xE4.html"},
+ HttpContentDisposition::ATTACHMENT, L"foo-\xE4.html"},
// http://greenbytes.de/tech/tc2231/#attmissingdisposition
// Note: tc2231 says we should fail to parse this header.
{"filename=foo.html", HttpContentDisposition::INLINE, L"foo.html"},
@@ -376,79 +350,65 @@
// http://greenbytes.de/tech/tc2231/#attmissingdisposition3
// Note: tc2231 says we should fail to parse this header.
{
- "\"foo; filename=bar;baz\"; filename=qux",
- HttpContentDisposition::INLINE,
- L"" // Firefox gets qux
+ "\"foo; filename=bar;baz\"; filename=qux",
+ HttpContentDisposition::INLINE,
+ L"" // Firefox gets qux
},
// http://greenbytes.de/tech/tc2231/#attmissingdisposition4
// Note: tc2231 says we should fail to parse this header.
- {"filename=foo.html, filename=bar.html",
- HttpContentDisposition::INLINE,
+ {"filename=foo.html, filename=bar.html", HttpContentDisposition::INLINE,
L"foo.html, filename=bar.html"},
// http://greenbytes.de/tech/tc2231/#emptydisposition
// Note: tc2231 says we should fail to parse this header.
{"; filename=foo.html", HttpContentDisposition::INLINE, L"foo.html"},
// http://greenbytes.de/tech/tc2231/#attandinline
// Note: tc2231 says we should fail to parse this header.
- {"inline; attachment; filename=foo.html",
- HttpContentDisposition::INLINE,
+ {"inline; attachment; filename=foo.html", HttpContentDisposition::INLINE,
L""},
// http://greenbytes.de/tech/tc2231/#attandinline2
// Note: tc2231 says we should fail to parse this header.
{"attachment; inline; filename=foo.html",
- HttpContentDisposition::ATTACHMENT,
- L""},
+ HttpContentDisposition::ATTACHMENT, L""},
// http://greenbytes.de/tech/tc2231/#attbrokenquotedfn
// Note: tc2231 says we should fail to parse this header.
{"attachment; filename=\"foo.html\".txt",
- HttpContentDisposition::ATTACHMENT,
- L"foo.html\".txt"},
+ HttpContentDisposition::ATTACHMENT, L"foo.html\".txt"},
// http://greenbytes.de/tech/tc2231/#attbrokenquotedfn2
// Note: tc2231 says we should fail to parse this header.
- {"attachment; filename=\"bar",
- HttpContentDisposition::ATTACHMENT,
+ {"attachment; filename=\"bar", HttpContentDisposition::ATTACHMENT,
L"bar"},
// http://greenbytes.de/tech/tc2231/#attbrokenquotedfn3
// Note: tc2231 says we should fail to parse this header.
{"attachment; filename=foo\"bar;baz\"qux",
- HttpContentDisposition::ATTACHMENT,
- L"foo\"bar;baz\"qux"},
+ HttpContentDisposition::ATTACHMENT, L"foo\"bar;baz\"qux"},
// http://greenbytes.de/tech/tc2231/#attmultinstances
// Note: tc2231 says we should fail to parse this header.
{"attachment; filename=foo.html, attachment; filename=bar.html",
- HttpContentDisposition::ATTACHMENT,
- L"foo.html, attachment"},
+ HttpContentDisposition::ATTACHMENT, L"foo.html, attachment"},
// http://greenbytes.de/tech/tc2231/#attmissingdelim
- {"attachment; foo=foo filename=bar",
- HttpContentDisposition::ATTACHMENT,
+ {"attachment; foo=foo filename=bar", HttpContentDisposition::ATTACHMENT,
L""},
// http://greenbytes.de/tech/tc2231/#attreversed
// Note: tc2231 says we should fail to parse this header.
- {"filename=foo.html; attachment",
- HttpContentDisposition::INLINE,
+ {"filename=foo.html; attachment", HttpContentDisposition::INLINE,
L"foo.html"},
// http://greenbytes.de/tech/tc2231/#attconfusedparam
- {"attachment; xfilename=foo.html",
- HttpContentDisposition::ATTACHMENT,
+ {"attachment; xfilename=foo.html", HttpContentDisposition::ATTACHMENT,
L""},
// http://greenbytes.de/tech/tc2231/#attabspath
- {"attachment; filename=\"/foo.html\"",
- HttpContentDisposition::ATTACHMENT,
+ {"attachment; filename=\"/foo.html\"", HttpContentDisposition::ATTACHMENT,
L"/foo.html"},
// http://greenbytes.de/tech/tc2231/#attabspathwin
{"attachment; filename=\"\\\\foo.html\"",
- HttpContentDisposition::ATTACHMENT,
- L"\\foo.html"},
+ HttpContentDisposition::ATTACHMENT, L"\\foo.html"},
// http://greenbytes.de/tech/tc2231/#dispext
{"foobar", HttpContentDisposition::ATTACHMENT, L""},
// http://greenbytes.de/tech/tc2231/#dispextbadfn
{"attachment; example=\"filename=example.txt\"",
- HttpContentDisposition::ATTACHMENT,
- L""},
+ HttpContentDisposition::ATTACHMENT, L""},
// http://greenbytes.de/tech/tc2231/#attnewandfn
{"attachment; foobar=x; filename=\"foo.html\"",
- HttpContentDisposition::ATTACHMENT,
- L"foo.html"},
+ HttpContentDisposition::ATTACHMENT, L"foo.html"},
// TODO(abarth): Add the filename* tests, but check
// HttpContentDispositionTest.Filename for overlap.
// TODO(abarth): http://greenbytes.de/tech/tc2231/#attrfc2047token
@@ -468,61 +428,63 @@
const char* header;
int expected_flags;
} kTestCases[] = {
- // Basic feature tests
- { "", HttpContentDisposition::INVALID },
- { "example=x", HttpContentDisposition::INVALID },
- { "attachment; filename=", HttpContentDisposition::HAS_DISPOSITION_TYPE },
- { "attachment; name=", HttpContentDisposition::HAS_DISPOSITION_TYPE },
- { "attachment; filename*=", HttpContentDisposition::HAS_DISPOSITION_TYPE },
- { "attachment; filename==?utf-8?Q?\?=",
- HttpContentDisposition::HAS_DISPOSITION_TYPE },
- { "filename=x", HttpContentDisposition::HAS_FILENAME },
- { "example; filename=x",
- HttpContentDisposition::HAS_DISPOSITION_TYPE |
- HttpContentDisposition::HAS_UNKNOWN_DISPOSITION_TYPE |
- HttpContentDisposition::HAS_FILENAME},
- { "attachment; filename=x",
- HttpContentDisposition::HAS_DISPOSITION_TYPE |
- HttpContentDisposition::HAS_FILENAME },
- { "attachment; filename=x; name=y",
- HttpContentDisposition::HAS_DISPOSITION_TYPE |
- HttpContentDisposition::HAS_FILENAME },
- { "attachment; name=y; filename*=utf-8''foo; name=x",
- HttpContentDisposition::HAS_DISPOSITION_TYPE |
- HttpContentDisposition::HAS_EXT_FILENAME },
+ // Basic feature tests
+ {"", HttpContentDisposition::INVALID},
+ {"example=x", HttpContentDisposition::INVALID},
+ {"attachment; filename=", HttpContentDisposition::HAS_DISPOSITION_TYPE},
+ {"attachment; name=", HttpContentDisposition::HAS_DISPOSITION_TYPE},
+ {"attachment; filename*=", HttpContentDisposition::HAS_DISPOSITION_TYPE},
+ {"attachment; filename==?utf-8?Q?\?=",
+ HttpContentDisposition::HAS_DISPOSITION_TYPE},
+ {"filename=x", HttpContentDisposition::HAS_FILENAME},
+ {"example; filename=x",
+ HttpContentDisposition::HAS_DISPOSITION_TYPE |
+ HttpContentDisposition::HAS_UNKNOWN_DISPOSITION_TYPE |
+ HttpContentDisposition::HAS_FILENAME},
+ {"attachment; filename=x", HttpContentDisposition::HAS_DISPOSITION_TYPE |
+ HttpContentDisposition::HAS_FILENAME},
+ {"attachment; filename='x'",
+ HttpContentDisposition::HAS_DISPOSITION_TYPE |
+ HttpContentDisposition::HAS_FILENAME |
+ HttpContentDisposition::HAS_SINGLE_QUOTED_FILENAME},
+ {"attachment; filename=x; name=y",
+ HttpContentDisposition::HAS_DISPOSITION_TYPE |
+ HttpContentDisposition::HAS_FILENAME},
+ {"attachment; name=y; filename*=utf-8''foo; name=x",
+ HttpContentDisposition::HAS_DISPOSITION_TYPE |
+ HttpContentDisposition::HAS_EXT_FILENAME},
- // Feature tests for 'filename' attribute.
- { "filename=foo\xcc\x88",
- HttpContentDisposition::HAS_FILENAME |
- HttpContentDisposition::HAS_NON_ASCII_STRINGS },
- { "filename=foo%cc%88",
- HttpContentDisposition::HAS_FILENAME |
- HttpContentDisposition::HAS_PERCENT_ENCODED_STRINGS },
- { "filename==?utf-8?Q?foo?=",
- HttpContentDisposition::HAS_FILENAME |
- HttpContentDisposition::HAS_RFC2047_ENCODED_STRINGS },
- { "filename=\"=?utf-8?Q?foo?=\"",
- HttpContentDisposition::HAS_FILENAME |
- HttpContentDisposition::HAS_RFC2047_ENCODED_STRINGS },
- { "filename==?utf-8?Q?foo?", HttpContentDisposition::INVALID },
+ // Feature tests for 'filename' attribute.
+ {"filename=foo\xcc\x88",
+ HttpContentDisposition::HAS_FILENAME |
+ HttpContentDisposition::HAS_NON_ASCII_STRINGS},
+ {"filename=foo%cc%88",
+ HttpContentDisposition::HAS_FILENAME |
+ HttpContentDisposition::HAS_PERCENT_ENCODED_STRINGS},
+ {"filename==?utf-8?Q?foo?=",
+ HttpContentDisposition::HAS_FILENAME |
+ HttpContentDisposition::HAS_RFC2047_ENCODED_STRINGS},
+ {"filename=\"=?utf-8?Q?foo?=\"",
+ HttpContentDisposition::HAS_FILENAME |
+ HttpContentDisposition::HAS_RFC2047_ENCODED_STRINGS},
+ {"filename==?utf-8?Q?foo?", HttpContentDisposition::INVALID},
- // Test 'name' isn't a synonym for 'filename'.
- { "name=foo\xcc\x88", HttpContentDisposition::INVALID },
+ // Test 'name' isn't a synonym for 'filename'.
+ {"name=foo\xcc\x88", HttpContentDisposition::INVALID},
- // Shouldn't set |has_non_ascii_strings| based on 'name' attribute.
- { "filename=x; name=foo\xcc\x88",
- HttpContentDisposition::HAS_FILENAME },
- { "filename=foo\xcc\x88 foo%cc%88 =?utf-8?Q?foo?=",
- HttpContentDisposition::HAS_FILENAME |
- HttpContentDisposition::HAS_NON_ASCII_STRINGS |
- HttpContentDisposition::HAS_PERCENT_ENCODED_STRINGS |
- HttpContentDisposition::HAS_RFC2047_ENCODED_STRINGS },
+ // Shouldn't set |has_non_ascii_strings| based on 'name' attribute.
+ {"filename=x; name=foo\xcc\x88", HttpContentDisposition::HAS_FILENAME},
+ {"filename=foo\xcc\x88 foo%cc%88 =?utf-8?Q?foo?=",
+ HttpContentDisposition::HAS_FILENAME |
+ HttpContentDisposition::HAS_NON_ASCII_STRINGS |
+ HttpContentDisposition::HAS_PERCENT_ENCODED_STRINGS |
+ HttpContentDisposition::HAS_RFC2047_ENCODED_STRINGS},
- // If 'filename' attribute is invalid, should set any flags based on it.
- { "filename=foo\xcc\x88 foo%cc%88 =?utf-8?Q?foo?",
- HttpContentDisposition::INVALID },
- { "filename=foo\xcc\x88 foo%cc%88 =?utf-8?Q?foo?; name=x",
- HttpContentDisposition::INVALID },
+ // If 'filename' attribute is invalid, should set any flags based on it.
+ {"filename=foo\xcc\x88 foo%cc%88 =?utf-8?Q?foo?",
+ HttpContentDisposition::INVALID},
+ {"filename=foo\xcc\x88 foo%cc%88 =?utf-8?Q?foo?; name=x",
+ HttpContentDisposition::INVALID},
};
for (size_t i = 0; i < arraysize(kTestCases); ++i) {
diff --git a/net/http/http_util.cc b/net/http/http_util.cc
index 0097c9b..658c91b 100644
--- a/net/http/http_util.cc
+++ b/net/http/http_util.cc
@@ -542,10 +542,9 @@
}
namespace {
+
bool IsQuote(char c) {
- // Single quote mark isn't actually part of quoted-text production,
- // but apparently some servers rely on this.
- return c == '"' || c == '\'';
+ return c == '"';
}
bool UnquoteImpl(std::string::const_iterator begin,
@@ -560,16 +559,10 @@
if (!IsQuote(*begin))
return false;
- // Anything other than double quotes in strict mode.
- if (strict_quotes && *begin != '"')
- return false;
-
// No terminal quote mark.
if (end - begin < 2 || *begin != *(end - 1))
return false;
- char quote = *begin;
-
// Strip quotemarks
++begin;
--end;
@@ -583,7 +576,7 @@
prev_escape = true;
continue;
}
- if (strict_quotes && !prev_escape && c == quote)
+ if (strict_quotes && !prev_escape && IsQuote(c))
return false;
prev_escape = false;
unescaped.push_back(c);
@@ -596,6 +589,7 @@
*out = std::move(unescaped);
return true;
}
+
} // anonymous namespace
std::string HttpUtil::Unquote(std::string::const_iterator begin,
@@ -1023,7 +1017,7 @@
std::string::const_iterator values_end,
char delimiter)
: values_(values_begin, values_end, std::string(1, delimiter)) {
- values_.set_quote_chars("\'\"");
+ values_.set_quote_chars("\"");
}
HttpUtil::ValuesIterator::ValuesIterator(const ValuesIterator& other) = default;
@@ -1058,8 +1052,6 @@
value_is_quoted_(false),
values_optional_(optional_values == Values::NOT_REQUIRED),
strict_quotes_(strict_quotes == Quotes::STRICT_QUOTES) {
- if (strict_quotes_)
- props_.set_quote_chars("\"");
}
HttpUtil::NameValuePairsIterator::NameValuePairsIterator(
@@ -1152,15 +1144,6 @@
return true;
}
-bool HttpUtil::NameValuePairsIterator::IsQuote(char c) const {
- if (strict_quotes_)
- return c == '"';
-
- // The call to the file-scoped IsQuote must be qualified to avoid re-entrantly
- // calling NameValuePairsIterator::IsQuote again.
- return net::IsQuote(c);
-}
-
bool HttpUtil::ParseAcceptEncoding(const std::string& accept_encoding,
std::set<std::string>* allowed_encodings) {
DCHECK(allowed_encodings);
diff --git a/net/http/http_util.h b/net/http/http_util.h
index b4dd8ef..e8cd4df 100644
--- a/net/http/http_util.h
+++ b/net/http/http_util.h
@@ -150,8 +150,6 @@
// unescaped actually is a valid quoted string. Returns false for an empty
// string, a string without quotes, a string with mismatched quotes, and
// a string with unescaped embeded quotes.
- // In accordance with RFC 2616 this method only allows double quotes to
- // enclose the string.
static bool StrictUnquote(std::string::const_iterator begin,
std::string::const_iterator end,
std::string* out) WARN_UNUSED_RESULT;
@@ -344,12 +342,6 @@
ValuesIterator(const ValuesIterator& other);
~ValuesIterator();
- // Set the characters to regard as quotes. By default, this includes both
- // single and double quotes.
- void set_quote_chars(const char* quotes) {
- values_.set_quote_chars(quotes);
- }
-
// Advances the iterator to the next value, if any. Returns true if there
// is a next value. Use value* methods to access the resultant value.
bool GetNext();
@@ -439,8 +431,6 @@
value_end_); }
private:
- bool IsQuote(char c) const;
-
HttpUtil::ValuesIterator props_;
bool valid_;
diff --git a/net/http/http_util_unittest.cc b/net/http/http_util_unittest.cc
index d0b59ca..537ffe1 100644
--- a/net/http/http_util_unittest.cc
+++ b/net/http/http_util_unittest.cc
@@ -222,10 +222,6 @@
EXPECT_STREQ("X", HttpUtil::Unquote("X").c_str());
EXPECT_STREQ("\"", HttpUtil::Unquote("\"").c_str());
- // Allow single quotes to act as quote marks.
- // Not part of RFC 2616.
- EXPECT_STREQ("x\"", HttpUtil::Unquote("'x\"'").c_str());
-
// Allow quotes in the middle of the input.
EXPECT_STREQ("foo\"bar", HttpUtil::Unquote("\"foo\"bar\"").c_str());
@@ -1156,25 +1152,26 @@
} // namespace
TEST(HttpUtilTest, NameValuePairsIteratorCopyAndAssign) {
- std::string data = "alpha='\\'a\\''; beta=\" b \"; cappa='c;'; delta=\"d\"";
+ std::string data =
+ "alpha=\"\\\"a\\\"\"; beta=\" b \"; cappa=\"c;\"; delta=\"d\"";
HttpUtil::NameValuePairsIterator parser_a(data.begin(), data.end(), ';');
EXPECT_TRUE(parser_a.valid());
ASSERT_NO_FATAL_FAILURE(
- CheckNextNameValuePair(&parser_a, true, true, "alpha", "'a'"));
+ CheckNextNameValuePair(&parser_a, true, true, "alpha", "\"a\""));
HttpUtil::NameValuePairsIterator parser_b(parser_a);
// a and b now point to same location
ASSERT_NO_FATAL_FAILURE(
- CheckCurrentNameValuePair(&parser_b, true, "alpha", "'a'"));
+ CheckCurrentNameValuePair(&parser_b, true, "alpha", "\"a\""));
ASSERT_NO_FATAL_FAILURE(
- CheckCurrentNameValuePair(&parser_a, true, "alpha", "'a'"));
+ CheckCurrentNameValuePair(&parser_a, true, "alpha", "\"a\""));
// advance a, no effect on b
ASSERT_NO_FATAL_FAILURE(
CheckNextNameValuePair(&parser_a, true, true, "beta", " b "));
ASSERT_NO_FATAL_FAILURE(
- CheckCurrentNameValuePair(&parser_b, true, "alpha", "'a'"));
+ CheckCurrentNameValuePair(&parser_b, true, "alpha", "\"a\""));
// assign b the current state of a, no effect on a
parser_b = parser_a;
@@ -1200,10 +1197,13 @@
}
TEST(HttpUtilTest, NameValuePairsIterator) {
- std::string data = "alpha=1; beta= 2 ;cappa =' 3; ';"
- "delta= \" \\\"4\\\" \"; e= \" '5'\"; e=6;"
- "f='\\'\\h\\e\\l\\l\\o\\ \\w\\o\\r\\l\\d\\'';"
- "g=''; h='hello'";
+ std::string data =
+ "alpha=1; beta= 2 ;"
+ "cappa =' 3; foo=';"
+ "cappa =\" 3; foo=\";"
+ "delta= \" \\\"4\\\" \"; e= \" '5'\"; e=6;"
+ "f=\"\\\"\\h\\e\\l\\l\\o\\ \\w\\o\\r\\l\\d\\\"\";"
+ "g=\"\"; h=\"hello\"";
HttpUtil::NameValuePairsIterator parser(data.begin(), data.end(), ';');
EXPECT_TRUE(parser.valid());
@@ -1211,8 +1211,17 @@
CheckNextNameValuePair(&parser, true, true, "alpha", "1"));
ASSERT_NO_FATAL_FAILURE(
CheckNextNameValuePair(&parser, true, true, "beta", "2"));
+
+ // Single quotes shouldn't be treated as quotes.
ASSERT_NO_FATAL_FAILURE(
- CheckNextNameValuePair(&parser, true, true, "cappa", " 3; "));
+ CheckNextNameValuePair(&parser, true, true, "cappa", "' 3"));
+ ASSERT_NO_FATAL_FAILURE(
+ CheckNextNameValuePair(&parser, true, true, "foo", "'"));
+
+ // But double quotes should be, and can contain semi-colons and equal signs.
+ ASSERT_NO_FATAL_FAILURE(
+ CheckNextNameValuePair(&parser, true, true, "cappa", " 3; foo="));
+
ASSERT_NO_FATAL_FAILURE(
CheckNextNameValuePair(&parser, true, true, "delta", " \"4\" "));
ASSERT_NO_FATAL_FAILURE(
@@ -1220,7 +1229,7 @@
ASSERT_NO_FATAL_FAILURE(
CheckNextNameValuePair(&parser, true, true, "e", "6"));
ASSERT_NO_FATAL_FAILURE(
- CheckNextNameValuePair(&parser, true, true, "f", "'hello world'"));
+ CheckNextNameValuePair(&parser, true, true, "f", "\"hello world\""));
ASSERT_NO_FATAL_FAILURE(
CheckNextNameValuePair(&parser, true, true, "g", std::string()));
ASSERT_NO_FATAL_FAILURE(
@@ -1277,8 +1286,9 @@
ASSERT_NO_FATAL_FAILURE(CheckInvalidNameValuePair("alpha=1", "; beta"));
ASSERT_NO_FATAL_FAILURE(CheckInvalidNameValuePair(std::string(), "beta"));
- ASSERT_NO_FATAL_FAILURE(CheckInvalidNameValuePair("alpha=1", "; 'beta'=2"));
- ASSERT_NO_FATAL_FAILURE(CheckInvalidNameValuePair(std::string(), "'beta'=2"));
+ ASSERT_NO_FATAL_FAILURE(CheckInvalidNameValuePair("alpha=1", "; \"beta\"=2"));
+ ASSERT_NO_FATAL_FAILURE(
+ CheckInvalidNameValuePair(std::string(), "\"beta\"=2"));
ASSERT_NO_FATAL_FAILURE(CheckInvalidNameValuePair("alpha=1", ";beta="));
ASSERT_NO_FATAL_FAILURE(CheckInvalidNameValuePair("alpha=1",
";beta=;cappa=2"));
@@ -1309,7 +1319,7 @@
// See comments on the implementation of NameValuePairsIterator::GetNext
// regarding this derogation from the spec.
TEST(HttpUtilTest, NameValuePairsIteratorMissingEndQuote) {
- std::string data = "name='value";
+ std::string data = "name=\"value";
HttpUtil::NameValuePairsIterator parser(data.begin(), data.end(), ';');
EXPECT_TRUE(parser.valid());
diff --git a/tools/metrics/histograms/enums.xml b/tools/metrics/histograms/enums.xml
index ab7d76f7..8d8840d 100644
--- a/tools/metrics/histograms/enums.xml
+++ b/tools/metrics/histograms/enums.xml
@@ -11678,6 +11678,7 @@
<int value="8" label="Has percent encoded strings"/>
<int value="9" label="Has RFC 2047 encoded strings"/>
<int value="10" label="Has 'name' attribute only (Obsolete 04/2015)"/>
+ <int value="11" label="Filename is a single quoted string"/>
</enum>
<enum name="DownloadContentType">
