Skip to content

Commit

Permalink
fix: cookies filter secure invalid (#37246)
Browse files Browse the repository at this point in the history
* fix: cookies filter secure and session invalid

Co-authored-by: Black-Hole1 <158blackhole@gmail.com>

* test: add test

Co-authored-by: Black-Hole1 <158blackhole@gmail.com>

* test: fix test failed

Co-authored-by: Black-Hole1 <158blackhole@gmail.com>

* test: fix test failed again

Co-authored-by: Black-Hole1 <158blackhole@gmail.com>

* fix: session check logic incorrect

Co-authored-by: Black-Hole1 <158blackhole@gmail.com>

* refactor: reset cookies filter session logic

Co-authored-by: Black-Hole1 <158blackhole@gmail.com>

* Update shell/browser/api/electron_api_cookies.cc

Co-authored-by: Robo <hop2deep@gmail.com>

Co-authored-by: Black-Hole <158blackhole@gmail.com>

* chore: re-enable worker spec failures (#37015)

Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>

---------

Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Black-Hole1 <158blackhole@gmail.com>
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
  • Loading branch information
3 people committed Feb 15, 2023
1 parent 91776c5 commit 99d648f
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 5 deletions.
3 changes: 0 additions & 3 deletions script/node-disabled-tests.json
Original file line number Diff line number Diff line change
Expand Up @@ -134,9 +134,6 @@
"parallel/test-webcrypto-export-import-cfrg",
"parallel/test-webcrypto-keygen",
"parallel/test-webcrypto-sign-verify-eddsa",
"parallel/test-worker-debug",
"parallel/test-worker-init-failure",
"parallel/test-worker-stdio",
"parallel/test-zlib-unused-weak",
"report/test-report-fatalerror-oomerror-set",
"report/test-report-fatalerror-oomerror-directory",
Expand Down
4 changes: 2 additions & 2 deletions shell/browser/api/electron_api_cookies.cc
Original file line number Diff line number Diff line change
Expand Up @@ -128,10 +128,10 @@ bool MatchesCookie(const base::Value::Dict& filter,
!MatchesDomain(*str, cookie.Domain()))
return false;
absl::optional<bool> secure_filter = filter.FindBool("secure");
if (secure_filter && *secure_filter == cookie.IsSecure())
if (secure_filter && *secure_filter != cookie.IsSecure())
return false;
absl::optional<bool> session_filter = filter.FindBool("session");
if (session_filter && *session_filter != !cookie.IsPersistent())
if (session_filter && *session_filter == cookie.IsPersistent())
return false;
return true;
}
Expand Down
65 changes: 65 additions & 0 deletions spec/api-net-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -813,6 +813,71 @@ describe('net module', () => {
});
}

it('should be able correctly filter out cookies that are secure', async () => {
const sess = session.fromPartition(`cookie-tests-${Math.random()}`);

await Promise.all([
sess.cookies.set({
url: 'https://electronjs.org',
domain: 'electronjs.org',
name: 'cookie1',
value: '1',
secure: true
}),
sess.cookies.set({
url: 'https://electronjs.org',
domain: 'electronjs.org',
name: 'cookie2',
value: '2',
secure: false
})
]);

const secureCookies = await sess.cookies.get({
secure: true
});
expect(secureCookies).to.have.lengthOf(1);
expect(secureCookies[0].name).to.equal('cookie1');

const cookies = await sess.cookies.get({
secure: false
});
expect(cookies).to.have.lengthOf(1);
expect(cookies[0].name).to.equal('cookie2');
});

it('should be able correctly filter out cookies that are session', async () => {
const sess = session.fromPartition(`cookie-tests-${Math.random()}`);

await Promise.all([
sess.cookies.set({
url: 'https://electronjs.org',
domain: 'electronjs.org',
name: 'cookie1',
value: '1'
}),
sess.cookies.set({
url: 'https://electronjs.org',
domain: 'electronjs.org',
name: 'cookie2',
value: '2',
expirationDate: Math.round(Date.now() / 1000) + 10000
})
]);

const sessionCookies = await sess.cookies.get({
session: true
});
expect(sessionCookies).to.have.lengthOf(1);
expect(sessionCookies[0].name).to.equal('cookie1');

const cookies = await sess.cookies.get({
session: false
});
expect(cookies).to.have.lengthOf(1);
expect(cookies[0].name).to.equal('cookie2');
});

describe('when {"credentials":"omit"}', () => {
it('should not send cookies');
it('should not store cookies');
Expand Down

0 comments on commit 99d648f

Please sign in to comment.