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..7cda724404954035b56e895bb05664352f57f009 100644
--- a/net/cookies/parsed_cookie_unittest.cc
+++ b/net/cookies/parsed_cookie_unittest.cc
@@ -18,19 +18,22 @@ 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());
+  EXPECT_TRUE(pc1.IsValid());
   ParsedCookie pc2("= ; path=/; secure;");
-  EXPECT_FALSE(pc2.IsValid());
+  EXPECT_TRUE(pc2.IsValid());
   ParsedCookie pc3(" =; path=/; secure;");
-  EXPECT_FALSE(pc3.IsValid());
+  EXPECT_TRUE(pc3.IsValid());
   ParsedCookie pc4(" = ; path=/; secure;");
-  EXPECT_FALSE(pc4.IsValid());
+  EXPECT_TRUE(pc4.IsValid());
   ParsedCookie pc5(" ; path=/; secure;");
-  EXPECT_FALSE(pc5.IsValid());
+  EXPECT_TRUE(pc5.IsValid());
   ParsedCookie pc6("; path=/; secure;");
-  EXPECT_FALSE(pc6.IsValid());
+  EXPECT_TRUE(pc6.IsValid());
+  ParsedCookie pc7("\t; path=/; secure;");
+  EXPECT_TRUE(pc7.IsValid());

[mmenke, 2016/08/15 21:11:03]

We should check the actual value, too, right?  Also should test that they have
path and secure set (Except in the two cases I suggested adding)

[mmenke, 2016/08/15 21:11:03]

We should also check ";" and "=" (With no extra stuff on the end).

[mmenke, 2016/08/15 21:11:03]

optional: Suggest merging ValidOnlyWhitespace and ValidEmpty with this test,
too, since they're so closely related.

[jww, 2016/08/16 02:24:23]

Good calls all around. I've done all of your suggestions:
  * Added versions of all the tests without the attributes
  * Added value checking for each of them
  * Moved ValidOnlyWhitespace and ValidEmpty into TestEmpty

 }
 
 TEST(ParsedCookieTest, TestQuoted) {
@@ -221,11 +224,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,9 +235,16 @@ TEST(ParsedCookieTest, InvalidTooLong) {
   EXPECT_FALSE(pc2.IsValid());
 }
 
-TEST(ParsedCookieTest, InvalidEmpty) {
+// De facto standard behavior, per https://crbug.com/601786.
+TEST(ParsedCookieTest, ValidOnlyWhitespace) {
+  ParsedCookie pc("    ");
+  EXPECT_TRUE(pc.IsValid());
+}
+
+// De facto standard behavior, per https://crbug.com/601786.
+TEST(ParsedCookieTest, ValidEmpty) {
   ParsedCookie pc((std::string()));
-  EXPECT_FALSE(pc.IsValid());
+  EXPECT_TRUE(pc.IsValid());
 }
 
 TEST(ParsedCookieTest, EmbeddedTerminator) {
@@ -285,11 +290,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
@@ -415,6 +420,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());