Parsing cookie SameSite: add None & Extended options, change default

This CL changes the default net::CookieSameSite value to UNSPECIFIED
(it was previously NO_RESTRICTION). The default is used when the cookie
does not specify a SameSite attribute, or if the given SameSite
attribute does not specify a valid value.

Support is added for the new `SameSite=None` option, which parses as
CookieSameSite::NO_RESTRICTION. Support is also added for the new
`SameSite=Extended` option, which for now is identical to Lax mode.

Bug: 953306, 953995
Change-Id: I96ee07f5e626a2f99e65e9c27260a8f6e5b6b9f4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1573081
Commit-Queue: Lily Chen <chlily@chromium.org>
Reviewed-by: Maks Orlovich <morlovich@chromium.org>
Cr-Commit-Position: refs/heads/master@{#652529}

net/cookies/canonical_cookie.h

@@ -139,8 +139,8 @@ class NET_EXPORT CanonicalCookie {
   // having been canonicalized (in
   // GetCookieDomainWithString->CanonicalizeHost).
   bool IsEquivalent(const CanonicalCookie& ecc) const {
-    // It seems like it would make sense to take secure and httponly into
-    // account, but the RFC doesn't specify this.
+    // It seems like it would make sense to take secure, httponly, and samesite
+    // into account, but the RFC doesn't specify this.
     // NOTE: Keep this logic in-sync with TrimDuplicateCookiesForHost().
     return (name_ == ecc.Name() && domain_ == ecc.Domain()
             && path_ == ecc.Path());

net/cookies/canonical_cookie_unittest.cc

@@ -130,6 +130,18 @@ TEST(CanonicalCookieTest, Create) {
                                    same_site_options);
   ASSERT_TRUE(cookie.get());
   EXPECT_EQ(CookieSameSite::LAX_MODE, cookie->SameSite());
+  cookie = CanonicalCookie::Create(url, "A=2; SameSite=Extended", creation_time,
+                                   same_site_options);
+  ASSERT_TRUE(cookie.get());
+  EXPECT_EQ(CookieSameSite::EXTENDED_MODE, cookie->SameSite());
+  cookie = CanonicalCookie::Create(url, "A=2; SameSite=None", creation_time,
+                                   same_site_options);
+  ASSERT_TRUE(cookie.get());
+  EXPECT_EQ(CookieSameSite::NO_RESTRICTION, cookie->SameSite());
+  cookie =
+      CanonicalCookie::Create(url, "A=2", creation_time, same_site_options);
+  ASSERT_TRUE(cookie.get());
+  EXPECT_EQ(CookieSameSite::UNSPECIFIED, cookie->SameSite());
 
   // Test the creating cookies using specific parameter instead of a cookie
   // string.
@@ -171,12 +183,12 @@ TEST(CanonicalCookieTest, CreateNonStandardSameSite) {
   cookie =
       CanonicalCookie::Create(url, "A=2; SameSite=NonStandard", now, options);
   EXPECT_TRUE(cookie.get());
-  EXPECT_EQ(CookieSameSite::NO_RESTRICTION, cookie->SameSite());
+  EXPECT_EQ(CookieSameSite::UNSPECIFIED, cookie->SameSite());
 
   // Omit value for the SameSite attribute.
   cookie = CanonicalCookie::Create(url, "A=2; SameSite", now, options);
   EXPECT_TRUE(cookie.get());
-  EXPECT_EQ(CookieSameSite::NO_RESTRICTION, cookie->SameSite());
+  EXPECT_EQ(CookieSameSite::UNSPECIFIED, cookie->SameSite());
 }
 
 TEST(CanonicalCookieTest, CreateInvalidHttpOnly) {
@@ -568,10 +580,29 @@ TEST(CanonicalCookieTest, IncludeSameSiteForSameSiteURL) {
   EXPECT_EQ(cookie->IncludeForRequestURL(url, options),
             CanonicalCookie::CookieInclusionStatus::INCLUDE);
 
-  // TODO(chlily): Use SameSite=None once ParsedCookie supports it.
-  // Cookies without a SameSite attribute are parsed as
-  // CookieSameSite::NO_RESTRICTION, and are included for all URLs.
-  cookie = CanonicalCookie::Create(url, "A=2", creation_time, options);
+  // `SameSite=Extended` cookies are included for a URL only if the options'
+  // SameSiteCookieMode is SAME_SITE_STRICT or SAME_SITE_LAX.
+  // TODO(crbug.com/953995): Right now Extended behaves the same as Lax.
+  // Implement Extended.
+  cookie = CanonicalCookie::Create(url, "A=2; SameSite=Extended", creation_time,
+                                   options);
+  EXPECT_EQ(CookieSameSite::EXTENDED_MODE, cookie->SameSite());
+  options.set_same_site_cookie_context(
+      CookieOptions::SameSiteCookieContext::CROSS_SITE);
+  EXPECT_EQ(cookie->IncludeForRequestURL(url, options),
+            CanonicalCookie::CookieInclusionStatus::EXCLUDE_SAMESITE_LAX);
+  options.set_same_site_cookie_context(
+      CookieOptions::SameSiteCookieContext::SAME_SITE_LAX);
+  EXPECT_EQ(cookie->IncludeForRequestURL(url, options),
+            CanonicalCookie::CookieInclusionStatus::INCLUDE);
+  options.set_same_site_cookie_context(
+      CookieOptions::SameSiteCookieContext::SAME_SITE_STRICT);
+  EXPECT_EQ(cookie->IncludeForRequestURL(url, options),
+            CanonicalCookie::CookieInclusionStatus::INCLUDE);
+
+  // `SameSite=None` cookies are included for all URLs.
+  cookie = CanonicalCookie::Create(url, "A=2; SameSite=None", creation_time,
+                                   options);
   EXPECT_EQ(CookieSameSite::NO_RESTRICTION, cookie->SameSite());
   options.set_same_site_cookie_context(
       CookieOptions::SameSiteCookieContext::CROSS_SITE);
@@ -586,11 +617,9 @@ TEST(CanonicalCookieTest, IncludeSameSiteForSameSiteURL) {
   EXPECT_EQ(cookie->IncludeForRequestURL(url, options),
             CanonicalCookie::CookieInclusionStatus::INCLUDE);
 
-  // TODO(chlily): Use Create() once ParsedCookie supports UNSPECIFIED.
-  // Cookies with CookieSameSite::UNSPECIFIED depend on the FeatureList.
-  cookie = std::make_unique<CanonicalCookie>(
-      "A", "2", "example.test", "/", creation_time, base::Time(), base::Time(),
-      false, false, CookieSameSite::UNSPECIFIED, COOKIE_PRIORITY_DEFAULT);
+  // Cookies with no SameSite attribute are parsed as
+  // CookieSameSite::UNSPECIFIED, and inclusion depends on the FeatureList.
+  cookie = CanonicalCookie::Create(url, "A=2", creation_time, options);
   EXPECT_EQ(CookieSameSite::UNSPECIFIED, cookie->SameSite());
   {
     base::test::ScopedFeatureList feature_list;

net/cookies/cookie_constants.cc

@@ -67,11 +67,15 @@ std::string CookieSameSiteToString(CookieSameSite same_site) {
 }
 
 CookieSameSite StringToCookieSameSite(const std::string& same_site) {
+  if (base::EqualsCaseInsensitiveASCII(same_site, kSameSiteNone))
+    return CookieSameSite::NO_RESTRICTION;
   if (base::EqualsCaseInsensitiveASCII(same_site, kSameSiteLax))
     return CookieSameSite::LAX_MODE;
   if (base::EqualsCaseInsensitiveASCII(same_site, kSameSiteStrict))
     return CookieSameSite::STRICT_MODE;
-  return CookieSameSite::NO_RESTRICTION;
+  if (base::EqualsCaseInsensitiveASCII(same_site, kSameSiteExtended))
+    return CookieSameSite::EXTENDED_MODE;
+  return CookieSameSite::UNSPECIFIED;
 }
 
 }  // namespace net

net/cookies/cookie_constants.h

@@ -44,7 +44,7 @@ NET_EXPORT CookiePriority StringToCookiePriority(const std::string& priority);
 NET_EXPORT std::string CookieSameSiteToString(CookieSameSite same_site);
 
 // Converts the Set-Cookie header SameSite token |same_site| to a
-// CookieSameSite. Defaults to CookieSameSite::NO_RESTRICTION for empty or
+// CookieSameSite. Defaults to CookieSameSite::UNSPECIFIED for empty or
 // unrecognized strings.
 NET_EXPORT CookieSameSite StringToCookieSameSite(const std::string& same_site);
 

net/cookies/cookie_constants_unittest.cc

@@ -38,4 +38,27 @@ TEST(CookieConstantsTest, TestCookiePriority) {
   }
 }
 
+TEST(CookieConstantsTest, TestCookieSameSite) {
+  // Test case insensitivity
+  EXPECT_EQ(CookieSameSite::NO_RESTRICTION, StringToCookieSameSite("None"));
+  EXPECT_EQ(CookieSameSite::NO_RESTRICTION, StringToCookieSameSite("none"));
+  EXPECT_EQ(CookieSameSite::NO_RESTRICTION, StringToCookieSameSite("NONE"));
+  EXPECT_EQ(CookieSameSite::LAX_MODE, StringToCookieSameSite("Lax"));
+  EXPECT_EQ(CookieSameSite::LAX_MODE, StringToCookieSameSite("LAX"));
+  EXPECT_EQ(CookieSameSite::LAX_MODE, StringToCookieSameSite("lAx"));
+  EXPECT_EQ(CookieSameSite::STRICT_MODE, StringToCookieSameSite("Strict"));
+  EXPECT_EQ(CookieSameSite::STRICT_MODE, StringToCookieSameSite("STRICT"));
+  EXPECT_EQ(CookieSameSite::STRICT_MODE, StringToCookieSameSite("sTrIcT"));
+  EXPECT_EQ(CookieSameSite::EXTENDED_MODE, StringToCookieSameSite("extended"));
+  EXPECT_EQ(CookieSameSite::EXTENDED_MODE, StringToCookieSameSite("EXTENDED"));
+  EXPECT_EQ(CookieSameSite::EXTENDED_MODE, StringToCookieSameSite("ExtenDED"));
+
+  // Unrecognized tokens are interpreted as UNSPECIFIED.
+  const char* const bad_tokens[] = {"",          "foo",   "none ",
+                                    "strictest", " none", "0"};
+  for (const auto* bad_token : bad_tokens) {
+    EXPECT_EQ(CookieSameSite::UNSPECIFIED, StringToCookieSameSite(bad_token));
+  }
+}
+
 }  // namespace net

net/cookies/parsed_cookie.cc

@@ -155,7 +155,7 @@ bool ParsedCookie::IsValid() const {
 
 CookieSameSite ParsedCookie::SameSite() const {
   return (same_site_index_ == 0)
-             ? CookieSameSite::NO_RESTRICTION
+             ? CookieSameSite::UNSPECIFIED
              : StringToCookieSameSite(pairs_[same_site_index_].second);
 }
 
@@ -207,8 +207,8 @@ bool ParsedCookie::SetIsHttpOnly(bool is_http_only) {
   return SetBool(&httponly_index_, kHttpOnlyTokenName, is_http_only);
 }
 
-bool ParsedCookie::SetSameSite(const std::string& is_same_site) {
-  return SetString(&same_site_index_, kSameSiteTokenName, is_same_site);
+bool ParsedCookie::SetSameSite(const std::string& same_site) {
+  return SetString(&same_site_index_, kSameSiteTokenName, same_site);
 }
 
 bool ParsedCookie::SetPriority(const std::string& priority) {

net/cookies/parsed_cookie_unittest.cc

@@ -181,7 +181,7 @@ TEST(ParsedCookieTest, MultipleEquals) {
   EXPECT_FALSE(pc.HasDomain());
   EXPECT_TRUE(pc.IsSecure());
   EXPECT_TRUE(pc.IsHttpOnly());
-  EXPECT_EQ(CookieSameSite::NO_RESTRICTION, pc.SameSite());
+  EXPECT_EQ(CookieSameSite::UNSPECIFIED, pc.SameSite());
   EXPECT_EQ(COOKIE_PRIORITY_DEFAULT, pc.Priority());
   EXPECT_EQ(4U, pc.NumberOfAttributes());
 }
@@ -411,7 +411,7 @@ TEST(ParsedCookieTest, SetAttributes) {
   EXPECT_FALSE(pc.HasMaxAge());
   EXPECT_FALSE(pc.IsSecure());
   EXPECT_FALSE(pc.IsHttpOnly());
-  EXPECT_EQ(CookieSameSite::NO_RESTRICTION, pc.SameSite());
+  EXPECT_EQ(CookieSameSite::UNSPECIFIED, pc.SameSite());
   EXPECT_EQ("name2=value2", pc.ToCookieLine());
 }
 
@@ -465,7 +465,7 @@ TEST(ParsedCookieTest, SetSameSite) {
   EXPECT_TRUE(pc.IsValid());
 
   EXPECT_EQ("name=value", pc.ToCookieLine());
-  EXPECT_EQ(CookieSameSite::NO_RESTRICTION, pc.SameSite());
+  EXPECT_EQ(CookieSameSite::UNSPECIFIED, pc.SameSite());
 
   // Test each samesite directive, expect case-insensitive compare.
   EXPECT_TRUE(pc.SetSameSite("strict"));
@@ -483,15 +483,25 @@ TEST(ParsedCookieTest, SetSameSite) {
   EXPECT_EQ(CookieSameSite::LAX_MODE, pc.SameSite());
   EXPECT_TRUE(pc.IsValid());
 
+  EXPECT_TRUE(pc.SetSameSite("None"));
+  EXPECT_EQ("name=value; samesite=None", pc.ToCookieLine());
+  EXPECT_EQ(CookieSameSite::NO_RESTRICTION, pc.SameSite());
+  EXPECT_TRUE(pc.IsValid());
+
+  EXPECT_TRUE(pc.SetSameSite("NONE"));
+  EXPECT_EQ("name=value; samesite=NONE", pc.ToCookieLine());
+  EXPECT_EQ(CookieSameSite::NO_RESTRICTION, pc.SameSite());
+  EXPECT_TRUE(pc.IsValid());
+
   // Remove the SameSite attribute.
   EXPECT_TRUE(pc.SetSameSite(""));
   EXPECT_EQ("name=value", pc.ToCookieLine());
-  EXPECT_EQ(CookieSameSite::NO_RESTRICTION, pc.SameSite());
+  EXPECT_EQ(CookieSameSite::UNSPECIFIED, pc.SameSite());
   EXPECT_TRUE(pc.IsValid());
 
   EXPECT_TRUE(pc.SetSameSite("Blah"));
   EXPECT_EQ("name=value; samesite=Blah", pc.ToCookieLine());
-  EXPECT_EQ(CookieSameSite::NO_RESTRICTION, pc.SameSite());
+  EXPECT_EQ(CookieSameSite::UNSPECIFIED, pc.SameSite());
   EXPECT_TRUE(pc.IsValid());
 }
 
@@ -502,9 +512,10 @@ TEST(ParsedCookieTest, SameSiteValues) {
     CookieSameSite mode;
   } cases[]{{"n=v; samesite=strict", true, CookieSameSite::STRICT_MODE},
             {"n=v; samesite=lax", true, CookieSameSite::LAX_MODE},
-            {"n=v; samesite=boo", true, CookieSameSite::NO_RESTRICTION},
-            {"n=v; samesite", true, CookieSameSite::NO_RESTRICTION},
-            {"n=v", true, CookieSameSite::NO_RESTRICTION}};
+            {"n=v; samesite=none", true, CookieSameSite::NO_RESTRICTION},
+            {"n=v; samesite=boo", true, CookieSameSite::UNSPECIFIED},
+            {"n=v; samesite", true, CookieSameSite::UNSPECIFIED},
+            {"n=v", true, CookieSameSite::UNSPECIFIED}};
 
   for (const auto& test : cases) {
     SCOPED_TRACE(test.cookie);