From 5993a6a0e00adb742c2ae55cfbefed4df94c0c28 Mon Sep 17 00:00:00 2001 From: Yury Semikhatsky Date: Wed, 29 Apr 2020 18:35:04 -0700 Subject: [PATCH] fix: throw from eval methods if >1 argument is passed (#2043) --- src/frames.ts | 6 +++++- src/helper.ts | 4 ++++ src/page.ts | 8 +++++++- test/evaluation.spec.js | 18 ++++++++++++++++++ 4 files changed, 34 insertions(+), 2 deletions(-) diff --git a/src/frames.ts b/src/frames.ts index 333303a16c9a3..b52a6e4c8d770 100644 --- a/src/frames.ts +++ b/src/frames.ts @@ -21,7 +21,7 @@ import { ConsoleMessage } from './console'; import * as dom from './dom'; import { TimeoutError, NotConnectedError } from './errors'; import { Events } from './events'; -import { assert, helper, RegisteredListener } from './helper'; +import { assert, helper, RegisteredListener, assertMaxArguments } from './helper'; import * as js from './javascript'; import * as network from './network'; import { Page } from './page'; @@ -415,6 +415,7 @@ export class Frame { async evaluateHandle(pageFunction: types.Func1, arg: Arg): Promise>; async evaluateHandle(pageFunction: types.Func1, arg?: any): Promise>; async evaluateHandle(pageFunction: types.Func1, arg: Arg): Promise> { + assertMaxArguments(arguments.length, 2); const context = await this._mainContext(); return context.evaluateHandleInternal(pageFunction, arg); } @@ -422,6 +423,7 @@ export class Frame { async evaluate(pageFunction: types.Func1, arg: Arg): Promise; async evaluate(pageFunction: types.Func1, arg?: any): Promise; async evaluate(pageFunction: types.Func1, arg: Arg): Promise { + assertMaxArguments(arguments.length, 2); const context = await this._mainContext(); return context.evaluateInternal(pageFunction, arg); } @@ -464,6 +466,7 @@ export class Frame { async $eval(selector: string, pageFunction: types.FuncOn, arg: Arg): Promise; async $eval(selector: string, pageFunction: types.FuncOn, arg?: any): Promise; async $eval(selector: string, pageFunction: types.FuncOn, arg: Arg): Promise { + assertMaxArguments(arguments.length, 3); const handle = await this.$(selector); if (!handle) throw new Error(`Error: failed to find element matching selector "${selector}"`); @@ -475,6 +478,7 @@ export class Frame { async $$eval(selector: string, pageFunction: types.FuncOn, arg: Arg): Promise; async $$eval(selector: string, pageFunction: types.FuncOn, arg?: any): Promise; async $$eval(selector: string, pageFunction: types.FuncOn, arg: Arg): Promise { + assertMaxArguments(arguments.length, 3); const arrayHandle = await selectors._queryArray(this, selector); const result = await arrayHandle.evaluate(pageFunction, arg); arrayHandle.dispose(); diff --git a/src/helper.ts b/src/helper.ts index 5a78434185c59..c2d864fa1b536 100644 --- a/src/helper.ts +++ b/src/helper.ts @@ -337,6 +337,10 @@ export function assert(value: any, message?: string): asserts value { throw new Error(message); } +export function assertMaxArguments(count: number, max: number): asserts count { + assert(count <= max, 'Too many arguments. If you need to pass more than 1 argument to the function wrap them in an object.'); +} + export function getFromENV(name: string) { let value = process.env[name]; value = value || process.env[`npm_config_${name.toLowerCase()}`]; diff --git a/src/page.ts b/src/page.ts index 869da5d791b59..21ec69c5b8a1e 100644 --- a/src/page.ts +++ b/src/page.ts @@ -17,7 +17,7 @@ import * as dom from './dom'; import * as frames from './frames'; -import { assert, helper, Listener } from './helper'; +import { assert, helper, Listener, assertMaxArguments } from './helper'; import * as input from './input'; import * as js from './javascript'; import * as network from './network'; @@ -228,18 +228,21 @@ export class Page extends ExtendedEventEmitter implements InnerLogger { async evaluateHandle(pageFunction: types.Func1, arg: Arg): Promise>; async evaluateHandle(pageFunction: types.Func1, arg?: any): Promise>; async evaluateHandle(pageFunction: types.Func1, arg: Arg): Promise> { + assertMaxArguments(arguments.length, 2); return this.mainFrame().evaluateHandle(pageFunction, arg); } async $eval(selector: string, pageFunction: types.FuncOn, arg: Arg): Promise; async $eval(selector: string, pageFunction: types.FuncOn, arg?: any): Promise; async $eval(selector: string, pageFunction: types.FuncOn, arg: Arg): Promise { + assertMaxArguments(arguments.length, 3); return this.mainFrame().$eval(selector, pageFunction, arg); } async $$eval(selector: string, pageFunction: types.FuncOn, arg: Arg): Promise; async $$eval(selector: string, pageFunction: types.FuncOn, arg?: any): Promise; async $$eval(selector: string, pageFunction: types.FuncOn, arg: Arg): Promise { + assertMaxArguments(arguments.length, 3); return this.mainFrame().$$eval(selector, pageFunction, arg); } @@ -373,6 +376,7 @@ export class Page extends ExtendedEventEmitter implements InnerLogger { async evaluate(pageFunction: types.Func1, arg: Arg): Promise; async evaluate(pageFunction: types.Func1, arg?: any): Promise; async evaluate(pageFunction: types.Func1, arg: Arg): Promise { + assertMaxArguments(arguments.length, 2); return this.mainFrame().evaluate(pageFunction, arg); } @@ -568,12 +572,14 @@ export class Worker extends EventEmitter { async evaluate(pageFunction: types.Func1, arg: Arg): Promise; async evaluate(pageFunction: types.Func1, arg?: any): Promise; async evaluate(pageFunction: types.Func1, arg: Arg): Promise { + assertMaxArguments(arguments.length, 2); return (await this._executionContextPromise).evaluateInternal(pageFunction, arg); } async evaluateHandle(pageFunction: types.Func1, arg: Arg): Promise>; async evaluateHandle(pageFunction: types.Func1, arg?: any): Promise>; async evaluateHandle(pageFunction: types.Func1, arg: Arg): Promise> { + assertMaxArguments(arguments.length, 2); return (await this._executionContextPromise).evaluateHandleInternal(pageFunction, arg); } } diff --git a/test/evaluation.spec.js b/test/evaluation.spec.js index 4582391cf582b..379c5d83630d8 100644 --- a/test/evaluation.spec.js +++ b/test/evaluation.spec.js @@ -162,6 +162,24 @@ describe('Page.evaluate', function() { const result = await page.evaluate(() => -Infinity); expect(Object.is(result, -Infinity)).toBe(true); }); + it('should throw when passed more than one parameter', async({page, server}) => { + const expectThrow = async f => { + let error; + await f().catch(e => error = e); + expect('' + error).toContain('Too many arguments'); + } + await expectThrow(() => page.evaluate((a, b) => false, 1, 2)); + await expectThrow(() => page.evaluateHandle((a, b) => false, 1, 2)); + await expectThrow(() => page.$eval('sel', (a, b) => false, 1, 2)); + await expectThrow(() => page.$$eval('sel', (a, b) => false, 1, 2)); + await expectThrow(() => page.evaluate((a, b) => false, 1, 2)); + const frame = page.mainFrame(); + await expectThrow(() => frame.evaluate((a, b) => false, 1, 2)); + await expectThrow(() => frame.evaluateHandle((a, b) => false, 1, 2)); + await expectThrow(() => frame.$eval('sel', (a, b) => false, 1, 2)); + await expectThrow(() => frame.$$eval('sel', (a, b) => false, 1, 2)); + await expectThrow(() => frame.evaluate((a, b) => false, 1, 2)); + }); it('should accept "undefined" as one of multiple parameters', async({page, server}) => { const result = await page.evaluate(({ a, b }) => Object.is(a, undefined) && Object.is(b, 'foo'), { a: undefined, b: 'foo' }); expect(result).toBe(true);