From 5e11be689851d9bb1e658832680c57b49e3e8cf8 Mon Sep 17 00:00:00 2001
From: Cheng Zhao <zcbenz@gmail.com>
Date: Wed, 9 Oct 2019 15:57:40 +0900
Subject: [PATCH] fix: cookies.get should be able to filter domain (#20471)

* fix: use GetAllCookies when url is empty

* test: get cookie without url
---
 shell/browser/api/atom_api_cookies.cc | 49 ++++++++++++++++-----------
 shell/browser/api/atom_api_cookies.h  |  6 +++-
 spec-main/api-session-spec.ts         | 10 ++++++
 3 files changed, 45 insertions(+), 20 deletions(-)

diff --git a/shell/browser/api/atom_api_cookies.cc b/shell/browser/api/atom_api_cookies.cc
index 7d141ac8619a..fc392b5d230d 100644
--- a/shell/browser/api/atom_api_cookies.cc
+++ b/shell/browser/api/atom_api_cookies.cc
@@ -15,6 +15,7 @@
 #include "content/public/browser/storage_partition.h"
 #include "gin/dictionary.h"
 #include "gin/object_template_builder.h"
+#include "native_mate/dictionary.h"
 #include "net/cookies/canonical_cookie.h"
 #include "net/cookies/cookie_store.h"
 #include "net/cookies/cookie_util.h"
@@ -121,18 +122,23 @@ bool MatchesCookie(const base::Value& filter,
 // Remove cookies from |list| not matching |filter|, and pass it to |callback|.
 void FilterCookies(const base::Value& filter,
                    util::Promise<net::CookieList> promise,
-                   const net::CookieStatusList& list,
-                   const net::CookieStatusList& excluded_list) {
+                   const net::CookieList& cookies) {
   net::CookieList result;
-  net::CookieList stripped_cookies = net::cookie_util::StripStatuses(list);
-  for (const auto& cookie : stripped_cookies) {
+  for (const auto& cookie : cookies) {
     if (MatchesCookie(filter, cookie))
       result.push_back(cookie);
   }
-
   promise.ResolveWithGin(result);
 }
 
+void FilterCookieWithStatuses(const base::Value& filter,
+                              util::Promise<net::CookieList> promise,
+                              const net::CookieStatusList& list,
+                              const net::CookieStatusList& excluded_list) {
+  FilterCookies(filter, std::move(promise),
+                net::cookie_util::StripStatuses(list));
+}
+
 std::string InclusionStatusToString(
     net::CanonicalCookie::CookieInclusionStatus status) {
   if (status.HasExclusionReason(
@@ -169,28 +175,33 @@ Cookies::Cookies(v8::Isolate* isolate, AtomBrowserContext* browser_context)
 
 Cookies::~Cookies() = default;
 
-v8::Local<v8::Promise> Cookies::Get(const base::DictionaryValue& filter) {
+v8::Local<v8::Promise> Cookies::Get(const mate::Dictionary& filter) {
   util::Promise<net::CookieList> promise(isolate());
   v8::Local<v8::Promise> handle = promise.GetHandle();
 
-  std::string url_string;
-  filter.GetString("url", &url_string);
-  GURL url(url_string);
-
-  auto callback =
-      base::BindOnce(FilterCookies, filter.Clone(), std::move(promise));
-
   auto* storage_partition = content::BrowserContext::GetDefaultStoragePartition(
       browser_context_.get());
   auto* manager = storage_partition->GetCookieManagerForBrowserProcess();
 
-  net::CookieOptions options;
-  options.set_include_httponly();
-  options.set_same_site_cookie_context(
-      net::CookieOptions::SameSiteCookieContext::SAME_SITE_STRICT);
-  options.set_do_not_update_access_time();
+  base::DictionaryValue dict;
+  mate::ConvertFromV8(isolate(), filter.GetHandle(), &dict);
 
-  manager->GetCookieList(url, options, std::move(callback));
+  std::string url;
+  filter.Get("url", &url);
+  if (url.empty()) {
+    manager->GetAllCookies(
+        base::BindOnce(&FilterCookies, std::move(dict), std::move(promise)));
+  } else {
+    net::CookieOptions options;
+    options.set_include_httponly();
+    options.set_same_site_cookie_context(
+        net::CookieOptions::SameSiteCookieContext::SAME_SITE_STRICT);
+    options.set_do_not_update_access_time();
+
+    manager->GetCookieList(GURL(url), options,
+                           base::BindOnce(&FilterCookieWithStatuses,
+                                          std::move(dict), std::move(promise)));
+  }
 
   return handle;
 }
diff --git a/shell/browser/api/atom_api_cookies.h b/shell/browser/api/atom_api_cookies.h
index bc273db9633a..eba0fb622906 100644
--- a/shell/browser/api/atom_api_cookies.h
+++ b/shell/browser/api/atom_api_cookies.h
@@ -19,6 +19,10 @@ namespace base {
 class DictionaryValue;
 }
 
+namespace mate {
+class Dictionary;
+}
+
 namespace net {
 class URLRequestContextGetter;
 }
@@ -42,7 +46,7 @@ class Cookies : public mate::TrackableObject<Cookies> {
   Cookies(v8::Isolate* isolate, AtomBrowserContext* browser_context);
   ~Cookies() override;
 
-  v8::Local<v8::Promise> Get(const base::DictionaryValue& filter);
+  v8::Local<v8::Promise> Get(const mate::Dictionary& filter);
   v8::Local<v8::Promise> Set(const base::DictionaryValue& details);
   v8::Local<v8::Promise> Remove(const GURL& url, const std::string& name);
   v8::Local<v8::Promise> FlushStore();
diff --git a/spec-main/api-session-spec.ts b/spec-main/api-session-spec.ts
index f22566bcc4ff..40975b971372 100644
--- a/spec-main/api-session-spec.ts
+++ b/spec-main/api-session-spec.ts
@@ -83,6 +83,16 @@ describe('session module', () => {
       expect(cs.some(c => c.name === name && c.value === value)).to.equal(true)
     })
 
+    it('gets cookies without url', async () => {
+      const { cookies } = session.defaultSession
+      const name = '1'
+      const value = '1'
+
+      await cookies.set({ url, name, value, expirationDate: (+new Date) / 1000 + 120 })
+      const cs = await cookies.get({ domain: '127.0.0.1' })
+      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 () => {
       const { cookies } = session.defaultSession
       const name = '1'