From d1ef0c8694d9d8b93aa025f4d369b3c583c1aa67 Mon Sep 17 00:00:00 2001 From: Andrey Lushnikov Date: Sat, 7 Mar 2020 08:41:57 -0800 Subject: [PATCH] fix(wk,ff): properly support getting and setting non-session cookies (#1280) --- docs/api.md | 1 - src/chromium/crBrowser.ts | 1 + src/firefox/ffBrowser.ts | 1 + src/network.ts | 1 - src/webkit/wkBrowser.ts | 16 +++++++---- test/cookies.spec.js | 46 ++++++++++++++++++++++-------- test/defaultbrowsercontext.spec.js | 2 -- 7 files changed, 47 insertions(+), 21 deletions(-) diff --git a/docs/api.md b/docs/api.md index 82c25aaec1..c2e488d118 100644 --- a/docs/api.md +++ b/docs/api.md @@ -383,7 +383,6 @@ will be closed. - `size` <[number]> - `httpOnly` <[boolean]> - `secure` <[boolean]> - - `session` <[boolean]> - `sameSite` <"Strict"|"Lax"|"None"> If no URLs are specified, this method returns all cookies. diff --git a/src/chromium/crBrowser.ts b/src/chromium/crBrowser.ts index aca7c7d966..790a91eae8 100644 --- a/src/chromium/crBrowser.ts +++ b/src/chromium/crBrowser.ts @@ -289,6 +289,7 @@ export class CRBrowserContext extends BrowserContextBase { const copy: any = { sameSite: 'None', ...c }; delete copy.size; delete copy.priority; + delete copy.session; return copy as network.NetworkCookie; }), urls); } diff --git a/src/firefox/ffBrowser.ts b/src/firefox/ffBrowser.ts index a67b6b15dd..b92d260f0a 100644 --- a/src/firefox/ffBrowser.ts +++ b/src/firefox/ffBrowser.ts @@ -324,6 +324,7 @@ export class FFBrowserContext extends BrowserContextBase { return network.filterCookies(cookies.map(c => { const copy: any = { ... c }; delete copy.size; + delete copy.session; return copy as network.NetworkCookie; }), urls); } diff --git a/src/network.ts b/src/network.ts index cfc60dd6b5..39b37a4bff 100644 --- a/src/network.ts +++ b/src/network.ts @@ -26,7 +26,6 @@ export type NetworkCookie = { expires: number, httpOnly: boolean, secure: boolean, - session: boolean, sameSite: 'Strict' | 'Lax' | 'None' }; diff --git a/src/webkit/wkBrowser.ts b/src/webkit/wkBrowser.ts index 4dc83599b6..532a1ca329 100644 --- a/src/webkit/wkBrowser.ts +++ b/src/webkit/wkBrowser.ts @@ -235,14 +235,20 @@ export class WKBrowserContext extends BrowserContextBase { async cookies(urls?: string | string[]): Promise { 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 - })), urls); + return network.filterCookies(cookies.map((c: network.NetworkCookie) => { + const copy: any = { ... c }; + copy.expires = c.expires === 0 ? -1 : c.expires / 1000; + delete copy.session; + return copy as network.NetworkCookie; + }), 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 }); } diff --git a/test/cookies.spec.js b/test/cookies.spec.js index 2f4a64b6ad..6a67ccb27b 100644 --- a/test/cookies.spec.js +++ b/test/cookies.spec.js @@ -40,7 +40,25 @@ module.exports.describe = function({testRunner, expect, playwright, defaultBrows expires: -1, httpOnly: false, secure: false, - session: true, + 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, sameSite: 'None', }]); }); @@ -91,7 +109,6 @@ module.exports.describe = function({testRunner, expect, playwright, defaultBrows expires: -1, httpOnly: false, secure: false, - session: true, sameSite: 'None', }, { @@ -102,7 +119,6 @@ module.exports.describe = function({testRunner, expect, playwright, defaultBrows expires: -1, httpOnly: false, secure: false, - session: true, sameSite: 'None', }, ]); @@ -131,7 +147,6 @@ module.exports.describe = function({testRunner, expect, playwright, defaultBrows expires: -1, httpOnly: false, secure: true, - session: true, sameSite: 'None', }, { name: 'doggo', @@ -141,7 +156,6 @@ module.exports.describe = function({testRunner, expect, playwright, defaultBrows expires: -1, httpOnly: false, secure: true, - session: true, sameSite: 'None', }]); }); @@ -157,6 +171,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) => { @@ -250,7 +278,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); @@ -285,7 +313,6 @@ module.exports.describe = function({testRunner, expect, playwright, defaultBrows value: '123456' }]); const cookies = await context.cookies(); - expect(cookies[0].session).toBe(true); expect(cookies[0].expires).toBe(-1); }); it('should set cookie with reasonable defaults', async({context, server}) => { @@ -303,7 +330,6 @@ module.exports.describe = function({testRunner, expect, playwright, defaultBrows expires: -1, httpOnly: false, secure: false, - session: true, sameSite: 'None', }]); }); @@ -323,7 +349,6 @@ module.exports.describe = function({testRunner, expect, playwright, defaultBrows expires: -1, httpOnly: false, secure: false, - session: true, sameSite: 'None', }]); expect(await page.evaluate('document.cookie')).toBe('gridcookie=GRID'); @@ -393,7 +418,6 @@ module.exports.describe = function({testRunner, expect, playwright, defaultBrows expires: -1, httpOnly: false, secure: true, - session: true, sameSite: 'None', }]); }); @@ -424,7 +448,6 @@ module.exports.describe = function({testRunner, expect, playwright, defaultBrows expires: -1, httpOnly: false, secure: false, - session: true, sameSite: 'None', }]); @@ -436,7 +459,6 @@ module.exports.describe = function({testRunner, expect, playwright, defaultBrows expires: -1, httpOnly: false, secure: false, - session: true, sameSite: 'None', }]); }); diff --git a/test/defaultbrowsercontext.spec.js b/test/defaultbrowsercontext.spec.js index 51ed6fae50..d2981bde64 100644 --- a/test/defaultbrowsercontext.spec.js +++ b/test/defaultbrowsercontext.spec.js @@ -50,7 +50,6 @@ module.exports.describe = function ({ testRunner, expect, defaultBrowserOptions, expires: -1, httpOnly: false, secure: false, - session: true, sameSite: 'None', }]); }); @@ -70,7 +69,6 @@ module.exports.describe = function ({ testRunner, expect, defaultBrowserOptions, expires: -1, httpOnly: false, secure: false, - session: true, sameSite: 'None', }]); });