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(wk,ff): properly support getting and setting non-session cookies #1280

Merged
merged 2 commits into from
Mar 7, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -477,6 +477,7 @@ An array of all pages inside the browser context.
- `expires` <[number]> Unix time in seconds.
- `httpOnly` <[boolean]>
- `secure` <[boolean]>
- `session` <[boolean]>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about the opposite? Never return session, never set session. Rationale: expires defines session anyways.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair

- `sameSite` <"Strict"|"Lax"|"None">
- returns: <[Promise]>

Expand Down
6 changes: 6 additions & 0 deletions src/network.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ export type SetNetworkCookieParam = {
expires?: number,
httpOnly?: boolean,
secure?: boolean,
session?: boolean,
sameSite?: 'Strict' | 'Lax' | 'None'
};

Expand Down Expand Up @@ -66,11 +67,16 @@ export function filterCookies(cookies: NetworkCookie[], urls: string | string[]
export function rewriteCookies(cookies: SetNetworkCookieParam[]): SetNetworkCookieParam[] {
return cookies.map(c => {
assert(c.name, 'Cookie should have a name');
if (c.session === true)
assert(!c.expires, 'Session cookie cannot have an expiration date');
else if (c.session === false)
assert(c.expires && c.expires !== -1, 'Non-session cookie must have an expiration date');
assert(c.value, 'Cookie should have a value');
assert(c.url || (c.domain && c.path), 'Cookie should have a url or a domain/path pair');
assert(!(c.url && c.domain), 'Cookie should have either url or domain');
assert(!(c.url && c.path), 'Cookie should have either url or domain');
const copy = {...c};
delete copy.session;
if (copy.url) {
assert(copy.url !== 'about:blank', `Blank page can not have cookie "${c.name}"`);
assert(!copy.url.startsWith('data:'), `Data URL page can not have cookie "${c.name}"`);
Expand Down
8 changes: 6 additions & 2 deletions src/webkit/wkBrowser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -237,12 +237,16 @@ export class WKBrowserContext extends BrowserContextBase {
const { cookies } = await this._browser._browserSession.send('Browser.getAllCookies', { browserContextId: this._browserContextId });
return network.filterCookies(cookies.map((c: network.NetworkCookie) => ({
...c,
expires: c.expires === 0 ? -1 : c.expires
expires: c.expires === 0 ? -1 : c.expires / 1000,
})), urls);
}

async setCookies(cookies: network.SetNetworkCookieParam[]) {
const cc = network.rewriteCookies(cookies).map(c => ({ ...c, session: c.expires === -1 || c.expires === undefined })) as Protocol.Browser.SetCookieParam[];
const cc = network.rewriteCookies(cookies).map(c => ({
...c,
session: c.expires === -1 || c.expires === undefined,
expires: c.expires && c.expires !== -1 ? c.expires * 1000 : c.expires
})) as Protocol.Browser.SetCookieParam[];
await this._browser._browserSession.send('Browser.setCookies', { cookies: cc, browserContextId: this._browserContextId });
}

Expand Down
36 changes: 35 additions & 1 deletion test/cookies.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,26 @@ module.exports.describe = function({testRunner, expect, playwright, defaultBrows
sameSite: 'None',
}]);
});
it('should get a non-session cookie', async({context, page, server}) => {
await page.goto(server.EMPTY_PAGE);
// @see https://en.wikipedia.org/wiki/Year_2038_problem
const date = +(new Date('1/1/2038'));
await page.evaluate(timestamp => {
const date = new Date(timestamp);
document.cookie = `username=John Doe;expires=${date.toUTCString()}`;
}, date);
expect(await context.cookies()).toEqual([{
name: 'username',
value: 'John Doe',
domain: 'localhost',
path: '/',
expires: date / 1000,
httpOnly: false,
secure: false,
session: false,
sameSite: 'None',
}]);
});
it('should properly report httpOnly cookie', async({context, page, server}) => {
server.setRoute('/empty.html', (req, res) => {
res.setHeader('Set-Cookie', 'name=value;HttpOnly; Path=/');
Expand Down Expand Up @@ -157,6 +177,20 @@ module.exports.describe = function({testRunner, expect, playwright, defaultBrows
}]);
expect(await page.evaluate(() => document.cookie)).toEqual('password=123456');
});
it('should roundtrip cookie', async({context, page, server}) => {
await page.goto(server.EMPTY_PAGE);
// @see https://en.wikipedia.org/wiki/Year_2038_problem
const date = +(new Date('1/1/2038'));
await page.evaluate(timestamp => {
const date = new Date(timestamp);
document.cookie = `username=John Doe;expires=${date.toUTCString()}`;
}, date);
const cookies = await context.cookies();
await context.clearCookies();
expect(await context.cookies()).toEqual([]);
await context.setCookies(cookies);
expect(await context.cookies()).toEqual(cookies);
});
it('should send cookie header', async({server, context}) => {
let cookie = '';
server.setRoute('/empty.html', (req, res) => {
Expand Down Expand Up @@ -250,7 +284,7 @@ module.exports.describe = function({testRunner, expect, playwright, defaultBrows
it.slow()('should isolate cookies between launches', async({server}) => {
const browser1 = await playwright.launch(defaultBrowserOptions);
const context1 = await browser1.newContext();
await context1.setCookies([{url: server.EMPTY_PAGE, name: 'cookie-in-context-1', value: 'value', expires: Date.now() + 1000000000 }]);
await context1.setCookies([{url: server.EMPTY_PAGE, name: 'cookie-in-context-1', value: 'value', expires: Date.now() / 1000 + 10000}]);
await browser1.close();

const browser2 = await playwright.launch(defaultBrowserOptions);
Expand Down