diff options
Diffstat (limited to 'net/base')
-rw-r--r-- | net/base/cookie_monster.cc | 60 | ||||
-rw-r--r-- | net/base/cookie_monster.h | 4 | ||||
-rw-r--r-- | net/base/cookie_monster_unittest.cc | 82 |
3 files changed, 101 insertions, 45 deletions
diff --git a/net/base/cookie_monster.cc b/net/base/cookie_monster.cc index dbae99a..6d9afcb 100644 --- a/net/base/cookie_monster.cc +++ b/net/base/cookie_monster.cc @@ -904,7 +904,6 @@ void CookieMonster::ParsedCookie::ParseTokenValuePairs( static const char kTerminator[] = "\n\r\0"; static const int kTerminatorLen = sizeof(kTerminator) - 1; static const char kWhitespace[] = " \t"; - static const char kQuoteTerminator[] = "\""; static const char kValueSeparator[] = ";"; static const char kTokenSeparator[] = ";="; @@ -987,36 +986,35 @@ void CookieMonster::ParsedCookie::ParseTokenValuePairs( // value_start should point at the first character of the value. value_start = it; - // The value is double quoted, process <quoted-string>. - if (it != end && *it == '"') { - // Skip over the first double quote, and parse until - // a terminating double quote or the end. - for (++it; it != end && !CharIsA(*it, kQuoteTerminator); ++it) { - // Allow an escaped \" in a double quoted string. - if (*it == '\\') { - ++it; - if (it == end) - break; - } - } - - SeekTo(&it, end, kValueSeparator); - // We could seek to the end, that's ok. - value_end = it; - } else { - // The value is non-quoted, process <token-value>. - // Just look for ';' to terminate ('=' allowed). - // We can hit the end, maybe they didn't terminate. - SeekTo(&it, end, kValueSeparator); - - // Ignore any whitespace between the value and the value separator - if (it != value_start) { // Could have an empty value - --it; - SeekBackPast(&it, value_start, kWhitespace); - ++it; - } - - value_end = it; + // It is unclear exactly how quoted string values should be handled. + // Major browsers do different things, for example, Firefox supports + // semicolons embedded in a quoted value, while IE does not. Looking at + // the specs, RFC 2109 and 2965 allow for a quoted-string as the value. + // However, these specs were apparently written after browsers had + // implemented cookies, and they seem very distant from the reality of + // what is actually implemented and used on the web. The original spec + // from Netscape is possibly what is closest to the cookies used today. + // This spec didn't have explicit support for double quoted strings, and + // states that ; is not allowed as part of a value. We had originally + // implement the Firefox behavior (A="B;C"; -> A="B;C";). However, since + // there is no standard that makes sense, we decided to follow the behavior + // of IE and Safari, which is closer to the original Netscape proposal. + // This means that A="B;C" -> A="B;. This also makes the code much simpler + // and reduces the possibility for invalid cookies, where other browsers + // like Opera currently reject those invalid cookies (ex A="B" "C";). + + // Just look for ';' to terminate ('=' allowed). + // We can hit the end, maybe they didn't terminate. + SeekTo(&it, end, kValueSeparator); + + // Will be pointed at the ; seperator or the end. + value_end = it; + + // Ignore any unwanted whitespace after the value. + if (value_end != value_start) { // Could have an empty value + --value_end; + SeekBackPast(&value_end, value_start, kWhitespace); + ++value_end; } // OK, we're finished with a Token/Value. diff --git a/net/base/cookie_monster.h b/net/base/cookie_monster.h index fef0e8f..c9e48d7 100644 --- a/net/base/cookie_monster.h +++ b/net/base/cookie_monster.h @@ -265,6 +265,10 @@ class CookieMonster::ParsedCookie { bool IsSecure() const { return secure_index_ != 0; } bool IsHttpOnly() const { return httponly_index_ != 0; } + // Return the number of attributes, for example, returning 2 for: + // "BLAH=hah; path=/; domain=.google.com" + size_t NumberOfAttributes() const { return pairs_.size() - 1; } + // For debugging only! std::string DebugString() const; diff --git a/net/base/cookie_monster_unittest.cc b/net/base/cookie_monster_unittest.cc index fcf8dbc..1d5d963 100644 --- a/net/base/cookie_monster_unittest.cc +++ b/net/base/cookie_monster_unittest.cc @@ -32,16 +32,47 @@ TEST(ParsedCookieTest, TestBasic) { } TEST(ParsedCookieTest, TestQuoted) { - net::CookieMonster::ParsedCookie pc("a=\"b=;\"; path=\"/\""); - EXPECT_TRUE(pc.IsValid()); - EXPECT_FALSE(pc.IsSecure()); - EXPECT_TRUE(pc.HasPath()); - EXPECT_EQ("a", pc.Name()); - EXPECT_EQ("\"b=;\"", pc.Value()); - // If a path was quoted, the path attribute keeps the quotes. This will - // make the cookie effectively useless, but path parameters aren't supposed - // to be quoted. Bug 1261605. - EXPECT_EQ("\"/\"", pc.Path()); + // These are some quoting cases which the major browsers all + // handle differently. I've tested Internet Explorer 6, Opera 9.6, + // Firefox 3, and Safari Windows 3.2.1. We originally tried to match + // Firefox closely, however we now match Internet Explorer and Safari. + const char* values[] = { + // Trailing whitespace after a quoted value. The whitespace after + // the quote is stripped in all browsers. + "\"zzz \" ", "\"zzz \"", + // Handling a quoted value with a ';', like FOO="zz;pp" ; + // IE and Safari: "zz; + // Firefox and Opera: "zz;pp" + "\"zz;pp\" ;", "\"zz", + // Handling a value with multiple quoted parts, like FOO="zzz " "ppp" ; + // IE and Safari: "zzz " "ppp"; + // Firefox: "zzz "; + // Opera: <rejects cookie> + "\"zzz \" \"ppp\" ", "\"zzz \" \"ppp\"", + // A quote in a value that didn't start quoted. like FOO=A"B ; + // IE, Safari, and Firefox: A"B; + // Opera: <rejects cookie> + "A\"B", "A\"B", + }; + + for (size_t i = 0; i < arraysize(values); i += 2) { + std::string input(values[i]); + std::string expected(values[i + 1]); + + net::CookieMonster::ParsedCookie pc( + "aBc=" + input + " ; path=\"/\" ; httponly "); + EXPECT_TRUE(pc.IsValid()); + EXPECT_FALSE(pc.IsSecure()); + EXPECT_TRUE(pc.IsHttpOnly()); + EXPECT_TRUE(pc.HasPath()); + EXPECT_EQ("aBc", pc.Name()); + EXPECT_EQ(expected, pc.Value()); + + // If a path was quoted, the path attribute keeps the quotes. This will + // make the cookie effectively useless, but path parameters aren't supposed + // to be quoted. Bug 1261605. + EXPECT_EQ("\"/\"", pc.Path()); + } } TEST(ParsedCookieTest, TestNameless) { @@ -63,6 +94,7 @@ TEST(ParsedCookieTest, TestAttributeCase) { EXPECT_EQ("/", pc.Path()); EXPECT_EQ("", pc.Name()); EXPECT_EQ("BLAHHH", pc.Value()); + EXPECT_EQ(3U, pc.NumberOfAttributes()); } TEST(ParsedCookieTest, TestDoubleQuotedNameless) { @@ -73,6 +105,7 @@ TEST(ParsedCookieTest, TestDoubleQuotedNameless) { EXPECT_EQ("/", pc.Path()); EXPECT_EQ("", pc.Name()); EXPECT_EQ("\"BLA\\\"HHH\"", pc.Value()); + EXPECT_EQ(2U, pc.NumberOfAttributes()); } TEST(ParsedCookieTest, QuoteOffTheEnd) { @@ -80,6 +113,7 @@ TEST(ParsedCookieTest, QuoteOffTheEnd) { EXPECT_TRUE(pc.IsValid()); EXPECT_EQ("a", pc.Name()); EXPECT_EQ("\"B", pc.Value()); + EXPECT_EQ(0U, pc.NumberOfAttributes()); } TEST(ParsedCookieTest, MissingName) { @@ -87,6 +121,7 @@ TEST(ParsedCookieTest, MissingName) { EXPECT_TRUE(pc.IsValid()); EXPECT_EQ("", pc.Name()); EXPECT_EQ("ABC", pc.Value()); + EXPECT_EQ(0U, pc.NumberOfAttributes()); } TEST(ParsedCookieTest, MissingValue) { @@ -96,6 +131,7 @@ TEST(ParsedCookieTest, MissingValue) { EXPECT_EQ("", pc.Value()); EXPECT_TRUE(pc.HasPath()); EXPECT_EQ("/wee", pc.Path()); + EXPECT_EQ(1U, pc.NumberOfAttributes()); } TEST(ParsedCookieTest, Whitespace) { @@ -107,6 +143,9 @@ TEST(ParsedCookieTest, Whitespace) { EXPECT_FALSE(pc.HasDomain()); EXPECT_TRUE(pc.IsSecure()); EXPECT_TRUE(pc.IsHttpOnly()); + // We parse anything between ; as attributes, so we end up with two + // attributes with an empty string name and value. + EXPECT_EQ(4U, pc.NumberOfAttributes()); } TEST(ParsedCookieTest, MultipleEquals) { net::CookieMonster::ParsedCookie pc(" A=== BC ;secure;;; httponly"); @@ -117,19 +156,34 @@ TEST(ParsedCookieTest, MultipleEquals) { EXPECT_FALSE(pc.HasDomain()); EXPECT_TRUE(pc.IsSecure()); EXPECT_TRUE(pc.IsHttpOnly()); + EXPECT_EQ(4U, pc.NumberOfAttributes()); +} + +TEST(ParsedCookieTest, QuotedTrailingWhitespace) { + net::CookieMonster::ParsedCookie pc("ANCUUID=\"zohNumRKgI0oxyhSsV3Z7D\" ; " + "expires=Sun, 18-Apr-2027 21:06:29 GMT ; " + "path=/ ; "); + EXPECT_TRUE(pc.IsValid()); + EXPECT_EQ("ANCUUID", pc.Name()); + // Stripping whitespace after the quotes matches all other major browsers. + EXPECT_EQ("\"zohNumRKgI0oxyhSsV3Z7D\"", pc.Value()); + EXPECT_TRUE(pc.HasExpires()); + EXPECT_TRUE(pc.HasPath()); + EXPECT_EQ("/", pc.Path()); + EXPECT_EQ(2U, pc.NumberOfAttributes()); } TEST(ParsedCookieTest, TrailingWhitespace) { - net::CookieMonster::ParsedCookie pc("ANCUUID=zohNumRKgI0oxyhSsV3Z7D; " - "expires=Sun, 18-Apr-2027 21:06:29 GMT; " + net::CookieMonster::ParsedCookie pc("ANCUUID=zohNumRKgI0oxyhSsV3Z7D ; " + "expires=Sun, 18-Apr-2027 21:06:29 GMT ; " "path=/ ; "); EXPECT_TRUE(pc.IsValid()); EXPECT_EQ("ANCUUID", pc.Name()); + EXPECT_EQ("zohNumRKgI0oxyhSsV3Z7D", pc.Value()); EXPECT_TRUE(pc.HasExpires()); EXPECT_TRUE(pc.HasPath()); EXPECT_EQ("/", pc.Path()); - // TODO should export like NumAttributes() and make sure that the - // trailing whitespace doesn't end up as an empty attribute or something. + EXPECT_EQ(2U, pc.NumberOfAttributes()); } TEST(ParsedCookieTest, TooManyPairs) { |