Add SameParty attribute to ParsedCookie

This adds parsing support for a "SameParty" cookie attribute, which is a
boolean attribute (similar to "Secure" or "HttpOnly") to ParsedCookie.
Nothing uses this attribute yet.

Bug: 1142606
Change-Id: Ib1543df24ceefb1bb67be7fefd436a322d278f32
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2500481Reviewed-by: Maksim Orlovich <morlovich@chromium.org>
Reviewed-by: Dan McArdle <dmcardle@chromium.org>
Commit-Queue: Lily Chen <chlily@chromium.org>
Cr-Commit-Position: refs/heads/master@{#821237}

net/cookies/parse_cookie_line_fuzzer.cc

@@ -25,7 +25,7 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
 
   // Call zero or one of ParsedCookie's mutator methods.  Should not call
   // anything other than SetName/SetValue when !IsValid().
-  const uint8_t action = data_provider.ConsumeIntegralInRange(0, 10);
+  const uint8_t action = data_provider.ConsumeIntegralInRange(0, 11);
   switch (action) {
     case 1:
       parsed_cookie.SetName(GetArbitraryString(&data_provider));
@@ -62,6 +62,9 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
       case 10:
         parsed_cookie.SetPriority(GetArbitraryString(&data_provider));
         break;
+      case 11:
+        parsed_cookie.SetIsSameParty(data_provider.ConsumeBool());
+        break;
     }
   }
 

net/cookies/parsed_cookie.cc

@@ -60,6 +60,7 @@ const char kSecureTokenName[] = "secure";
 const char kHttpOnlyTokenName[] = "httponly";
 const char kSameSiteTokenName[] = "samesite";
 const char kPriorityTokenName[] = "priority";
+const char kSamePartyTokenName[] = "sameparty";
 
 const char kTerminator[] = "\n\r\0";
 const int kTerminatorLen = sizeof(kTerminator) - 1;
