Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: name and expirationDate should be optional when setting cookie (7-1-x) #21481

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
42 changes: 16 additions & 26 deletions shell/browser/api/atom_api_cookies.cc
Expand Up @@ -141,6 +141,15 @@ void FilterCookieWithStatuses(const base::Value& filter,
net::cookie_util::StripStatuses(list));
}

// Parse dictionary property to CanonicalCookie time correctly.
base::Time ParseTimeProperty(const base::Optional<double>& value) {
if (!value) // empty time means ignoring the parameter
return base::Time();
if (*value == 0) // FromDoubleT would convert 0 to empty Time
return base::Time::UnixEpoch();
return base::Time::FromDoubleT(*value);
}

std::string InclusionStatusToString(
net::CanonicalCookie::CookieInclusionStatus status) {
if (status.HasExclusionReason(
Expand Down Expand Up @@ -243,21 +252,6 @@ v8::Local<v8::Promise> Cookies::Set(const base::DictionaryValue& details) {
const std::string* path = details.FindStringKey("path");
bool secure = details.FindBoolKey("secure").value_or(false);
bool http_only = details.FindBoolKey("httpOnly").value_or(false);
base::Optional<double> creation_date = details.FindDoubleKey("creationDate");
base::Optional<double> expiration_date =
details.FindDoubleKey("expirationDate");
base::Optional<double> last_access_date =
details.FindDoubleKey("lastAccessDate");

base::Time creation_time = creation_date
? base::Time::FromDoubleT(*creation_date)
: base::Time::UnixEpoch();
base::Time expiration_time = expiration_date
? base::Time::FromDoubleT(*expiration_date)
: base::Time::UnixEpoch();
base::Time last_access_time = last_access_date
? base::Time::FromDoubleT(*last_access_date)
: base::Time::UnixEpoch();

GURL url(url_string ? *url_string : "");
if (!url.is_valid()) {
Expand All @@ -268,18 +262,14 @@ v8::Local<v8::Promise> Cookies::Set(const base::DictionaryValue& details) {
return handle;
}

if (!name || name->empty()) {
promise.RejectWithErrorMessage(
InclusionStatusToString(net::CanonicalCookie::CookieInclusionStatus(
net::CanonicalCookie::CookieInclusionStatus::
EXCLUDE_FAILURE_TO_STORE)));
return handle;
}

auto canonical_cookie = net::CanonicalCookie::CreateSanitizedCookie(
url, *name, value ? *value : "", domain ? *domain : "", path ? *path : "",
creation_time, expiration_time, last_access_time, secure, http_only,
net::CookieSameSite::NO_RESTRICTION, net::COOKIE_PRIORITY_DEFAULT);
url, name ? *name : "", value ? *value : "", domain ? *domain : "",
path ? *path : "",
ParseTimeProperty(details.FindDoubleKey("creationDate")),
ParseTimeProperty(details.FindDoubleKey("expirationDate")),
ParseTimeProperty(details.FindDoubleKey("lastAccessDate")), secure,
http_only, net::CookieSameSite::NO_RESTRICTION,
net::COOKIE_PRIORITY_DEFAULT);
if (!canonical_cookie || !canonical_cookie->IsCanonical()) {
promise.RejectWithErrorMessage(
InclusionStatusToString(net::CanonicalCookie::CookieInclusionStatus(
Expand Down
3 changes: 2 additions & 1 deletion spec-main/api-net-spec.ts
Expand Up @@ -591,7 +591,8 @@ describe('net module', () => {
customSession.cookies.set({
url: `${serverUrl}`,
name: 'test',
value: '11111'
value: '11111',
expirationDate: 0
}).then(() => { // resolved
const urlRequest = net.request({
method: 'GET',
Expand Down
39 changes: 36 additions & 3 deletions spec-main/api-session-spec.js
Expand Up @@ -59,6 +59,15 @@ describe('session module', () => {
const value = '0'
afterEach(closeAllWindows)

// Clear cookie of defaultSession after each test.
afterEach(async () => {
const { cookies } = session.defaultSession
const cs = await cookies.get({ url })
for (const c of cs) {
await cookies.remove(url, c.name)
}
})

it('should get cookies', async () => {
const server = http.createServer((req, res) => {
res.setHeader('Set-Cookie', [`${name}=${value}`])
Expand All @@ -79,9 +88,33 @@ describe('session module', () => {
const name = '1'
const value = '1'

await cookies.set({ url, name, value, expirationDate: (+new Date) / 1000 + 120 })
const cs = await cookies.get({ url })
expect(cs.some(c => c.name === name && c.value === value)).to.equal(true)
await cookies.set({ url, name, value, expirationDate: (+new Date()) / 1000 + 120 })
const c = (await cookies.get({ url }))[0]
expect(c.name).to.equal(name)
expect(c.value).to.equal(value)
expect(c.session).to.equal(false)
})

it('sets session cookies', async () => {
const { cookies } = session.defaultSession
const name = '2'
const value = '1'

await cookies.set({ url, name, value })
const c = (await cookies.get({ url }))[0]
expect(c.name).to.equal(name)
expect(c.value).to.equal(value)
expect(c.session).to.equal(true)
})

it('sets cookies without name', async () => {
const { cookies } = session.defaultSession
const value = '3'

await cookies.set({ url, value })
const c = (await cookies.get({ url }))[0]
expect(c.name).to.be.empty()
expect(c.value).to.equal(value)
})

it('gets cookies without url', async () => {
Expand Down