From 94c32e4bc8aedfef48c7ac12cd476bf5524346d7 Mon Sep 17 00:00:00 2001 From: JoelEinbinder Date: Tue, 3 Apr 2018 15:05:27 -0700 Subject: [PATCH] feat(Launcher): introduce `pipe` option (#2288) This patch introduces a new `pipe` option to the launcher to connect over a pipe. In certain environments, exposing web socket for remote debugging is a security risk. Pipe connection eliminates this risk. --- docs/api.md | 1 + lib/Browser.js | 12 +++++++++--- lib/Launcher.js | 35 ++++++++++++++++++++++++++++++----- lib/Page.js | 6 +++--- lib/Target.js | 8 ++++---- test/puppeteer.spec.js | 29 +++++++++++++++++++++++++++++ 6 files changed, 76 insertions(+), 15 deletions(-) diff --git a/docs/api.md b/docs/api.md index c27bba1078d..a238a4efe56 100644 --- a/docs/api.md +++ b/docs/api.md @@ -327,6 +327,7 @@ This methods attaches Puppeteer to an existing Chromium instance. - `userDataDir` <[string]> Path to a [User Data Directory](https://chromium.googlesource.com/chromium/src/+/master/docs/user_data_dir.md). - `env` <[Object]> Specify environment variables that will be visible to the browser. Defaults to `process.env`. - `devtools` <[boolean]> Whether to auto-open a DevTools panel for each tab. If this option is `true`, the `headless` option will be set `false`. + - `pipe` <[boolean]> Connects to the browser over a pipe instead of a WebSocket. Defaults to `false`. - returns: <[Promise]<[Browser]>> Promise which resolves to browser instance. The method launches a browser instance with given arguments. The browser will be closed when the parent node.js process is closed. diff --git a/lib/Browser.js b/lib/Browser.js index 6e1e3e7ce6c..f33c1c04a49 100644 --- a/lib/Browser.js +++ b/lib/Browser.js @@ -22,7 +22,7 @@ const TaskQueue = require('./TaskQueue'); class Browser extends EventEmitter { /** * @param {!Puppeteer.Connection} connection - * @param {!Object=} options + * @param {!BrowserOptions=} options * @param {?Puppeteer.ChildProcess} process * @param {(function():Promise)=} closeCallback */ @@ -53,7 +53,7 @@ class Browser extends EventEmitter { /** * @param {!Puppeteer.Connection} connection - * @param {!Object=} options + * @param {!BrowserOptions=} options * @param {?Puppeteer.ChildProcess} process * @param {function()=} closeCallback */ @@ -68,7 +68,7 @@ class Browser extends EventEmitter { */ async _targetCreated(event) { const targetInfo = event.targetInfo; - const target = new Target(targetInfo, () => this._connection.createSession(targetInfo.targetId), this._ignoreHTTPSErrors, this._appMode, this._screenshotTaskQueue); + const target = new Target(targetInfo, () => this._connection.createSession(targetInfo.targetId), this._ignoreHTTPSErrors, !this._appMode, this._screenshotTaskQueue); console.assert(!this._targets.has(event.targetInfo.targetId), 'Target should not exist before targetCreated'); this._targets.set(event.targetInfo.targetId, target); @@ -178,3 +178,9 @@ Browser.Events = { helper.tracePublicAPI(Browser); module.exports = Browser; + +/** + * @typedef {Object} BrowserOptions + * @property {boolean=} appMode + * @property {boolean=} ignoreHTTPSErrors + */ \ No newline at end of file diff --git a/lib/Launcher.js b/lib/Launcher.js index 1d39ff79329..af88265cf79 100644 --- a/lib/Launcher.js +++ b/lib/Launcher.js @@ -50,12 +50,11 @@ const AUTOMATION_ARGS = [ '--enable-automation', '--password-store=basic', '--use-mock-keychain', - '--remote-debugging-port=0', ]; class Launcher { /** - * @param {!Object=} options + * @param {!LaunchOptions=} options * @return {!Promise} */ static async launch(options) { @@ -67,11 +66,14 @@ class Launcher { chromeArguments.push(...DEFAULT_ARGS); if (options.appMode) { options.headless = false; - chromeArguments.push('--remote-debugging-pipe'); + options.pipe = true; } else if (!options.ignoreDefaultArgs) { chromeArguments.push(...AUTOMATION_ARGS); } + if (!options.ignoreDefaultArgs || !chromeArguments.some(argument => argument.startsWith('--remote-debugging-'))) + chromeArguments.push(options.pipe ? '--remote-debugging-pipe' : '--remote-debugging-port=0'); + if (!options.args || !options.args.some(arg => arg.startsWith('--user-data-dir'))) { if (!options.userDataDir) temporaryUserDataDir = await mkdtempAsync(CHROME_PROFILE_PATH); @@ -100,8 +102,10 @@ class Launcher { if (Array.isArray(options.args)) chromeArguments.push(...options.args); + + const usePipe = chromeArguments.includes('--remote-debugging-pipe'); const stdio = ['pipe', 'pipe', 'pipe']; - if (options.appMode) + if (usePipe) stdio.push('pipe', 'pipe'); const chromeProcess = childProcess.spawn( chromeExecutable, @@ -147,7 +151,7 @@ class Launcher { let connection = null; try { const connectionDelay = options.slowMo || 0; - if (!options.appMode) { + if (!usePipe) { const timeout = helper.isNumber(options.timeout) ? options.timeout : 30000; const browserWSEndpoint = await waitForWSEndpoint(chromeProcess, timeout); connection = await Connection.createForWebSocket(browserWSEndpoint, connectionDelay); @@ -277,4 +281,25 @@ function waitForWSEndpoint(chromeProcess, timeout) { }); } +/** + * @typedef {Object} LaunchOptions + * @property {boolean=} ignoreHTTPSErrors + * @property {boolean=} headless + * @property {string=} executablePath + * @property {number=} slowMo + * @property {!Array=} args + * @property {boolean=} ignoreDefaultArgs + * @property {boolean=} handleSIGINT + * @property {boolean=} handleSIGTERM + * @property {boolean=} handleSIGHUP + * @property {number=} timeout + * @property {boolean=} dumpio + * @property {string=} userDataDir + * @property {!Object=} env + * @property {boolean=} devtools + * @property {boolean=} pipe + * @property {boolean=} appMode + */ + + module.exports = Launcher; diff --git a/lib/Page.js b/lib/Page.js index 56554f160ea..bb5fb3f42dc 100644 --- a/lib/Page.js +++ b/lib/Page.js @@ -34,11 +34,11 @@ class Page extends EventEmitter { * @param {!Puppeteer.CDPSession} client * @param {!Puppeteer.Target} target * @param {boolean} ignoreHTTPSErrors - * @param {boolean} appMode + * @param {boolean} setDefaultViewport * @param {!Puppeteer.TaskQueue} screenshotTaskQueue * @return {!Promise} */ - static async create(client, target, ignoreHTTPSErrors, appMode, screenshotTaskQueue) { + static async create(client, target, ignoreHTTPSErrors, setDefaultViewport, screenshotTaskQueue) { await client.send('Page.enable'); const {frameTree} = await client.send('Page.getFrameTree'); @@ -54,7 +54,7 @@ class Page extends EventEmitter { if (ignoreHTTPSErrors) await client.send('Security.setOverrideCertificateErrors', {override: true}); // Initialize default page size. - if (!appMode) + if (setDefaultViewport) await page.setViewport({width: 800, height: 600}); return page; diff --git a/lib/Target.js b/lib/Target.js index 2df89384e1e..1b8f606cd5e 100644 --- a/lib/Target.js +++ b/lib/Target.js @@ -6,15 +6,15 @@ class Target { * @param {!Puppeteer.TargetInfo} targetInfo * @param {!function():!Promise} sessionFactory * @param {boolean} ignoreHTTPSErrors - * @param {boolean} appMode + * @param {boolean} setDefaultViewport * @param {!Puppeteer.TaskQueue} screenshotTaskQueue */ - constructor(targetInfo, sessionFactory, ignoreHTTPSErrors, appMode, screenshotTaskQueue) { + constructor(targetInfo, sessionFactory, ignoreHTTPSErrors, setDefaultViewport, screenshotTaskQueue) { this._targetInfo = targetInfo; this._targetId = targetInfo.targetId; this._sessionFactory = sessionFactory; this._ignoreHTTPSErrors = ignoreHTTPSErrors; - this._appMode = appMode; + this._setDefaultViewport = setDefaultViewport; this._screenshotTaskQueue = screenshotTaskQueue; /** @type {?Promise} */ this._pagePromise = null; @@ -38,7 +38,7 @@ class Target { async page() { if (this._targetInfo.type === 'page' && !this._pagePromise) { this._pagePromise = this._sessionFactory() - .then(client => Page.create(client, this, this._ignoreHTTPSErrors, this._appMode, this._screenshotTaskQueue)); + .then(client => Page.create(client, this, this._ignoreHTTPSErrors, this._setDefaultViewport, this._screenshotTaskQueue)); } return this._pagePromise; } diff --git a/test/puppeteer.spec.js b/test/puppeteer.spec.js index e8ec2e1c59e..8045b37e18a 100644 --- a/test/puppeteer.spec.js +++ b/test/puppeteer.spec.js @@ -219,6 +219,35 @@ module.exports.addTests = function({testRunner, expect, defaultBrowserOptions, p process.kill(res.pid); await Promise.all(promises); }); + it('should support the pipe option', async() => { + const options = Object.assign({pipe: true}, defaultBrowserOptions); + const browser = await puppeteer.launch(options); + expect(browser.wsEndpoint()).toBe(''); + const page = await browser.newPage(); + expect(await page.evaluate('11 * 11')).toBe(121); + await page.close(); + await browser.close(); + }); + it('should support the pipe argument', async() => { + const options = Object.assign({}, defaultBrowserOptions); + options.ignoreDefaultArgs = true; + options.args = ['--remote-debugging-pipe'].concat(options.args); + const browser = await puppeteer.launch(options); + expect(browser.wsEndpoint()).toBe(''); + const page = await browser.newPage(); + expect(await page.evaluate('11 * 11')).toBe(121); + await page.close(); + await browser.close(); + }); + it('should work with no default arguments', async() => { + const options = Object.assign({}, defaultBrowserOptions); + options.ignoreDefaultArgs = true; + const browser = await puppeteer.launch(options); + const page = await browser.newPage(); + expect(await page.evaluate('11 * 11')).toBe(121); + await page.close(); + await browser.close(); + }); }); describe('Puppeteer.connect', function() { it('should be able to connect multiple times to the same browser', async({server}) => {