fix: name and expirationDate should be optional when setting cookie (#…

…21454) (#21481)

* fix: correctly set cookie date

* fix: name is not required for setting cookie

* test: clear cookie after each cookie test

* test: should test session property

* chore: style fixes

shell/browser/api/atom_api_cookies.cc

@@ -141,6 +141,15 @@ void FilterCookieWithStatuses(const base::Value& filter,
                 net::cookie_util::StripStatuses(list));
 }
 
+// Parse dictionary property to CanonicalCookie time correctly.
+base::Time ParseTimeProperty(const base::Optional<double>& value) {
+  if (!value)  // empty time means ignoring the parameter
+    return base::Time();
+  if (*value == 0)  // FromDoubleT would convert 0 to empty Time
+    return base::Time::UnixEpoch();
+  return base::Time::FromDoubleT(*value);
+}
+
 std::string InclusionStatusToString(
     net::CanonicalCookie::CookieInclusionStatus status) {
   if (status.HasExclusionReason(
@@ -243,21 +252,6 @@ v8::Local<v8::Promise> Cookies::Set(const base::DictionaryValue& details) {
   const std::string* path = details.FindStringKey("path");
   bool secure = details.FindBoolKey("secure").value_or(false);
   bool http_only = details.FindBoolKey("httpOnly").value_or(false);
-  base::Optional<double> creation_date = details.FindDoubleKey("creationDate");
-  base::Optional<double> expiration_date =
-      details.FindDoubleKey("expirationDate");
-  base::Optional<double> last_access_date =
-      details.FindDoubleKey("lastAccessDate");
-
-  base::Time creation_time = creation_date
-                                 ? base::Time::FromDoubleT(*creation_date)
-                                 : base::Time::UnixEpoch();
-  base::Time expiration_time = expiration_date
-                                   ? base::Time::FromDoubleT(*expiration_date)
-                                   : base::Time::UnixEpoch();
-  base::Time last_access_time = last_access_date
-                                    ? base::Time::FromDoubleT(*last_access_date)
-                                    : base::Time::UnixEpoch();
 
   GURL url(url_string ? *url_string : "");
   if (!url.is_valid()) {
@@ -268,18 +262,14 @@ v8::Local<v8::Promise> Cookies::Set(const base::DictionaryValue& details) {
     return handle;
   }
 
-  if (!name || name->empty()) {
-    promise.RejectWithErrorMessage(
-        InclusionStatusToString(net::CanonicalCookie::CookieInclusionStatus(
-            net::CanonicalCookie::CookieInclusionStatus::
-                EXCLUDE_FAILURE_TO_STORE)));
-    return handle;
-  }
-
   auto canonical_cookie = net::CanonicalCookie::CreateSanitizedCookie(
-      url, *name, value ? *value : "", domain ? *domain : "", path ? *path : "",
-      creation_time, expiration_time, last_access_time, secure, http_only,
-      net::CookieSameSite::NO_RESTRICTION, net::COOKIE_PRIORITY_DEFAULT);
+      url, name ? *name : "", value ? *value : "", domain ? *domain : "",
+      path ? *path : "",
+      ParseTimeProperty(details.FindDoubleKey("creationDate")),
+      ParseTimeProperty(details.FindDoubleKey("expirationDate")),
+      ParseTimeProperty(details.FindDoubleKey("lastAccessDate")), secure,
+      http_only, net::CookieSameSite::NO_RESTRICTION,
+      net::COOKIE_PRIORITY_DEFAULT);
   if (!canonical_cookie || !canonical_cookie->IsCanonical()) {
     promise.RejectWithErrorMessage(
         InclusionStatusToString(net::CanonicalCookie::CookieInclusionStatus(

spec-main/api-net-spec.ts

@@ -591,7 +591,8 @@ describe('net module', () => {
         customSession.cookies.set({
           url: `${serverUrl}`,
           name: 'test',
-          value: '11111'
+          value: '11111',
+          expirationDate: 0
         }).then(() => { // resolved
           const urlRequest = net.request({
             method: 'GET',

spec-main/api-session-spec.js

@@ -59,6 +59,15 @@ describe('session module', () => {
     const value = '0'
     afterEach(closeAllWindows)
 
+    // Clear cookie of defaultSession after each test.
+    afterEach(async () => {
+      const { cookies } = session.defaultSession
+      const cs = await cookies.get({ url })
+      for (const c of cs) {
+        await cookies.remove(url, c.name)
+      }
+    })
+
     it('should get cookies', async () => {
       const server = http.createServer((req, res) => {
         res.setHeader('Set-Cookie', [`${name}=${value}`])
@@ -79,9 +88,33 @@ describe('session module', () => {
       const name = '1'
       const value = '1'
 
-      await cookies.set({ url, name, value, expirationDate: (+new Date) / 1000 + 120 })
-      const cs = await cookies.get({ url })
-      expect(cs.some(c => c.name === name && c.value === value)).to.equal(true)
+      await cookies.set({ url, name, value, expirationDate: (+new Date()) / 1000 + 120 })
+      const c = (await cookies.get({ url }))[0]
+      expect(c.name).to.equal(name)
+      expect(c.value).to.equal(value)
+      expect(c.session).to.equal(false)
+    })
+
+    it('sets session cookies', async () => {
+      const { cookies } = session.defaultSession
+      const name = '2'
+      const value = '1'
+
+      await cookies.set({ url, name, value })
+      const c = (await cookies.get({ url }))[0]
+      expect(c.name).to.equal(name)
+      expect(c.value).to.equal(value)
+      expect(c.session).to.equal(true)
+    })
+
+    it('sets cookies without name', async () => {
+      const { cookies } = session.defaultSession
+      const value = '3'
+
+      await cookies.set({ url, value })
+      const c = (await cookies.get({ url }))[0]
+      expect(c.name).to.be.empty()
+      expect(c.value).to.equal(value)
     })
 
     it('gets cookies without url', async () => {