refactor: use url::DomainIs() to check cookie domains (#43262)

* test: add tests to exercise pre-exsiting cookie domain matching behavior

* refactor: use url::DomainIs() to match cookie domains

* docs: fix typo

docs/api/cookies.md

@@ -74,7 +74,7 @@ The following methods are available on instances of `Cookies`:
     `url`. Empty implies retrieving cookies of all URLs.
   * `name` string (optional) - Filters cookies by name.
   * `domain` string (optional) - Retrieves cookies whose domains match or are
-    subdomains of `domains`.
+    subdomains of `domain`.
   * `path` string (optional) - Retrieves cookies whose path matches `path`.
   * `secure` boolean (optional) - Filters cookies by their Secure property.
   * `session` boolean (optional) - Filters out session or persistent cookies.

shell/browser/api/electron_api_cookies.cc

@@ -30,6 +30,7 @@
 #include "shell/common/gin_helper/dictionary.h"
 #include "shell/common/gin_helper/object_template_builder.h"
 #include "shell/common/gin_helper/promise.h"
+#include "url/url_util.h"
 
 namespace gin {
 
@@ -100,25 +101,12 @@ namespace electron::api {
 
 namespace {
 
-// Returns whether |domain| matches |filter|.
-bool MatchesDomain(std::string filter, const std::string& domain) {
-  // Add a leading '.' character to the filter domain if it doesn't exist.
-  if (net::cookie_util::DomainIsHostOnly(filter))
-    filter.insert(0, ".");
-
-  std::string sub_domain(domain);
+bool DomainIs(std::string_view host, const std::string_view domain) {
   // Strip any leading '.' character from the input cookie domain.
-  if (!net::cookie_util::DomainIsHostOnly(sub_domain))
-    sub_domain = sub_domain.substr(1);
-
-  // Now check whether the domain argument is a subdomain of the filter domain.
-  for (sub_domain.insert(0, "."); sub_domain.length() >= filter.length();) {
-    if (sub_domain == filter)
-      return true;
-    const size_t next_dot = sub_domain.find('.', 1);  // Skip over leading dot.
-    sub_domain.erase(0, next_dot);
-  }
-  return false;
+  if (host.starts_with('.'))
+    host.remove_prefix(1);
+
+  return url::DomainIs(host, domain);
 }
 
 // Returns whether |cookie| matches |filter|.
@@ -129,8 +117,7 @@ bool MatchesCookie(const base::Value::Dict& filter,
     return false;
   if ((str = filter.FindString("path")) && *str != cookie.Path())
     return false;
-  if ((str = filter.FindString("domain")) &&
-      !MatchesDomain(*str, cookie.Domain()))
+  if ((str = filter.FindString("domain")) && !DomainIs(cookie.Domain(), *str))
     return false;
   std::optional<bool> secure_filter = filter.FindBool("secure");
   if (secure_filter && *secure_filter != cookie.SecureAttribute())

spec/api-session-spec.ts

@@ -1,4 +1,5 @@
 import { expect } from 'chai';
+import * as crypto from 'node:crypto';
 import * as http from 'node:http';
 import * as https from 'node:https';
 import * as path from 'node:path';
@@ -126,6 +127,54 @@ describe('session module', () => {
       expect(cs.some(c => c.name === name && c.value === value)).to.equal(true);
     });
 
+    it('does not match on empty domain filter strings', async () => {
+      const { cookies } = session.defaultSession;
+      const name = crypto.randomBytes(20).toString('hex');
+      const value = '1';
+      const url = 'https://microsoft.com/';
+
+      await cookies.set({ url, name, value });
+      const cs = await cookies.get({ domain: '' });
+      expect(cs.some(c => c.name === name && c.value === value)).to.equal(false);
+      cookies.remove(url, name);
+    });
+
+    it('gets domain-equal cookies', async () => {
+      const { cookies } = session.defaultSession;
+      const name = crypto.randomBytes(20).toString('hex');
+      const value = '1';
+      const url = 'https://microsoft.com/';
+
+      await cookies.set({ url, name, value });
+      const cs = await cookies.get({ domain: 'microsoft.com' });
+      expect(cs.some(c => c.name === name && c.value === value)).to.equal(true);
+      cookies.remove(url, name);
+    });
+
+    it('gets domain-inclusive cookies', async () => {
+      const { cookies } = session.defaultSession;
+      const name = crypto.randomBytes(20).toString('hex');
+      const value = '1';
+      const url = 'https://subdomain.microsoft.com/';
+
+      await cookies.set({ url, name, value });
+      const cs = await cookies.get({ domain: 'microsoft.com' });
+      expect(cs.some(c => c.name === name && c.value === value)).to.equal(true);
+      cookies.remove(url, name);
+    });
+
+    it('omits domain-exclusive cookies', async () => {
+      const { cookies } = session.defaultSession;
+      const name = crypto.randomBytes(20).toString('hex');
+      const value = '1';
+      const url = 'https://microsoft.com';
+
+      await cookies.set({ url, name, value });
+      const cs = await cookies.get({ domain: 'subdomain.microsoft.com' });
+      expect(cs.some(c => c.name === name && c.value === value)).to.equal(false);
+      cookies.remove(url, name);
+    });
+
     it('rejects when setting a cookie with missing required fields', async () => {
       const { cookies } = session.defaultSession;
       const name = '1';