diff --git a/src/chromium/BrowserContext.ts b/src/chromium/BrowserContext.ts index 5d5c950aa8..5f0ffc6c62 100644 --- a/src/chromium/BrowserContext.ts +++ b/src/chromium/BrowserContext.ts @@ -17,7 +17,7 @@ import { EventEmitter } from 'events'; import { assert } from '../helper'; -import { NetworkCookie, SetNetworkCookieParam, filterCookies } from '../network'; +import { filterCookies, NetworkCookie, rewriteCookies, SetNetworkCookieParam } from '../network'; import { Browser } from './Browser'; import { CDPSession } from './Connection'; import { Permissions } from './features/permissions'; @@ -68,7 +68,10 @@ export class BrowserContext extends EventEmitter { async cookies(...urls: string[]): Promise { const { cookies } = await this._browser._client.send('Storage.getCookies', { browserContextId: this._id || undefined }); - return filterCookies(cookies.map(c => ({ sameSite: 'None', ...c })), urls); + return filterCookies(cookies.map(c => { + const copy = { sameSite: 'None', ...c, size: undefined } as NetworkCookie; + return copy; + }), urls); } async clearCookies() { @@ -76,13 +79,8 @@ export class BrowserContext extends EventEmitter { } async setCookies(cookies: SetNetworkCookieParam[]) { - const items = cookies.map(cookie => { - const item = Object.assign({}, cookie); - assert(item.url !== 'about:blank', `Blank page can not have cookie "${item.name}"`); - assert(!String.prototype.startsWith.call(item.url || '', 'data:'), `Data URL page can not have cookie "${item.name}"`); - return item; - }); - await this._browser._client.send('Storage.setCookies', { cookies: items, browserContextId: this._id || undefined }); + cookies = rewriteCookies(cookies); + await this._browser._client.send('Storage.setCookies', { cookies, browserContextId: this._id || undefined }); } async close() { diff --git a/src/firefox/Browser.ts b/src/firefox/Browser.ts index 984dd169b7..0c8f6e122e 100644 --- a/src/firefox/Browser.ts +++ b/src/firefox/Browser.ts @@ -17,7 +17,7 @@ import { EventEmitter } from 'events'; import { assert, helper, RegisteredListener } from '../helper'; -import { filterCookies, NetworkCookie, SetNetworkCookieParam } from '../network'; +import { filterCookies, NetworkCookie, SetNetworkCookieParam, rewriteCookies } from '../network'; import { Connection, ConnectionEvents } from './Connection'; import { Events } from './events'; import { Permissions } from './features/permissions'; @@ -304,11 +304,7 @@ export class BrowserContext extends EventEmitter { } async setCookies(cookies: SetNetworkCookieParam[]) { - cookies.forEach(cookie => { - const item = Object.assign({}, cookie); - assert(item.url !== 'about:blank', `Blank page can not have cookie "${item.name}"`); - assert(!String.prototype.startsWith.call(item.url || '', 'data:'), `Data URL page can not have cookie "${item.name}"`); - }); + cookies = rewriteCookies(cookies); await this._connection.send('Browser.setCookies', { browserContextId: this._browserContextId || undefined, cookies diff --git a/src/network.ts b/src/network.ts index 52ce335654..456bebc0ad 100644 --- a/src/network.ts +++ b/src/network.ts @@ -2,6 +2,7 @@ // Licensed under the MIT license. import * as frames from './frames'; +import { assert } from './helper'; export type NetworkCookie = { name: string, @@ -9,7 +10,6 @@ export type NetworkCookie = { domain: string, path: string, expires: number, - size: number, httpOnly: boolean, secure: boolean, session: boolean, @@ -47,6 +47,26 @@ export function filterCookies(cookies: NetworkCookie[], urls: string[]) { }); } +export function rewriteCookies(cookies: SetNetworkCookieParam[]): SetNetworkCookieParam[] { + return cookies.map(c => { + assert(c.name, "Cookie should have a name"); + 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}; + 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}"`); + const url = new URL(copy.url); + copy.domain = url.hostname; + copy.path = url.pathname.substring(0, url.pathname.lastIndexOf('/') + 1); + copy.secure = url.protocol === 'https:'; + } + return copy; + }); +} + export type Headers = { [key: string]: string }; export class Request { diff --git a/src/webkit/Browser.ts b/src/webkit/Browser.ts index dc9169a6dd..9ea7af0b8c 100644 --- a/src/webkit/Browser.ts +++ b/src/webkit/Browser.ts @@ -18,7 +18,7 @@ import * as childProcess from 'child_process'; import { EventEmitter } from 'events'; import { assert, helper, RegisteredListener } from '../helper'; -import { filterCookies, NetworkCookie } from '../network'; +import { filterCookies, NetworkCookie, rewriteCookies, SetNetworkCookieParam } from '../network'; import { Connection } from './Connection'; import { Events } from './events'; import { Page, Viewport } from './Page'; @@ -35,6 +35,8 @@ export class Browser extends EventEmitter { private _contexts = new Map(); _targets = new Map(); private _eventListeners: RegisteredListener[]; + _waitForFirstTarget: Promise; + private _waitForFirstTargetCallback: () => void; static async create( connection: Connection, @@ -71,6 +73,7 @@ export class Browser extends EventEmitter { // Taking multiple screenshots in parallel doesn't work well, so we serialize them. this._screenshotTaskQueue = new TaskQueue(); + this._waitForFirstTarget = new Promise(f => this._waitForFirstTargetCallback = f); } async userAgent(): Promise { @@ -115,7 +118,7 @@ export class Browser extends EventEmitter { } async _createPageInContext(browserContextId?: string): Promise { - const {targetId} = await this._connection.send('Browser.createPage', {browserContextId}); + const { targetId } = await this._connection.send('Browser.createPage', { browserContextId }); const target = this._targets.get(targetId); return await target.page(); } @@ -173,6 +176,7 @@ export class Browser extends EventEmitter { this._targets.set(targetInfo.targetId, target); this.emit(Events.Browser.TargetCreated, target); context.emit(Events.BrowserContext.TargetCreated, target); + this._waitForFirstTargetCallback(); } _onTargetDestroyed({targetId}) { @@ -232,6 +236,7 @@ export class BrowserContext extends EventEmitter { } async pages(): Promise { + await this._browser._waitForFirstTarget; const pages = await Promise.all( this.targets() .filter(target => target.type() === 'page') @@ -258,28 +263,20 @@ export class BrowserContext extends EventEmitter { } async cookies(...urls: string[]): Promise { - const page = (await this.pages())[0]; - const response = await page._session.send('Page.getCookies'); - const cookies = response.cookies.map(cookie => { - // Webkit returns 0 for a cookie without an expiration - if (cookie.expires === 0) - cookie.expires = -1; - return cookie; - }); - return filterCookies(cookies, urls); + const { cookies } = await this._browser._connection.send('Browser.getAllCookies', { browserContextId: this._id }); + return filterCookies(cookies.map((c: NetworkCookie) => ({ + ...c, + expires: c.expires === 0 ? -1 : c.expires + })), urls); + } + + async setCookies(cookies: SetNetworkCookieParam[]) { + cookies = rewriteCookies(cookies); + const cc = cookies.map(c => ({ ...c, session: c.expires === -1 || c.expires === undefined })); + await this._browser._connection.send('Browser.setCookies', { cookies: cc, browserContextId: this._id }); } async clearCookies() { - const page = (await this.pages())[0]; - const response = await page._session.send('Page.getCookies'); - const promises = []; - for (const cookie of response.cookies) { - const item = { - cookieName: cookie.name, - url: (cookie.secure ? 'https://' : 'http://') + cookie.domain + cookie.path - }; - promises.push(page._session.send('Page.deleteCookie', item)); - } - await Promise.all(promises); + await this._browser._connection.send('Browser.deleteAllCookies', { browserContextId: this._id }); } } diff --git a/test/cookies.spec.js b/test/cookies.spec.js index 0e6976db7c..a44b66faec 100644 --- a/test/cookies.spec.js +++ b/test/cookies.spec.js @@ -21,7 +21,6 @@ module.exports.addTests = function({testRunner, expect, FFOX, CHROME, WEBKIT}) { describe('BrowserContext.cookies', function() { it('should return no cookies in pristine browser context', async({context, page, server}) => { - await page.goto(server.EMPTY_PAGE); expect(await context.cookies()).toEqual([]); }); it('should get a cookie', async({context, page, server}) => { @@ -35,7 +34,6 @@ module.exports.addTests = function({testRunner, expect, FFOX, CHROME, WEBKIT}) { domain: 'localhost', path: '/', expires: -1, - size: 16, httpOnly: false, secure: false, session: true, @@ -44,7 +42,7 @@ module.exports.addTests = function({testRunner, expect, FFOX, CHROME, WEBKIT}) { }); it('should properly report httpOnly cookie', async({context, page, server}) => { server.setRoute('/empty.html', (req, res) => { - res.setHeader('Set-Cookie', ';HttpOnly; Path=/'); + res.setHeader('Set-Cookie', 'name=value;HttpOnly; Path=/'); res.end(); }); await page.goto(server.EMPTY_PAGE); @@ -52,9 +50,9 @@ module.exports.addTests = function({testRunner, expect, FFOX, CHROME, WEBKIT}) { expect(cookies.length).toBe(1); expect(cookies[0].httpOnly).toBe(true); }); - it.skip(WEBKIT)('should properly report "Strict" sameSite cookie', async({context, page, server}) => { + it.skip(WEBKIT && process.platform === 'linux')('should properly report "Strict" sameSite cookie', async({context, page, server}) => { server.setRoute('/empty.html', (req, res) => { - res.setHeader('Set-Cookie', ';SameSite=Strict'); + res.setHeader('Set-Cookie', 'name=value;SameSite=Strict'); res.end(); }); await page.goto(server.EMPTY_PAGE); @@ -62,9 +60,9 @@ module.exports.addTests = function({testRunner, expect, FFOX, CHROME, WEBKIT}) { expect(cookies.length).toBe(1); expect(cookies[0].sameSite).toBe('Strict'); }); - it.skip(WEBKIT)('should properly report "Lax" sameSite cookie', async({context, page, server}) => { + it.skip(WEBKIT && process.platform === 'linux')('should properly report "Lax" sameSite cookie', async({context, page, server}) => { server.setRoute('/empty.html', (req, res) => { - res.setHeader('Set-Cookie', ';SameSite=Lax'); + res.setHeader('Set-Cookie', 'name=value;SameSite=Lax'); res.end(); }); await page.goto(server.EMPTY_PAGE); @@ -87,7 +85,6 @@ module.exports.addTests = function({testRunner, expect, FFOX, CHROME, WEBKIT}) { domain: 'localhost', path: '/', expires: -1, - size: 12, httpOnly: false, secure: false, session: true, @@ -99,7 +96,6 @@ module.exports.addTests = function({testRunner, expect, FFOX, CHROME, WEBKIT}) { domain: 'localhost', path: '/', expires: -1, - size: 16, httpOnly: false, secure: false, session: true, @@ -107,7 +103,7 @@ module.exports.addTests = function({testRunner, expect, FFOX, CHROME, WEBKIT}) { }, ]); }); - it.skip(WEBKIT)('should get cookies from multiple urls', async({context}) => { + it('should get cookies from multiple urls', async({context}) => { await context.setCookies([{ url: 'https://foo.com', name: 'doggo', @@ -129,7 +125,6 @@ module.exports.addTests = function({testRunner, expect, FFOX, CHROME, WEBKIT}) { domain: 'baz.com', path: '/', expires: -1, - size: 11, httpOnly: false, secure: true, session: true, @@ -140,7 +135,6 @@ module.exports.addTests = function({testRunner, expect, FFOX, CHROME, WEBKIT}) { domain: 'foo.com', path: '/', expires: -1, - size: 10, httpOnly: false, secure: true, session: true, @@ -149,7 +143,7 @@ module.exports.addTests = function({testRunner, expect, FFOX, CHROME, WEBKIT}) { }); }); - describe.skip(WEBKIT)('BrowserContext.setCookies', function() { + describe('BrowserContext.setCookies', function() { it('should work', async({context, page, server}) => { await page.goto(server.EMPTY_PAGE); await context.setCookies([{ @@ -216,7 +210,6 @@ module.exports.addTests = function({testRunner, expect, FFOX, CHROME, WEBKIT}) { domain: 'localhost', path: '/', expires: -1, - size: 14, httpOnly: false, secure: false, session: true, @@ -237,7 +230,6 @@ module.exports.addTests = function({testRunner, expect, FFOX, CHROME, WEBKIT}) { domain: 'localhost', path: '/grid.html', expires: -1, - size: 14, httpOnly: false, secure: false, session: true, @@ -308,14 +300,13 @@ module.exports.addTests = function({testRunner, expect, FFOX, CHROME, WEBKIT}) { domain: 'www.example.com', path: '/', expires: -1, - size: 18, httpOnly: false, secure: true, session: true, sameSite: 'None', }]); }); - it('should set cookies from a frame', async({context, page, server}) => { + it.skip(WEBKIT)('should set cookies from a frame', async({context, page, server}) => { await page.goto(server.PREFIX + '/grid.html'); await context.setCookies([ {url: server.PREFIX, name: 'localhost-cookie', value: 'best'}, @@ -330,6 +321,7 @@ module.exports.addTests = function({testRunner, expect, FFOX, CHROME, WEBKIT}) { iframe.src = src; return promise; }, server.CROSS_PROCESS_PREFIX); + expect(await page.evaluate('document.cookie')).toBe('localhost-cookie=best'); expect(await page.frames()[1].evaluate('document.cookie')).toBe('127-cookie=worst'); @@ -339,7 +331,6 @@ module.exports.addTests = function({testRunner, expect, FFOX, CHROME, WEBKIT}) { domain: 'localhost', path: '/', expires: -1, - size: 20, httpOnly: false, secure: false, session: true, @@ -352,7 +343,6 @@ module.exports.addTests = function({testRunner, expect, FFOX, CHROME, WEBKIT}) { domain: '127.0.0.1', path: '/', expires: -1, - size: 15, httpOnly: false, secure: false, session: true, @@ -361,8 +351,8 @@ module.exports.addTests = function({testRunner, expect, FFOX, CHROME, WEBKIT}) { }); }); - describe.skip(WEBKIT)('BrowserContext.setCookies', function() { - it.skip(WEBKIT)('should clear cookies', async({context, page, server}) => { + describe('BrowserContext.setCookies', function() { + it('should clear cookies', async({context, page, server}) => { await page.goto(server.EMPTY_PAGE); await context.setCookies([{ url: server.EMPTY_PAGE,