Browse Source

fix: cookies.get should be able to filter domain (#20471) (#20496)

* fix: use GetAllCookies when url is empty

* test: get cookie without url
Cheng Zhao 5 years ago
parent
commit
26d059b3ea

+ 30 - 18
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"
@@ -122,11 +123,9 @@ 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 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);
   }
@@ -134,6 +133,14 @@ void FilterCookies(const base::Value& filter,
   promise.Resolve(gin::ConvertToV8(promise.isolate(), result));
 }
 
+void FilterCookieWithStatuses(const base::Value& filter,
+                              util::Promise 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(
@@ -170,28 +177,33 @@ Cookies::Cookies(v8::Isolate* isolate, AtomBrowserContext* browser_context)
 
 Cookies::~Cookies() {}
 
-v8::Local<v8::Promise> Cookies::Get(const base::DictionaryValue& filter) {
+v8::Local<v8::Promise> Cookies::Get(const mate::Dictionary& filter) {
   util::Promise 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);
+
+  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(url, options, std::move(callback));
+    manager->GetCookieList(GURL(url), options,
+                           base::BindOnce(&FilterCookieWithStatuses,
+                                          std::move(dict), std::move(promise)));
+  }
 
   return handle;
 }

+ 5 - 1
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();

+ 10 - 0
spec-main/api-session-spec.js

@@ -84,6 +84,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'