Update cookie value and attribute setting behavior to match other UAs

net/cookies/parsed_cookie_unittest.cc

Index: net/cookies/parsed_cookie_unittest.cc
diff --git a/net/cookies/parsed_cookie_unittest.cc b/net/cookies/parsed_cookie_unittest.cc
index 27efb99adfcea226c2a12a378b89feda74a7c408..cdb6ae5fcd8d879c36fd5de927797e520347b1ea 100644
--- a/net/cookies/parsed_cookie_unittest.cc
+++ b/net/cookies/parsed_cookie_unittest.cc
@@ -18,19 +18,29 @@ TEST(ParsedCookieTest, TestBasic) {
   EXPECT_EQ("b", pc.Value());
 }
 
+// De facto standard behavior, per https://crbug.com/601786.
 TEST(ParsedCookieTest, TestEmpty) {
-  ParsedCookie pc1("=; path=/; secure;");
-  EXPECT_FALSE(pc1.IsValid());
-  ParsedCookie pc2("= ; path=/; secure;");
-  EXPECT_FALSE(pc2.IsValid());
-  ParsedCookie pc3(" =; path=/; secure;");
-  EXPECT_FALSE(pc3.IsValid());
-  ParsedCookie pc4(" = ; path=/; secure;");
-  EXPECT_FALSE(pc4.IsValid());
-  ParsedCookie pc5(" ; path=/; secure;");
-  EXPECT_FALSE(pc5.IsValid());
-  ParsedCookie pc6("; path=/; secure;");
-  EXPECT_FALSE(pc6.IsValid());
+  struct {

[mmenke, 2016/08/16 15:06:47]

const

[jww, 2016/08/16 18:18:17]

Done.

+    const char* cookie;
+    const char* expected_path;
+    bool expect_secure;
+  } test_cookie_lines[]{

[mmenke, 2016/08/16 15:06:47]

kTestCookieLines

[jww, 2016/08/16 18:18:17]

Done.

+      {"", "", false},     {"     ", "", false},
+      {"=;", "", false},   {"=; path=/; secure;", "/", true},
+      {"= ;", "", false},  {"= ; path=/; secure;", "/", true},
+      {" =;", "", false},  {" =; path=/; secure;", "/", true},
+      {" = ;", "", false}, {" = ; path=/; secure;", "/", true},
+      {" ;", "", false},   {" ; path=/; secure;", "/", true},
+      {";", "", false},    {"; path=/; secure;", "/", true},
+      {"\t;", "", false},  {"\t; path=/; secure;", "/", true}};

[mmenke, 2016/08/16 15:06:47]

Suggest a blank line, for readability.

[jww, 2016/08/16 18:18:17]

Done.

+  for (size_t i = 0; i < arraysize(test_cookie_lines); i++) {

[mmenke, 2016/08/16 15:06:47]

Can just do a range-based loop (Or does it require that we have a named type? 
If so, I'd just add a name for the type in the array)

for (const auto& test : test_cookie_lines)

[jww, 2016/08/16 18:18:17]

Done.

+    ParsedCookie pc(test_cookie_lines[i].cookie);
+    EXPECT_TRUE(pc.IsValid());
+    EXPECT_EQ("", pc.Name());
+    EXPECT_EQ("", pc.Value());
+    EXPECT_EQ(test_cookie_lines[i].expected_path, pc.Path());
+    EXPECT_EQ(test_cookie_lines[i].expect_secure, pc.IsSecure());
+  }
 }
 
 TEST(ParsedCookieTest, TestQuoted) {
@@ -221,11 +231,6 @@ TEST(ParsedCookieTest, TooManyPairs) {
 }
 
 // TODO(erikwright): some better test cases for invalid cookies.
-TEST(ParsedCookieTest, InvalidWhitespace) {
-  ParsedCookie pc("    ");
-  EXPECT_FALSE(pc.IsValid());
-}
-
 TEST(ParsedCookieTest, InvalidTooLong) {
   std::string maxstr;
   maxstr.resize(ParsedCookie::kMaxCookieSize, 'a');
@@ -237,11 +242,6 @@ TEST(ParsedCookieTest, InvalidTooLong) {
   EXPECT_FALSE(pc2.IsValid());
 }
 
-TEST(ParsedCookieTest, InvalidEmpty) {
-  ParsedCookie pc((std::string()));
-  EXPECT_FALSE(pc.IsValid());
-}
-
 TEST(ParsedCookieTest, EmbeddedTerminator) {
   ParsedCookie pc1("AAA=BB\0ZYX");
   ParsedCookie pc2("AAA=BB\rZYX");
@@ -285,11 +285,11 @@ TEST(ParsedCookieTest, SerializeCookieLine) {
 
 TEST(ParsedCookieTest, SetNameAndValue) {
   ParsedCookie empty((std::string()));
-  EXPECT_FALSE(empty.IsValid());
-  EXPECT_FALSE(empty.SetDomain("foobar.com"));
+  EXPECT_TRUE(empty.IsValid());
+  EXPECT_TRUE(empty.SetDomain("foobar.com"));
   EXPECT_TRUE(empty.SetName("name"));
   EXPECT_TRUE(empty.SetValue("value"));
-  EXPECT_EQ("name=value", empty.ToCookieLine());
+  EXPECT_EQ("name=value; domain=foobar.com", empty.ToCookieLine());
   EXPECT_TRUE(empty.IsValid());
 
   // We don't test
@@ -307,10 +307,6 @@ TEST(ParsedCookieTest, SetNameAndValue) {
   EXPECT_EQ("name=value", pc.ToCookieLine());
   EXPECT_TRUE(pc.IsValid());
 
-  EXPECT_FALSE(pc.SetName(std::string()));
-  EXPECT_EQ("name=value", pc.ToCookieLine());
-  EXPECT_TRUE(pc.IsValid());
-
   EXPECT_FALSE(pc.SetValue("foo bar"));
   EXPECT_EQ("name=value", pc.ToCookieLine());
   EXPECT_TRUE(pc.IsValid());
@@ -320,6 +316,10 @@ TEST(ParsedCookieTest, SetNameAndValue) {
   EXPECT_TRUE(pc.IsValid());
 
   // Set valid name / value
+  EXPECT_TRUE(pc.SetName(std::string()));
+  EXPECT_EQ("=value", pc.ToCookieLine());
+  EXPECT_TRUE(pc.IsValid());
+
   EXPECT_TRUE(pc.SetName("test"));
   EXPECT_EQ("test=value", pc.ToCookieLine());
   EXPECT_TRUE(pc.IsValid());
@@ -415,6 +415,17 @@ TEST(ParsedCookieTest, SetAttributes) {
   EXPECT_EQ("name2=value2", pc.ToCookieLine());
 }
 
+// Set the domain attribute twice in a cookie line. If the second attribute's
+// value is empty, it shoud be ignored.
+//
+// This is de facto standard behavior, per https://crbug.com/601786.
+TEST(ParsedCookieTest, MultipleDomainAttributes) {
+  ParsedCookie pc1("name=value; domain=foo.com; domain=bar.com");
+  EXPECT_EQ("bar.com", pc1.Domain());
+  ParsedCookie pc2("name=value; domain=foo.com; domain=");
+  EXPECT_EQ("foo.com", pc2.Domain());
+}
+
 TEST(ParsedCookieTest, SetPriority) {
   ParsedCookie pc("name=value");
   EXPECT_TRUE(pc.IsValid());