@@ -129,15 +130,7 @@ bool IsControlCharacter(unsigned char c) {
 
 namespace net {
 
-ParsedCookie::ParsedCookie(const std::string& cookie_line)
-    : path_index_(0),
-      domain_index_(0),
-      expires_index_(0),
-      maxage_index_(0),
-      secure_index_(0),
-      httponly_index_(0),
-      same_site_index_(0),
-      priority_index_(0) {
+ParsedCookie::ParsedCookie(const std::string& cookie_line) {
   if (cookie_line.size() > kMaxCookieSize) {
     DVLOG(1) << "Not parsing cookie, too large: " << cookie_line.size();
     return;
@@ -232,6 +225,10 @@ bool ParsedCookie::SetPriority(const std::string& priority) {
   return SetString(&priority_index_, kPriorityTokenName, priority);
 }
 
+bool ParsedCookie::SetIsSameParty(bool is_same_party) {
+  return SetBool(&same_party_index_, kSamePartyTokenName, is_same_party);
+}
+
 std::string ParsedCookie::ToCookieLine() const {
   std::string out;
   for (auto it = pairs_.begin(); it != pairs_.end(); ++it) {
@@ -242,7 +239,8 @@ std::string ParsedCookie::ToCookieLine() const {
     // print it for the first pair(see crbug.com/977619). After the first pair,
     // we need to consider whether the name component is a special token.
     if (it == pairs_.begin() ||
-        (it->first != kSecureTokenName && it->first != kHttpOnlyTokenName)) {
+        (it->first != kSecureTokenName && it->first != kHttpOnlyTokenName &&
+         it->first != kSamePartyTokenName)) {
       out.append("=");
       out.append(it->second);
     }
@@ -464,6 +462,8 @@ void ParsedCookie::SetupAttributes() {
       same_site_index_ = i;
     } else if (pairs_[i].first == kPriorityTokenName) {
       priority_index_ = i;
+    } else if (pairs_[i].first == kSamePartyTokenName) {
+      same_party_index_ = i;
     } else {
       /* some attribute we don't know or don't care about. */
     }
@@ -531,7 +531,7 @@ void ParsedCookie::ClearAttributePair(size_t index) {
 
   size_t* indexes[] = {&path_index_,      &domain_index_,   &expires_index_,
                        &maxage_index_,    &secure_index_,   &httponly_index_,
-                       &same_site_index_, &priority_index_};
+                       &same_site_index_, &priority_index_, &same_party_index_};
   for (size_t* attribute_index : indexes) {
     if (*attribute_index == index)
       *attribute_index = 0;

net/cookies/parsed_cookie.h

@@ -53,6 +53,7 @@ class NET_EXPORT ParsedCookie {
   CookieSameSite SameSite(
       CookieSameSiteString* samesite_string = nullptr) const;
   CookiePriority Priority() const;
+  bool IsSameParty() const { return same_party_index_ != 0; }
 
   // Returns the number of attributes, for example, returning 2 for:
   //   "BLAH=hah; path=/; domain=.google.com"
@@ -76,6 +77,7 @@ class NET_EXPORT ParsedCookie {
   bool SetIsHttpOnly(bool is_http_only);
   bool SetSameSite(const std::string& same_site);
   bool SetPriority(const std::string& priority);
+  bool SetIsSameParty(bool is_same_party);
 
   // Returns the cookie description as it appears in a HTML response header.
   std::string ToCookieLine() const;
@@ -139,17 +141,16 @@ class NET_EXPORT ParsedCookie {
 
   PairList pairs_;
   // These will default to 0, but that should never be valid since the
-  // 0th index is the user supplied token/value, not an attribute.
-  // We're really never going to have more than like 8 attributes, so we
-  // could fit these into 3 bits each if we're worried about size...
-  size_t path_index_;
-  size_t domain_index_;
-  size_t expires_index_;
-  size_t maxage_index_;
-  size_t secure_index_;
-  size_t httponly_index_;
-  size_t same_site_index_;
-  size_t priority_index_;
+  // 0th index is the user supplied cookie name/value, not an attribute.
+  size_t path_index_ = 0;
+  size_t domain_index_ = 0;
+  size_t expires_index_ = 0;
+  size_t maxage_index_ = 0;
+  size_t secure_index_ = 0;
+  size_t httponly_index_ = 0;
+  size_t same_site_index_ = 0;
+  size_t priority_index_ = 0;
+  size_t same_party_index_ = 0;
 
   DISALLOW_COPY_AND_ASSIGN(ParsedCookie);
 };

net/cookies/parsed_cookie_unittest.cc

@@ -11,11 +11,38 @@
 namespace net {
 
 TEST(ParsedCookieTest, TestBasic) {
-  ParsedCookie pc("a=b");
-  EXPECT_TRUE(pc.IsValid());
-  EXPECT_FALSE(pc.IsSecure());
-  EXPECT_EQ("a", pc.Name());
-  EXPECT_EQ("b", pc.Value());
+  ParsedCookie pc1("a=b");
+  EXPECT_TRUE(pc1.IsValid());
+  EXPECT_FALSE(pc1.IsSecure());
+  EXPECT_FALSE(pc1.IsHttpOnly());
+  EXPECT_FALSE(pc1.IsSameParty());
+  EXPECT_EQ("a", pc1.Name());
+  EXPECT_EQ("b", pc1.Value());
+  EXPECT_FALSE(pc1.HasPath());
+  EXPECT_FALSE(pc1.HasDomain());
+  EXPECT_FALSE(pc1.HasExpires());
+  EXPECT_FALSE(pc1.HasMaxAge());
+  EXPECT_EQ(CookieSameSite::UNSPECIFIED, pc1.SameSite());
+  EXPECT_EQ(CookiePriority::COOKIE_PRIORITY_DEFAULT, pc1.Priority());
+
+  ParsedCookie pc2(
+      "c=d; secure; httponly; sameparty; path=/foo; domain=bar.test; "
+      "max-age=60; samesite=lax; priority=high");
+  EXPECT_TRUE(pc2.IsValid());
+  EXPECT_TRUE(pc2.IsSecure());
+  EXPECT_TRUE(pc2.IsHttpOnly());
+  EXPECT_TRUE(pc2.IsSameParty());
+  EXPECT_EQ("c", pc2.Name());
+  EXPECT_EQ("d", pc2.Value());
+  EXPECT_TRUE(pc2.HasPath());
+  EXPECT_EQ("/foo", pc2.Path());
+  EXPECT_TRUE(pc2.HasDomain());
+  EXPECT_EQ("bar.test", pc2.Domain());
+  EXPECT_FALSE(pc2.HasExpires());
+  EXPECT_TRUE(pc2.HasMaxAge());
+  EXPECT_EQ("60", pc2.MaxAge());
+  EXPECT_EQ(CookieSameSite::LAX_MODE, pc2.SameSite());
+  EXPECT_EQ(CookiePriority::COOKIE_PRIORITY_HIGH, pc2.Priority());
 }
 
 TEST(ParsedCookieTest, TestEmpty) {
@@ -111,17 +138,18 @@ TEST(ParsedCookieTest, TestNameless) {
 
 TEST(ParsedCookieTest, TestAttributeCase) {
   ParsedCookie pc(
-      "BLAHHH; Path=/; sECuRe; httpONLY; sAmESitE=StrIct; pRIoRitY=hIgH");
+      "BLAH; Path=/; sECuRe; httpONLY; sAmESitE=LaX; pRIoRitY=hIgH; samePaRtY");
   EXPECT_TRUE(pc.IsValid());
   EXPECT_TRUE(pc.IsSecure());
   EXPECT_TRUE(pc.IsHttpOnly());
-  EXPECT_EQ(CookieSameSite::STRICT_MODE, pc.SameSite());
+  EXPECT_TRUE(pc.IsSameParty());
+  EXPECT_EQ(CookieSameSite::LAX_MODE, pc.SameSite());
   EXPECT_TRUE(pc.HasPath());
   EXPECT_EQ("/", pc.Path());
   EXPECT_EQ("", pc.Name());
-  EXPECT_EQ("BLAHHH", pc.Value());
+  EXPECT_EQ("BLAH", pc.Value());
   EXPECT_EQ(COOKIE_PRIORITY_HIGH, pc.Priority());
-  EXPECT_EQ(5U, pc.NumberOfAttributes());
+  EXPECT_EQ(6U, pc.NumberOfAttributes());
 }
 
 TEST(ParsedCookieTest, TestDoubleQuotedNameless) {
@@ -367,10 +395,11 @@ TEST(ParsedCookieTest, SetAttributes) {
   EXPECT_TRUE(pc.SetIsHttpOnly(true));
   EXPECT_TRUE(pc.SetSameSite("LAX"));
   EXPECT_TRUE(pc.SetPriority("HIGH"));
+  EXPECT_TRUE(pc.SetIsSameParty(true));
   EXPECT_EQ(
       "name=value; domain=domain.com; path=/; "
       "expires=Sun, 18-Apr-2027 21:06:29 GMT; max-age=12345; secure; "
-      "httponly; samesite=LAX; priority=HIGH",
+      "httponly; samesite=LAX; priority=HIGH; sameparty",
       pc.ToCookieLine());
   EXPECT_TRUE(pc.HasDomain());
   EXPECT_TRUE(pc.HasPath());
@@ -380,22 +409,35 @@ TEST(ParsedCookieTest, SetAttributes) {
   EXPECT_TRUE(pc.IsHttpOnly());
   EXPECT_EQ(CookieSameSite::LAX_MODE, pc.SameSite());
   EXPECT_EQ(COOKIE_PRIORITY_HIGH, pc.Priority());
+  EXPECT_TRUE(pc.IsSameParty());
 
-  // Clear one attribute from the middle.
+  // Modify one attribute in the middle.
   EXPECT_TRUE(pc.SetPath("/foo"));
   EXPECT_TRUE(pc.HasDomain());
   EXPECT_TRUE(pc.HasPath());
+  EXPECT_EQ("/foo", pc.Path());
   EXPECT_TRUE(pc.HasExpires());
   EXPECT_TRUE(pc.IsSecure());
   EXPECT_TRUE(pc.IsHttpOnly());
+  EXPECT_TRUE(pc.IsSameParty());
   EXPECT_EQ(
       "name=value; domain=domain.com; path=/foo; "
       "expires=Sun, 18-Apr-2027 21:06:29 GMT; max-age=12345; secure; "
-      "httponly; samesite=LAX; priority=HIGH",
+      "httponly; samesite=LAX; priority=HIGH; sameparty",
       pc.ToCookieLine());
 
   // Set priority to medium.
   EXPECT_TRUE(pc.SetPriority("medium"));
+  EXPECT_EQ(CookiePriority::COOKIE_PRIORITY_MEDIUM, pc.Priority());
+  EXPECT_EQ(
+      "name=value; domain=domain.com; path=/foo; "
+      "expires=Sun, 18-Apr-2027 21:06:29 GMT; max-age=12345; secure; "
+      "httponly; samesite=LAX; priority=medium; sameparty",
+      pc.ToCookieLine());
+
+  // Clear attribute from the end.
+  EXPECT_TRUE(pc.SetIsSameParty(false));
+  EXPECT_FALSE(pc.IsSameParty());
   EXPECT_EQ(
       "name=value; domain=domain.com; path=/foo; "
       "expires=Sun, 18-Apr-2027 21:06:29 GMT; max-age=12345; secure; "
@@ -421,6 +463,7 @@ TEST(ParsedCookieTest, SetAttributes) {
   EXPECT_FALSE(pc.IsHttpOnly());
   EXPECT_EQ(CookieSameSite::UNSPECIFIED, pc.SameSite());
   EXPECT_EQ("name2=value2", pc.ToCookieLine());
+  EXPECT_FALSE(pc.IsSameParty());
 }
 
 // Set the domain attribute twice in a cookie line. If the second attribute's
@@ -570,8 +613,8 @@ TEST(ParsedCookieTest, SettersInputValidation) {
 }
 
 TEST(ParsedCookieTest, ToCookieLineSpecialTokens) {
-  // Special tokens "secure" and "httponly" should be treated as any other name
-  // when they are in the first position.
+  // Special tokens "secure", "httponly", and "sameparty" should be treated as
+  // any other name when they are in the first position.
   {
     ParsedCookie pc("");
     pc.SetName("secure");
@@ -593,6 +636,10 @@ TEST(ParsedCookieTest, ToCookieLineSpecialTokens) {
     ParsedCookie pc("httponly=foo");
     EXPECT_EQ(pc.ToCookieLine(), "httponly=foo");
   }
+  {
+    ParsedCookie pc("sameparty=foo");
+    EXPECT_EQ(pc.ToCookieLine(), "sameparty=foo");
+  }
   {
     ParsedCookie pc("foo");
     pc.SetName("secure");
@@ -621,10 +668,30 @@ TEST(ParsedCookieTest, ToCookieLineSpecialTokens) {
     ParsedCookie pc("name=foo; httponly=baz");
     EXPECT_EQ(pc.ToCookieLine(), "name=foo; httponly");
   }
+  {
+    ParsedCookie pc("name=foo; sameparty=baz");
+    EXPECT_EQ(pc.ToCookieLine(), "name=foo; sameparty");
+  }
   {
     ParsedCookie pc("name=foo; bar=secure");
     EXPECT_EQ(pc.ToCookieLine(), "name=foo; bar=secure");
   }
+  // Repeated instances of the special tokens are also fine.
+  {
+    ParsedCookie pc("name=foo; secure; secure=yesplease; secure; secure");
+    EXPECT_TRUE(pc.IsValid());
+    EXPECT_TRUE(pc.IsSecure());
+    EXPECT_FALSE(pc.IsHttpOnly());
+    EXPECT_FALSE(pc.IsSameParty());
+  }
+  {
+    ParsedCookie pc("sameparty; sameparty; secure; httponly; httponly; secure");
+    EXPECT_EQ("", pc.Name());
+    EXPECT_EQ("sameparty", pc.Value());
+    EXPECT_TRUE(pc.IsSecure());
+    EXPECT_TRUE(pc.IsSameParty());
+    EXPECT_TRUE(pc.IsHttpOnly());
+  }
 }
 
 TEST(ParsedCookieTest, SameSiteValues) {