refactor: improve cookie failure rejection messages (#42398)

Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <[email protected]>

shell/browser/api/electron_api_cookies.cc

@@ -171,25 +171,96 @@ base::Time ParseTimeProperty(const std::optional<double>& value) {
   return base::Time::FromSecondsSinceUnixEpoch(*value);
 }
 
-std::string_view InclusionStatusToString(net::CookieInclusionStatus status) {
-  if (status.HasExclusionReason(net::CookieInclusionStatus::EXCLUDE_HTTP_ONLY))
-    return "Failed to create httponly cookie";
-  if (status.HasExclusionReason(
-          net::CookieInclusionStatus::EXCLUDE_SECURE_ONLY))
-    return "Cannot create a secure cookie from an insecure URL";
-  if (status.HasExclusionReason(
-          net::CookieInclusionStatus::EXCLUDE_FAILURE_TO_STORE))
-    return "Failed to parse cookie";
-  if (status.HasExclusionReason(
-          net::CookieInclusionStatus::EXCLUDE_INVALID_DOMAIN))
-    return "Failed to set cookie with an invalid domain attribute";
-  if (status.HasExclusionReason(
-          net::CookieInclusionStatus::EXCLUDE_INVALID_PREFIX))
-    return "Failed because the cookie violated prefix rules.";
-  if (status.HasExclusionReason(
-          net::CookieInclusionStatus::EXCLUDE_NONCOOKIEABLE_SCHEME))
-    return "Cannot set cookie for current scheme";
-  return "Setting cookie failed";
+const std::string InclusionStatusToString(net::CookieInclusionStatus status) {
+  // See net/cookies/cookie_inclusion_status.h for cookie error descriptions.
+  constexpr std::array<
+      std::pair<net::CookieInclusionStatus::ExclusionReason, std::string_view>,
+      net::CookieInclusionStatus::ExclusionReason::NUM_EXCLUSION_REASONS>
+      exclusion_reasons = {
+          {{net::CookieInclusionStatus::EXCLUDE_HTTP_ONLY,
+            "The cookie was HttpOnly, but the attempted access was through a "
+            "non-HTTP API."},
+           {net::CookieInclusionStatus::EXCLUDE_SECURE_ONLY,
+            "The cookie was Secure, but the URL was not allowed to access "
+            "Secure cookies."},
+           {net::CookieInclusionStatus::EXCLUDE_DOMAIN_MISMATCH,
+            "The cookie's domain attribute did not match the domain of the URL "
+            "attempting access."},
+           {net::CookieInclusionStatus::EXCLUDE_NOT_ON_PATH,
+            "The cookie's path attribute did not match the path of the URL "
+            "attempting access."},
+           {net::CookieInclusionStatus::EXCLUDE_SAMESITE_STRICT,
+            "The cookie had SameSite=Strict, and the attempted access did not "
+            "have an appropriate SameSiteCookieContext"},
+           {net::CookieInclusionStatus::EXCLUDE_SAMESITE_LAX,
+            "The cookie had SameSite=Lax, and the attempted access did not "
+            "have "
+            "an appropriate SameSiteCookieContext"},
+           {net::CookieInclusionStatus::
+                EXCLUDE_SAMESITE_UNSPECIFIED_TREATED_AS_LAX,
+            "The cookie did not specify a SameSite attribute, and therefore "
+            "was "
+            "treated as if it were SameSite=Lax, and the attempted access did "
+            "not have an appropriate SameSiteCookieContext."},
+           {net::CookieInclusionStatus::EXCLUDE_SAMESITE_NONE_INSECURE,
+            "The cookie specified SameSite=None, but it was not Secure."},
+           {net::CookieInclusionStatus::EXCLUDE_USER_PREFERENCES,
+            "Caller did not allow access to the cookie."},
+           {net::CookieInclusionStatus::EXCLUDE_FAILURE_TO_STORE,
+            "The cookie was malformed and could not be stored"},
+           {net::CookieInclusionStatus::EXCLUDE_NONCOOKIEABLE_SCHEME,
+            "Attempted to set a cookie from a scheme that does not support "
+            "cookies."},
+           {net::CookieInclusionStatus::EXCLUDE_OVERWRITE_SECURE,
+            "The cookie would have overwritten a Secure cookie, and was not "
+            "allowed to do so."},
+           {net::CookieInclusionStatus::EXCLUDE_OVERWRITE_HTTP_ONLY,
+            "The cookie would have overwritten an HttpOnly cookie, and was not "
+            "allowed to do so."},
+           {net::CookieInclusionStatus::EXCLUDE_INVALID_DOMAIN,
+            "The cookie was set with an invalid Domain attribute."},
+           {net::CookieInclusionStatus::EXCLUDE_INVALID_PREFIX,
+            "The cookie was set with an invalid __Host- or __Secure- prefix."},
+           {net::CookieInclusionStatus::EXCLUDE_INVALID_PARTITIONED,
+            "Cookie was set with an invalid Partitioned attribute, which is "
+            "only valid if the cookie has a __Host- prefix."},
+           {net::CookieInclusionStatus::
+                EXCLUDE_NAME_VALUE_PAIR_EXCEEDS_MAX_SIZE,
+            "The cookie exceeded the name/value pair size limit."},
+           {net::CookieInclusionStatus::
+                EXCLUDE_ATTRIBUTE_VALUE_EXCEEDS_MAX_SIZE,
+            "Cookie exceeded the attribute size limit."},
+           {net::CookieInclusionStatus::EXCLUDE_DOMAIN_NON_ASCII,
+            "The cookie was set with a Domain attribute containing non ASCII "
+            "characters."},
+           {net::CookieInclusionStatus::
+                EXCLUDE_THIRD_PARTY_BLOCKED_WITHIN_FIRST_PARTY_SET,
+            "The cookie is blocked by third-party cookie blocking but the two "
+            "sites are in the same First-Party Set"},
+           {net::CookieInclusionStatus::EXCLUDE_PORT_MISMATCH,
+            "The cookie's source_port did not match the port of the request."},
+           {net::CookieInclusionStatus::EXCLUDE_SCHEME_MISMATCH,
+            "The cookie's source_scheme did not match the scheme of the "
+            "request."},
+           {net::CookieInclusionStatus::EXCLUDE_SHADOWING_DOMAIN,
+            "The cookie is a domain cookie and has the same name as an origin "
+            "cookie on this origin."},
+           {net::CookieInclusionStatus::EXCLUDE_DISALLOWED_CHARACTER,
+            "The cookie contains ASCII control characters"},
+           {net::CookieInclusionStatus::EXCLUDE_THIRD_PARTY_PHASEOUT,
+            "The cookie is blocked for third-party cookie phaseout."},
+           {net::CookieInclusionStatus::EXCLUDE_NO_COOKIE_CONTENT,
+            "The cookie contains no content or only whitespace."}}};
+
+  std::string reason = "Failed to set cookie - ";
+  for (const auto& [val, str] : exclusion_reasons) {
+    if (status.HasExclusionReason(val)) {
+      reason += str;
+    }
+  }
+
+  reason += status.GetDebugString();
+  return reason;
 }
 
 std::string StringToCookieSameSite(const std::string* str_ptr,

spec/api-net-session-spec.ts

@@ -378,7 +378,7 @@ describe('net module (session)', () => {
         url: 'https://electronjs.org',
         domain: 'wssss.iamabaddomain.fun',
         name: 'cookie1'
-      })).to.eventually.be.rejectedWith(/Failed to set cookie with an invalid domain attribute/);
+      })).to.eventually.be.rejectedWith(/Failed to set cookie - The cookie was set with an invalid Domain attribute./);
     });
 
     it('should be able correctly filter out cookies that are session', async () => {

spec/api-session-spec.ts

@@ -126,24 +126,34 @@ describe('session module', () => {
       expect(cs.some(c => c.name === name && c.value === value)).to.equal(true);
     });
 
-    it('yields an error when setting a cookie with missing required fields', async () => {
+    it('rejects when setting a cookie with missing required fields', async () => {
       const { cookies } = session.defaultSession;
       const name = '1';
       const value = '1';
 
       await expect(
         cookies.set({ url: '', name, value })
-      ).to.eventually.be.rejectedWith('Failed to set cookie with an invalid domain attribute');
+      ).to.eventually.be.rejectedWith('Failed to set cookie - The cookie was set with an invalid Domain attribute.');
     });
 
-    it('yields an error when setting a cookie with an invalid URL', async () => {
+    it('rejects when setting a cookie with an invalid URL', async () => {
       const { cookies } = session.defaultSession;
       const name = '1';
       const value = '1';
 
       await expect(
         cookies.set({ url: 'asdf', name, value })
-      ).to.eventually.be.rejectedWith('Failed to set cookie with an invalid domain attribute');
+      ).to.eventually.be.rejectedWith('Failed to set cookie - The cookie was set with an invalid Domain attribute.');
+    });
+
+    it('rejects when setting a cookie with an invalid ASCII control character', async () => {
+      const { cookies } = session.defaultSession;
+      const name = 'BadCookie';
+      const value = 'test;test';
+
+      await expect(
+        cookies.set({ url, name, value })
+      ).to.eventually.be.rejectedWith('Failed to set cookie - The cookie contains ASCII control characters');
     });
 
     it('should overwrite previous cookies', async () => {