Skip to content

Commit

Permalink
fix: cookies.get should be able to filter domain (#20471)
Browse files Browse the repository at this point in the history
* fix: use GetAllCookies when url is empty

* test: get cookie without url
  • Loading branch information
zcbenz committed Oct 9, 2019
1 parent ebd55c1 commit 5e11be6
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 20 deletions.
49 changes: 30 additions & 19 deletions shell/browser/api/atom_api_cookies.cc
Expand Up @@ -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"
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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);

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;
}
Expand Down
6 changes: 5 additions & 1 deletion shell/browser/api/atom_api_cookies.h
Expand Up @@ -19,6 +19,10 @@ namespace base {
class DictionaryValue;
}

namespace mate {
class Dictionary;
}

namespace net {
class URLRequestContextGetter;
}
Expand All @@ -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();
Expand Down
10 changes: 10 additions & 0 deletions spec-main/api-session-spec.ts
Expand Up @@ -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'
Expand Down

0 comments on commit 5e11be6

Please sign in to comment.