summaryrefslogtreecommitdiffstats
path: root/net/base
diff options
context:
space:
mode:
Diffstat (limited to 'net/base')
-rw-r--r--net/base/cookie_monster.cc60
-rw-r--r--net/base/cookie_monster.h4
-rw-r--r--net/base/cookie_monster_unittest.cc82
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) {