Prevent invalid ParsedCookie modifications.

After https://chromium-review.googlesource.com/c/chromium/src/+/1982549,
we treat cookies with neither names nor values as invalid. This patch
adjusts `ParsedCookie::SetName()` and `ParsedCookie::SetValue()` to
ensure that a `ParsedCookie` can't be modified into an invalid cookie.
It also applies the rule to `CanonicalCookie::CreateSanitizedCookie`
to ensure that code path has the same restrictions.

Bug: 1040296
Change-Id: I1e57c3ee8a4f6dc222c35d6e9844928c1ba7fb06
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1993962
Commit-Queue: Mike West <mkwst@chromium.org>
Reviewed-by: Lily Chen <chlily@chromium.org>
Cr-Commit-Position: refs/heads/master@{#733943}

net/cookies/canonical_cookie.cc

@@ -348,6 +348,9 @@ std::unique_ptr<CanonicalCookie> CanonicalCookie::CreateSanitizedCookie(
     return nullptr;
   }
 
+  if (name.empty() && value.empty())
+    return nullptr;
+
   // This validation step must happen before GetCookieDomainWithString, so it
   // doesn't fail DCHECKs.
   if (!cookie_util::DomainIsHostOnly(url.host()))

net/cookies/canonical_cookie_unittest.cc

@@ -1804,6 +1804,12 @@ TEST(CanonicalCookieTest, CreateSanitizedCookie_Logic) {
   ASSERT_TRUE(cc);
   EXPECT_EQ("/foo%7F", cc->Path());
 
+  // Empty name and value.
+  EXPECT_FALSE(CanonicalCookie::CreateSanitizedCookie(
+      GURL("http://www.foo.com"), "", "", std::string(), "/", base::Time(),
+      base::Time(), base::Time(), false /*secure*/, false /*httponly*/,
+      CookieSameSite::NO_RESTRICTION, COOKIE_PRIORITY_DEFAULT));
+
   // A __Secure- cookie must be Secure.
   EXPECT_TRUE(CanonicalCookie::CreateSanitizedCookie(
       GURL("https://www.foo.com"), "__Secure-A", "B", ".www.foo.com", "/",
@@ -1898,18 +1904,16 @@ TEST(CanonicalCookieTest, CreateSanitizedCookie_Logic) {
   // Check that a file URL with an IPv6 host, and matching IPv6 domain, are
   // valid.
   EXPECT_TRUE(CanonicalCookie::CreateSanitizedCookie(
-      GURL("file://[A::]"), std::string(), std::string(), "[A::]", "",
-      base::Time(), base::Time(), base::Time(), false /*secure*/,
-      false /*httponly*/, CookieSameSite::NO_RESTRICTION,
-      COOKIE_PRIORITY_DEFAULT));
+      GURL("file://[A::]"), "A", "B", "[A::]", "", base::Time(), base::Time(),
+      base::Time(), false /*secure*/, false /*httponly*/,
+      CookieSameSite::NO_RESTRICTION, COOKIE_PRIORITY_DEFAULT));
 
   // On Windows, URLs beginning with two backslashes are considered file
   // URLs. On other platforms, they are invalid.
   auto double_backslash_ipv6_cookie = CanonicalCookie::CreateSanitizedCookie(
-      GURL("\\\\[A::]"), std::string(), std::string(), "[A::]", "",
-      base::Time(), base::Time(), base::Time(), false /*secure*/,
-      false /*httponly*/, CookieSameSite::NO_RESTRICTION,
-      COOKIE_PRIORITY_DEFAULT);
+      GURL("\\\\[A::]"), "A", "B", "[A::]", "", base::Time(), base::Time(),
+      base::Time(), false /*secure*/, false /*httponly*/,
+      CookieSameSite::NO_RESTRICTION, COOKIE_PRIORITY_DEFAULT);
 #if defined(OS_WIN)
   EXPECT_TRUE(double_backslash_ipv6_cookie);
   EXPECT_TRUE(double_backslash_ipv6_cookie->IsCanonical());

net/cookies/parsed_cookie.cc

@@ -175,6 +175,11 @@ CookiePriority ParsedCookie::Priority() const {
 bool ParsedCookie::SetName(const std::string& name) {
   if (!name.empty() && !HttpUtil::IsToken(name))
     return false;
+
+  // Fail if we'd be creating a cookie with an empty name and value.
+  if (name.empty() && (pairs_.empty() || pairs_[0].second.empty()))
+    return false;
+
   if (pairs_.empty())
     pairs_.push_back(std::make_pair("", ""));
   pairs_[0].first = name;
@@ -184,6 +189,11 @@ bool ParsedCookie::SetName(const std::string& name) {
 bool ParsedCookie::SetValue(const std::string& value) {
   if (!IsValidCookieValue(value))
     return false;
+
+  // Fail if we'd be creating a cookie with an empty name and value.
+  if (value.empty() && (pairs_.empty() || pairs_[0].first.empty()))
+    return false;
+
   if (pairs_.empty())
     pairs_.push_back(std::make_pair("", ""));
   pairs_[0].second = value;

net/cookies/parsed_cookie.h

@@ -63,6 +63,9 @@ class NET_EXPORT ParsedCookie {
   // The functions return false in case an error occurred.
   // The cookie needs to be assigned a name/value before setting the other
   // attributes.
+  //
+  // TODO(chlily): Ideally, we can remove these mutators once we remove the
+  // single callsite.
   bool SetName(const std::string& name);
   bool SetValue(const std::string& value);
   bool SetPath(const std::string& path);

net/cookies/parsed_cookie_unittest.cc

@@ -29,6 +29,28 @@ TEST(ParsedCookieTest, TestEmpty) {
   }
 }
 
+TEST(ParsedCookieTest, TestSetEmptyNameValue) {
+  ParsedCookie empty("");
+  EXPECT_FALSE(empty.IsValid());
+  EXPECT_FALSE(empty.SetName(""));
+  EXPECT_FALSE(empty.SetValue(""));
+  EXPECT_FALSE(empty.IsValid());
+
+  ParsedCookie empty_value("name=");
+  EXPECT_TRUE(empty_value.IsValid());
+  EXPECT_EQ("name", empty_value.Name());
+  EXPECT_FALSE(empty_value.SetName(""));
+  EXPECT_EQ("name", empty_value.Name());
+  EXPECT_TRUE(empty_value.IsValid());
+
+  ParsedCookie empty_name("value");
+  EXPECT_TRUE(empty_name.IsValid());
+  EXPECT_EQ("value", empty_name.Value());
+  EXPECT_FALSE(empty_name.SetValue(""));
+  EXPECT_EQ("value", empty_name.Value());
+  EXPECT_TRUE(empty_value.IsValid());
+}
+
 TEST(ParsedCookieTest, TestQuoted) {
   // These are some quoting cases which the major browsers all
   // handle differently.  I've tested Internet Explorer 6, Opera 9.6,