cherrypick(release-1.7): accept path in save/load storage apis (#4717)

dgozman · web-flow · commit 4d7fb28dadca · 2020-12-14T16:43:16.000-08:00
Cherry-pick: PR #4714 SHA 355a58e
diff --git a/docs-src/api-body.md b/docs-src/api-body.md
@@ -545,6 +545,11 @@ Whether to emulate network being offline for the browser context.
 
 Returns storage state for this browser context, contains current cookies and local storage snapshot.
 
+### option: BrowserContext.storageState.path
+- `path` <[string]>
+
+The file path to save the storage state to. If `path` is a relative path, then it is resolved relative to [current working directory](https://nodejs.org/api/process.html#process_process_cwd). If no path is provided, storage state is still returned, but won't be saved to the disk.
+
 ## async method: BrowserContext.unroute
 
 Removes a route created with [browserContext.route()](). When `handler` is not specified, removes all routes for the
diff --git a/docs-src/api-params.md b/docs-src/api-params.md
@@ -115,7 +115,7 @@ Defaults to `'visible'`. Can be either:
 
 ## context-option-storage-state
 
-- `storageState` <[Object]>
+- `storageState` <[string]|[Object]>
   - `cookies` <[Array]<[Object]>> Optional cookies to set for context
     - `name` <[string]> **required**
     - `value` <[string]> **required**
@@ -133,7 +133,7 @@ Defaults to `'visible'`. Can be either:
       - `value` <[string]>
 
 Populates context with given storage state. This method can be used to initialize context with logged-in information
-obtained via [browserContext.storageState()]().
+obtained via [browserContext.storageState()](). Either a path to the file with saved storage, or an object with the following fields:
 
 ## context-option-acceptdownloads
 
diff --git a/docs/api.md b/docs/api.md
@@ -246,7 +246,7 @@ Indicates that the browser is connected.
     - `bypass` <[string]> Optional coma-separated domains to bypass proxy, for example `".com, chromium.org, .domain.com"`.
     - `username` <[string]> Optional username to use if HTTP proxy requires authentication.
     - `password` <[string]> Optional password to use if HTTP proxy requires authentication.
-  - `storageState` <[Object]> Populates context with given storage state. This method can be used to initialize context with logged-in information obtained via [browserContext.storageState()](#browsercontextstoragestate).
+  - `storageState` <[string]|[Object]> Populates context with given storage state. This method can be used to initialize context with logged-in information obtained via [browserContext.storageState([options])](#browsercontextstoragestateoptions). Either a path to the file with saved storage, or an object with the following fields:
     - `cookies` <[Array]<[Object]>> Optional cookies to set for context
       - `name` <[string]> **required**
       - `value` <[string]> **required**
@@ -321,7 +321,7 @@ Creates a new browser context. It won't share cookies/cache with other browser c
     - `bypass` <[string]> Optional coma-separated domains to bypass proxy, for example `".com, chromium.org, .domain.com"`.
     - `username` <[string]> Optional username to use if HTTP proxy requires authentication.
     - `password` <[string]> Optional password to use if HTTP proxy requires authentication.
-  - `storageState` <[Object]> Populates context with given storage state. This method can be used to initialize context with logged-in information obtained via [browserContext.storageState()](#browsercontextstoragestate).
+  - `storageState` <[string]|[Object]> Populates context with given storage state. This method can be used to initialize context with logged-in information obtained via [browserContext.storageState([options])](#browsercontextstoragestateoptions). Either a path to the file with saved storage, or an object with the following fields:
     - `cookies` <[Array]<[Object]>> Optional cookies to set for context
       - `name` <[string]> **required**
       - `value` <[string]> **required**
@@ -393,7 +393,7 @@ await context.close();
 - [browserContext.setGeolocation(geolocation)](#browsercontextsetgeolocationgeolocation)
 - [browserContext.setHTTPCredentials(httpCredentials)](#browsercontextsethttpcredentialshttpcredentials)
 - [browserContext.setOffline(offline)](#browsercontextsetofflineoffline)
-- [browserContext.storageState()](#browsercontextstoragestate)
+- [browserContext.storageState([options])](#browsercontextstoragestateoptions)
 - [browserContext.unroute(url[, handler])](#browsercontextunrouteurl-handler)
 - [browserContext.waitForEvent(event[, optionsOrPredicate])](#browsercontextwaitforeventevent-optionsorpredicate)
 <!-- GEN:stop -->
@@ -745,7 +745,9 @@ instead.
 - `offline` <[boolean]> Whether to emulate network being offline for the browser context.
 - returns: <[Promise]>
 
-#### browserContext.storageState()
+#### browserContext.storageState([options])
+- `options` <[Object]>
+  - `path` <[string]> The file path to save the storage state to. If `path` is a relative path, then it is resolved relative to [current working directory](https://nodejs.org/api/process.html#process_process_cwd). If no path is provided, storage state is still returned, but won't be saved to the disk.
 - returns: <[Promise]<[Object]>>
   - `cookies` <[Array]<[Object]>>
     - `name` <[string]>
@@ -5185,7 +5187,7 @@ const backgroundPage = await context.waitForEvent('backgroundpage');
 - [browserContext.setGeolocation(geolocation)](#browsercontextsetgeolocationgeolocation)
 - [browserContext.setHTTPCredentials(httpCredentials)](#browsercontextsethttpcredentialshttpcredentials)
 - [browserContext.setOffline(offline)](#browsercontextsetofflineoffline)
-- [browserContext.storageState()](#browsercontextstoragestate)
+- [browserContext.storageState([options])](#browsercontextstoragestateoptions)
 - [browserContext.unroute(url[, handler])](#browsercontextunrouteurl-handler)
 - [browserContext.waitForEvent(event[, optionsOrPredicate])](#browsercontextwaitforeventevent-optionsorpredicate)
 <!-- GEN:stop -->
diff --git a/src/client/android.ts b/src/client/android.ts
@@ -19,7 +19,7 @@ import * as util from 'util';
 import { isString } from '../utils/utils';
 import * as channels from '../protocol/channels';
 import { Events } from './events';
-import { BrowserContext, validateBrowserContextOptions } from './browserContext';
+import { BrowserContext, prepareBrowserContextOptions } from './browserContext';
 import { ChannelOwner } from './channelOwner';
 import * as apiInternal from '../../android-types-internal';
 import * as types from './types';
@@ -235,7 +235,7 @@ export class AndroidDevice extends ChannelOwner<channels.AndroidDeviceChannel, c
 
   async launchBrowser(options: types.BrowserContextOptions & { packageName?: string  } = {}): Promise<BrowserContext> {
     return this._wrapApiCall('androidDevice.launchBrowser', async () => {
-      const contextOptions = validateBrowserContextOptions(options);
+      const contextOptions = await prepareBrowserContextOptions(options);
       const { context } = await this._channel.launchBrowser(contextOptions);
       return BrowserContext.from(context);
     });
diff --git a/src/client/browser.ts b/src/client/browser.ts
@@ -15,7 +15,7 @@
  */
 
 import * as channels from '../protocol/channels';
-import { BrowserContext, validateBrowserContextOptions } from './browserContext';
+import { BrowserContext, prepareBrowserContextOptions } from './browserContext';
 import { Page } from './page';
 import { ChannelOwner } from './channelOwner';
 import { Events } from './events';
@@ -46,7 +46,7 @@ export class Browser extends ChannelOwner<channels.BrowserChannel, channels.Brow
     return this._wrapApiCall('browser.newContext', async () => {
       if (this._isRemote && options._tracePath)
         throw new Error(`"_tracePath" is not supported in connected browser`);
-      const contextOptions = validateBrowserContextOptions(options);
+      const contextOptions = await prepareBrowserContextOptions(options);
       const context = BrowserContext.from((await this._channel.newContext(contextOptions)).context);
       context._options = contextOptions;
       this._contexts.add(context);
diff --git a/src/client/browserContext.ts b/src/client/browserContext.ts
@@ -18,16 +18,21 @@
 import { Page, BindingCall, FunctionWithSource } from './page';
 import * as network from './network';
 import * as channels from '../protocol/channels';
+import * as util from 'util';
+import * as fs from 'fs';
 import { ChannelOwner } from './channelOwner';
 import { deprecate, evaluationScript, urlMatches } from './clientHelper';
 import { Browser } from './browser';
 import { Events } from './events';
 import { TimeoutSettings } from '../utils/timeoutSettings';
 import { Waiter } from './waiter';
 import { URLMatch, Headers, WaitForEventOptions, BrowserContextOptions, StorageState } from './types';
-import { isUnderTest, headersObjectToArray } from '../utils/utils';
+import { isUnderTest, headersObjectToArray, mkdirIfNeeded } from '../utils/utils';
 import { isSafeCloseError } from '../utils/errors';
 
+const fsWriteFileAsync = util.promisify(fs.writeFile.bind(fs));
+const fsReadFileAsync = util.promisify(fs.readFile.bind(fs));
+
 export class BrowserContext extends ChannelOwner<channels.BrowserContextChannel, channels.BrowserContextInitializer> {
   _pages = new Set<Page>();
   private _routes: { url: URLMatch, handler: network.RouteHandler }[] = [];
@@ -219,9 +224,14 @@ export class BrowserContext extends ChannelOwner<channels.BrowserContextChannel,
     return result;
   }
 
-  async storageState(): Promise<StorageState> {
+  async storageState(options: { path?: string } = {}): Promise<StorageState> {
     return await this._wrapApiCall('browserContext.storageState', async () => {
-      return await this._channel.storageState();
+      const state = await this._channel.storageState();
+      if (options.path) {
+        await mkdirIfNeeded(options.path);
+        await fsWriteFileAsync(options.path, JSON.stringify(state), 'utf8');
+      }
+      return state;
     });
   }
 
@@ -245,7 +255,7 @@ export class BrowserContext extends ChannelOwner<channels.BrowserContextChannel,
   }
 }
 
-export function validateBrowserContextOptions(options: BrowserContextOptions): channels.BrowserNewContextOptions {
+export async function prepareBrowserContextOptions(options: BrowserContextOptions): Promise<channels.BrowserNewContextOptions> {
   if (options.videoSize && !options.videosPath)
     throw new Error(`"videoSize" option requires "videosPath" to be specified`);
   if (options.extraHTTPHeaders)
@@ -255,6 +265,7 @@ export function validateBrowserContextOptions(options: BrowserContextOptions): c
     viewport: options.viewport === null ? undefined : options.viewport,
     noDefaultViewport: options.viewport === null,
     extraHTTPHeaders: options.extraHTTPHeaders ? headersObjectToArray(options.extraHTTPHeaders) : undefined,
+    storageState: typeof options.storageState === 'string' ? JSON.parse(await fsReadFileAsync(options.storageState, 'utf8')) : options.storageState,
   };
   if (!contextOptions.recordVideo && options.videosPath) {
     contextOptions.recordVideo = {
diff --git a/src/client/browserType.ts b/src/client/browserType.ts
@@ -16,7 +16,7 @@
 
 import * as channels from '../protocol/channels';
 import { Browser } from './browser';
-import { BrowserContext, validateBrowserContextOptions } from './browserContext';
+import { BrowserContext, prepareBrowserContextOptions } from './browserContext';
 import { ChannelOwner } from './channelOwner';
 import { LaunchOptions, LaunchServerOptions, ConnectOptions, LaunchPersistentContextOptions } from './types';
 import * as WebSocket from 'ws';
@@ -92,7 +92,7 @@ export class BrowserType extends ChannelOwner<channels.BrowserTypeChannel, chann
   async launchPersistentContext(userDataDir: string, options: LaunchPersistentContextOptions = {}): Promise<BrowserContext> {
     return this._wrapApiCall('browserType.launchPersistentContext', async () => {
       assert(!(options as any).port, 'Cannot specify a port without launching as a server.');
-      const contextOptions = validateBrowserContextOptions(options);
+      const contextOptions = await prepareBrowserContextOptions(options);
       const persistentOptions: channels.BrowserTypeLaunchPersistentContextParams = {
         ...contextOptions,
         ignoreDefaultArgs: Array.isArray(options.ignoreDefaultArgs) ? options.ignoreDefaultArgs : undefined,
diff --git a/src/client/types.ts b/src/client/types.ts
@@ -49,12 +49,13 @@ export type SetStorageState = {
 export type LifecycleEvent = 'load' | 'domcontentloaded' | 'networkidle';
 export const kLifecycleEvents: Set<LifecycleEvent> = new Set(['load', 'domcontentloaded', 'networkidle']);
 
-export type BrowserContextOptions = Omit<channels.BrowserNewContextOptions, 'viewport' | 'noDefaultViewport' | 'extraHTTPHeaders'> & {
+export type BrowserContextOptions = Omit<channels.BrowserNewContextOptions, 'viewport' | 'noDefaultViewport' | 'extraHTTPHeaders' | 'storageState'> & {
   viewport?: Size | null,
   extraHTTPHeaders?: Headers,
   logger?: Logger,
   videosPath?: string,
   videoSize?: Size,
+  storageState?: string | channels.BrowserNewContextOptions['storageState'],
 };
 
 type LaunchOverrides = {
diff --git a/src/server/firefox/ffBrowser.ts b/src/server/firefox/ffBrowser.ts
@@ -242,7 +242,11 @@ export class FFBrowserContext extends BrowserContext {
   }
 
   async addCookies(cookies: types.SetNetworkCookieParam[]) {
-    await this._browser._connection.send('Browser.setCookies', { browserContextId: this._browserContextId, cookies: network.rewriteCookies(cookies) });
+    const cc = network.rewriteCookies(cookies).map(c => ({
+      ...c,
+      expires: c.expires && c.expires !== -1 ? c.expires : undefined,
+    }));
+    await this._browser._connection.send('Browser.setCookies', { browserContextId: this._browserContextId, cookies: cc });
   }
 
   async clearCookies() {
diff --git a/test/browsercontext-add-cookies.spec.ts b/test/browsercontext-add-cookies.spec.ts
@@ -27,6 +27,24 @@ it('should work', async ({context, page, server}) => {
   expect(await page.evaluate(() => document.cookie)).toEqual('password=123456');
 });
 
+it('should work with expires=-1', async ({context, page}) => {
+  await context.addCookies([{
+    name: 'username',
+    value: 'John Doe',
+    domain: 'www.example.com',
+    path: '/',
+    expires: -1,
+    httpOnly: false,
+    secure: false,
+    sameSite: 'None',
+  }]);
+  await page.route('**/*', route => {
+    route.fulfill({ body: '<html></html>' }).catch(() => {});
+  });
+  await page.goto('https://www.example.com');
+  expect(await page.evaluate(() => document.cookie)).toEqual('username=John Doe');
+});
+
 it('should roundtrip cookie', async ({context, page, server}) => {
   await page.goto(server.EMPTY_PAGE);
   // @see https://en.wikipedia.org/wiki/Year_2038_problem
diff --git a/test/browsercontext-storage-state.spec.ts b/test/browsercontext-storage-state.spec.ts
@@ -16,6 +16,7 @@
  */
 
 import { it, expect } from './fixtures';
+import * as fs from 'fs';
 
 it('should capture local storage', async ({ context }) => {
   const page1 = await context.newPage();
@@ -60,7 +61,6 @@ it('should set local storage', async ({ browser }) => {
       ]
     }
   });
-  // await new Promise(f => setTimeout(f, 1000));
   const page = await context.newPage();
   await page.route('**/*', route => {
     route.fulfill({ body: '<html></html>' }).catch(() => {});
@@ -70,3 +70,33 @@ it('should set local storage', async ({ browser }) => {
   expect(localStorage).toEqual({ name1: 'value1' });
   await context.close();
 });
+
+it('should round-trip through the file', async ({ browser, context, testInfo }) => {
+  const page1 = await context.newPage();
+  await page1.route('**/*', route => {
+    route.fulfill({ body: '<html></html>' }).catch(() => {});
+  });
+  await page1.goto('https://www.example.com');
+  await page1.evaluate(() => {
+    localStorage['name1'] = 'value1';
+    document.cookie = 'username=John Doe';
+    return document.cookie;
+  });
+
+  const path = testInfo.outputPath('storage-state.json');
+  const state = await context.storageState({ path });
+  const written = await fs.promises.readFile(path, 'utf8');
+  expect(JSON.stringify(state)).toBe(written);
+
+  const context2 = await browser.newContext({ storageState: path });
+  const page2 = await context2.newPage();
+  await page2.route('**/*', route => {
+    route.fulfill({ body: '<html></html>' }).catch(() => {});
+  });
+  await page2.goto('https://www.example.com');
+  const localStorage = await page2.evaluate('window.localStorage');
+  expect(localStorage).toEqual({ name1: 'value1' });
+  const cookie = await page2.evaluate('document.cookie');
+  expect(cookie).toEqual('username=John Doe');
+  await context2.close();
+});

Original file line number	Diff line number	Diff line change
`@@ -242,7 +242,11 @@ export class FFBrowserContext extends BrowserContext {`
`242`	`242`	`}`
`243`	`243`
`244`	`244`	`async addCookies(cookies: types.SetNetworkCookieParam[]) {`
`245`		`- await this._browser._connection.send('Browser.setCookies', { browserContextId: this._browserContextId, cookies: network.rewriteCookies(cookies) });`
	`245`	`+ const cc = network.rewriteCookies(cookies).map(c => ({`
	`246`	`+ ...c,`
	`247`	`+ expires: c.expires && c.expires !== -1 ? c.expires : undefined,`
	`248`	`+ }));`
	`249`	`+ await this._browser._connection.send('Browser.setCookies', { browserContextId: this._browserContextId, cookies: cc });`
`246`	`250`	`}`
`247`	`251`
`248`	`252`	`async clearCookies() {`