fix: throw error on invalid URLs when setting cookie

atom/browser/api/atom_api_cookies.cc

@@ -226,13 +226,13 @@ void FlushCookieStoreOnIOThread(
 void SetCookieOnIO(scoped_refptr<net::URLRequestContextGetter> getter,
                    std::unique_ptr<base::DictionaryValue> details,
                    util::Promise promise) {
-  std::string url, name, value, domain, path;
+  std::string url_string, name, value, domain, path;
   bool secure = false;
   bool http_only = false;
   double creation_date;
   double expiration_date;
   double last_access_date;
-  details->GetString("url", &url);
+  details->GetString("url", &url_string);
   details->GetString("name", &name);
   details->GetString("value", &value);
   details->GetString("domain", &domain);
@@ -261,21 +261,24 @@ void SetCookieOnIO(scoped_refptr<net::URLRequestContextGetter> getter,
                            : base::Time::FromDoubleT(last_access_date);
   }
 
-  std::unique_ptr<net::CanonicalCookie> canonical_cookie(
-      net::CanonicalCookie::CreateSanitizedCookie(
-          GURL(url), name, value, domain, path, creation_time, expiration_time,
-          last_access_time, secure, http_only,
-          net::CookieSameSite::DEFAULT_MODE, net::COOKIE_PRIORITY_DEFAULT));
   auto completion_callback = base::BindOnce(OnSetCookie, std::move(promise));
-  if (!canonical_cookie || !canonical_cookie->IsCanonical()) {
+  GURL url(url_string);
+  if (!url.is_valid()) {
     std::move(completion_callback).Run(false);
     return;
   }
-  if (url.empty()) {
+
+  if (name.empty()) {
     std::move(completion_callback).Run(false);
     return;
   }
-  if (name.empty()) {
+
+  std::unique_ptr<net::CanonicalCookie> canonical_cookie(
+      net::CanonicalCookie::CreateSanitizedCookie(
+          url, name, value, domain, path, creation_time, expiration_time,
+          last_access_time, secure, http_only,
+          net::CookieSameSite::DEFAULT_MODE, net::COOKIE_PRIORITY_DEFAULT));
+  if (!canonical_cookie || !canonical_cookie->IsCanonical()) {
     std::move(completion_callback).Run(false);
     return;
   }

docs/api/cookies.md

@@ -104,7 +104,7 @@ with `callback(error, cookies)` on complete.
 #### `cookies.set(details)`
 
 * `details` Object
-  * `url` String - The url to associate the cookie with.
+  * `url` String - The url to associate the cookie with. The promise will be rejected if the url is invalid.
   * `name` String (optional) - The name of the cookie. Empty by default if omitted.
   * `value` String (optional) - The value of the cookie. Empty by default if omitted.
   * `domain` String (optional) - The domain of the cookie; this will be normalized with a preceding dot so that it's also valid for subdomains. Empty by default if omitted.

spec/api-session-spec.js

@@ -145,6 +145,20 @@ describe('session module', () => {
       expect(error).to.have.property('message').which.equals('Setting cookie failed')
     })
 
+    it('yields an error when setting a cookie with an invalid URL', async () => {
+      let error
+      try {
+        const { cookies } = session.defaultSession
+        const name = '1'
+        const value = '1'
+        await cookies.set({ url: 'asdf', name, value })
+      } catch (e) {
+        error = e
+      }
+      expect(error).is.an('Error')
+      expect(error).to.have.property('message').which.equals('Setting cookie failed')
+    })
+
     it('should overwrite previous cookies', async () => {
       let error
       try {