feat(cookies): implement cookies across the board (#121)

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<NetworkCookie[]> {
     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 => {
+    cookies = rewriteCookies(cookies);
-      const item = Object.assign({}, cookie);
+    await this._browser._client.send('Storage.setCookies', { cookies, browserContextId: this._id || undefined });
-      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 });
   }
 
   async close() {

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 => {
+    cookies = rewriteCookies(cookies);
-      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}"`);
-    });
     await this._connection.send('Browser.setCookies', {
       browserContextId: this._browserContextId || undefined,
       cookies

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 {

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<string, BrowserContext>();
   _targets = new Map<string, Target>();
   private _eventListeners: RegisteredListener[];
+  _waitForFirstTarget: Promise<void>;
+  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<string> {
@@ -115,7 +118,7 @@ export class Browser extends EventEmitter {
   }
 
   async _createPageInContext(browserContextId?: string): Promise<Page> {
-    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<Page[]> {
+    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<NetworkCookie[]> {
-    const page = (await this.pages())[0];
+    const { cookies } = await this._browser._connection.send('Browser.getAllCookies', { browserContextId: this._id });
-    const response = await page._session.send('Page.getCookies');
+    return filterCookies(cookies.map((c: NetworkCookie) => ({
-    const cookies = response.cookies.map(cookie => {
+      ...c,
-      // Webkit returns 0 for a cookie without an expiration
+      expires: c.expires === 0 ? -1 : c.expires
-      if (cookie.expires === 0)
+    })), urls);
-        cookie.expires = -1;
+  }
-      return cookie;
+
-    });
+  async setCookies(cookies: SetNetworkCookieParam[]) {
-    return filterCookies(cookies, urls);
+    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];
+    await this._browser._connection.send('Browser.deleteAllCookies', { browserContextId: this._id });
-    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);
   }
 }

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() {
+  describe('BrowserContext.setCookies', function() {
-    it.skip(WEBKIT)('should clear cookies', async({context, page, server}) => {
+    it('should clear cookies', async({context, page, server}) => {
       await page.goto(server.EMPTY_PAGE);
       await context.setCookies([{
         url: server.EMPTY_PAGE,