From 74e5f6e43b537cf16894c195cccfc7acf7d8cb02 Mon Sep 17 00:00:00 2001
From: Erick Zhao <erickzhao@github.com>
Date: Wed, 19 Jun 2019 18:17:02 -0700
Subject: [PATCH 1/2] fix: throw error on invalid URLs when setting cookie

---
 atom/browser/api/atom_api_cookies.cc | 22 +++++++++++-----------
 spec/api-session-spec.js             | 19 +++++++++++++++++++
 2 files changed, 30 insertions(+), 11 deletions(-)

diff --git a/atom/browser/api/atom_api_cookies.cc b/atom/browser/api/atom_api_cookies.cc
index 882937b1c6bed..bb1913c7c36a1 100644
--- a/atom/browser/api/atom_api_cookies.cc
+++ b/atom/browser/api/atom_api_cookies.cc
@@ -195,13 +195,13 @@ void FlushCookieStoreOnIOThread(
 void SetCookieOnIO(scoped_refptr<net::URLRequestContextGetter> getter,
                    std::unique_ptr<base::DictionaryValue> details,
                    const Cookies::SetCallback& callback) {
-  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);
@@ -229,22 +229,22 @@ void SetCookieOnIO(scoped_refptr<net::URLRequestContextGetter> getter,
                            ? base::Time::UnixEpoch()
                            : 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, callback);
-  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;
   }
diff --git a/spec/api-session-spec.js b/spec/api-session-spec.js
index 6e92aa263971f..72bbf457d16ee 100644
--- a/spec/api-session-spec.js
+++ b/spec/api-session-spec.js
@@ -91,6 +91,13 @@ describe('session module', () => {
       })
     })
 
+    it('sets cookies', (done) => {
+      const { cookies } = session.defaultSession
+      const name = '1'
+      const value = '1'
+      cookies.set({ url, name, value }, (error, list) => done(error))
+    })
+
     it('calls back with an error when setting a cookie with missing required fields', (done) => {
       session.defaultSession.cookies.set({
         url: '',
@@ -103,6 +110,18 @@ describe('session module', () => {
       })
     })
 
+    it('yields an error when setting a cookie with an invalid URL', (done) => {
+      session.defaultSession.cookies.set({
+        url: 'asdf',
+        name: '1',
+        value: '1'
+      }, (error) => {
+        assert(error, 'Should have an error')
+        assert.strictEqual(error.message, 'Setting cookie failed')
+        done()
+      })
+    })
+
     it('should over-write the existent cookie', (done) => {
       session.defaultSession.cookies.set({
         url,

From 0e8584d7f11e76bd8ebf29939fc3ade1d885b738 Mon Sep 17 00:00:00 2001
From: Erick Zhao <erickzhao@github.com>
Date: Wed, 19 Jun 2019 18:24:42 -0700
Subject: [PATCH 2/2] not sure where that test came from?

---
 spec/api-session-spec.js | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/spec/api-session-spec.js b/spec/api-session-spec.js
index 72bbf457d16ee..19346071266db 100644
--- a/spec/api-session-spec.js
+++ b/spec/api-session-spec.js
@@ -91,13 +91,6 @@ describe('session module', () => {
       })
     })
 
-    it('sets cookies', (done) => {
-      const { cookies } = session.defaultSession
-      const name = '1'
-      const value = '1'
-      cookies.set({ url, name, value }, (error, list) => done(error))
-    })
-
     it('calls back with an error when setting a cookie with missing required fields', (done) => {
       session.defaultSession.cookies.set({
         url: '',