From 9ac1bbc2a5e8d62c52fbde474ddfda0a8995052b Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Wed, 19 Aug 2020 13:27:58 -0700 Subject: [PATCH] chore: remove more paths and url matches from the server side (#3528) --- src/browserContext.ts | 5 -- src/frames.ts | 7 +- src/helper.ts | 109 -------------------------- src/page.ts | 4 +- src/rpc/client/browserContext.ts | 7 +- src/rpc/client/clientHelper.ts | 101 +++++++++++++++++++++++- src/rpc/client/frame.ts | 5 +- src/rpc/client/page.ts | 11 +-- src/rpc/client/selectors.ts | 4 +- src/rpc/server/frameDispatcher.ts | 4 - src/rpc/server/selectorsDispatcher.ts | 2 +- src/selectors.ts | 4 +- src/types.ts | 5 -- test/interception.spec.ts | 28 +++---- 14 files changed, 135 insertions(+), 161 deletions(-) diff --git a/src/browserContext.ts b/src/browserContext.ts index f0f37be30d..314cb6db49 100644 --- a/src/browserContext.ts +++ b/src/browserContext.ts @@ -123,11 +123,6 @@ export abstract class BrowserContext extends EventEmitter { this._doExposeBinding(binding); } - async addInitScript(script: string | Function | { path?: string | undefined; content?: string | undefined; }, arg?: any): Promise { - const source = await helper.evaluationScript(script, arg); - await this._doAddInitScript(source); - } - async grantPermissions(permissions: string[], options?: { origin?: string }) { let origin = '*'; if (options && options.origin) { diff --git a/src/frames.ts b/src/frames.ts index 67196601bc..522a099ef0 100644 --- a/src/frames.ts +++ b/src/frames.ts @@ -469,18 +469,17 @@ export class Frame { }); } - async waitForNavigation(options: types.WaitForNavigationOptions = {}): Promise { + async waitForNavigation(options: types.NavigateOptions = {}): Promise { return runNavigationTask(this, options, async progress => { - const toUrl = typeof options.url === 'string' ? ` to "${options.url}"` : ''; const waitUntil = verifyLifecycle('waitUntil', options.waitUntil === undefined ? 'load' : options.waitUntil); - progress.log(`waiting for navigation${toUrl} until "${waitUntil}"`); + progress.log(`waiting for navigation until "${waitUntil}"`); const navigationEvent: NavigationEvent = await helper.waitForEvent(progress, this._eventEmitter, kNavigationEvent, (event: NavigationEvent) => { // Any failed navigation results in a rejection. if (event.error) return true; progress.log(` navigated to "${this._url}"`); - return helper.urlMatches(this._url, options.url); + return true; }).promise; if (navigationEvent.error) throw navigationEvent.error; diff --git a/src/helper.ts b/src/helper.ts index 4f7c4c019e..ea534bee34 100644 --- a/src/helper.ts +++ b/src/helper.ts @@ -41,40 +41,6 @@ export type Listener = (...args: any[]) => void; const isInDebugMode = !!getFromENV('PWDEBUG'); class Helper { - - static evaluationString(fun: Function | string, ...args: any[]): string { - if (Helper.isString(fun)) { - assert(args.length === 0 || (args.length === 1 && args[0] === undefined), 'Cannot evaluate a string with arguments'); - return fun; - } - return Helper.evaluationStringForFunctionBody(String(fun), ...args); - } - - static evaluationStringForFunctionBody(functionBody: string, ...args: any[]): string { - return `(${functionBody})(${args.map(serializeArgument).join(',')})`; - function serializeArgument(arg: any): string { - if (Object.is(arg, undefined)) - return 'undefined'; - return JSON.stringify(arg); - } - } - - static async evaluationScript(fun: Function | string | { path?: string, content?: string }, arg?: any, addSourceUrl: boolean = true): Promise { - if (!helper.isString(fun) && typeof fun !== 'function') { - if (fun.content !== undefined) { - fun = fun.content; - } else if (fun.path !== undefined) { - let contents = await util.promisify(fs.readFile)(fun.path, 'utf8'); - if (addSourceUrl) - contents += '//# sourceURL=' + fun.path.replace(/\n/g, ''); - fun = contents; - } else { - throw new Error('Either path or content property must be present'); - } - } - return helper.evaluationString(fun, arg); - } - static addEventListener( emitter: EventEmitter, eventName: (string | symbol), @@ -117,62 +83,6 @@ class Helper { return typeof obj === 'boolean' || obj instanceof Boolean; } - static globToRegex(glob: string): RegExp { - const tokens = ['^']; - let inGroup; - for (let i = 0; i < glob.length; ++i) { - const c = glob[i]; - if (escapeGlobChars.has(c)) { - tokens.push('\\' + c); - continue; - } - if (c === '*') { - const beforeDeep = glob[i - 1]; - let starCount = 1; - while (glob[i + 1] === '*') { - starCount++; - i++; - } - const afterDeep = glob[i + 1]; - const isDeep = starCount > 1 && - (beforeDeep === '/' || beforeDeep === undefined) && - (afterDeep === '/' || afterDeep === undefined); - if (isDeep) { - tokens.push('((?:[^/]*(?:\/|$))*)'); - i++; - } else { - tokens.push('([^/]*)'); - } - continue; - } - - switch (c) { - case '?': - tokens.push('.'); - break; - case '{': - inGroup = true; - tokens.push('('); - break; - case '}': - inGroup = false; - tokens.push(')'); - break; - case ',': - if (inGroup) { - tokens.push('|'); - break; - } - tokens.push('\\' + c); - break; - default: - tokens.push(c); - } - } - tokens.push('$'); - return new RegExp(tokens.join('')); - } - static completeUserURL(urlString: string): string { if (urlString.startsWith('localhost') || urlString.startsWith('127.0.0.1')) urlString = 'http://' + urlString; @@ -191,23 +101,6 @@ class Helper { return { width: Math.floor(size.width + 1e-3), height: Math.floor(size.height + 1e-3) }; } - static urlMatches(urlString: string, match: types.URLMatch | undefined): boolean { - if (match === undefined || match === '') - return true; - if (helper.isString(match)) - match = helper.globToRegex(match); - if (helper.isRegExp(match)) - return match.test(urlString); - if (typeof match === 'string' && match === urlString) - return true; - const url = new URL(urlString); - if (typeof match === 'string') - return url.pathname === match; - - assert(typeof match === 'function', 'url parameter should be string, RegExp or function'); - return match(url); - } - // See https://joel.tools/microtasks/ static makeWaitForNextTask() { if (parseInt(process.versions.node, 10) >= 11) @@ -366,8 +259,6 @@ export async function mkdirIfNeeded(filePath: string) { await mkdirAsync(path.dirname(filePath), {recursive: true}).catch(() => {}); } -const escapeGlobChars = new Set(['/', '$', '^', '+', '.', '(', ')', '=', '!', '|']); - export const helper = Helper; const debugLoggerColorMap = { diff --git a/src/page.ts b/src/page.ts index ac3ed42074..fdb8df9947 100644 --- a/src/page.ts +++ b/src/page.ts @@ -244,7 +244,7 @@ export class Page extends EventEmitter { async reload(options?: types.NavigateOptions): Promise { const waitPromise = this.mainFrame().waitForNavigation(options); await this._delegate.reload(); - const response = waitPromise; + const response = await waitPromise; await this._doSlowMo(); return response; } @@ -424,7 +424,7 @@ export class PageBinding { constructor(name: string, playwrightFunction: frames.FunctionWithSource) { this.name = name; this.playwrightFunction = playwrightFunction; - this.source = helper.evaluationString(addPageBinding, name); + this.source = `(${addPageBinding.toString()})(${JSON.stringify(name)})`; } static async dispatch(page: Page, payload: string, context: dom.FrameExecutionContext) { diff --git a/src/rpc/client/browserContext.ts b/src/rpc/client/browserContext.ts index 7f6448448c..5e20ae320a 100644 --- a/src/rpc/client/browserContext.ts +++ b/src/rpc/client/browserContext.ts @@ -20,8 +20,7 @@ import { Page, BindingCall } from './page'; import * as network from './network'; import { BrowserContextChannel, BrowserContextInitializer } from '../channels'; import { ChannelOwner } from './channelOwner'; -import { helper } from '../../helper'; -import { isUnderTest, deprecate } from './clientHelper'; +import { isUnderTest, deprecate, evaluationScript, urlMatches } from './clientHelper'; import { Browser } from './browser'; import { Events } from './events'; import { TimeoutSettings } from '../../timeoutSettings'; @@ -68,7 +67,7 @@ export class BrowserContext extends ChannelOwner { return this._wrapApiCall('browserContext.addInitScript', async () => { - const source = await helper.evaluationScript(script, arg); + const source = await evaluationScript(script, arg); await this._channel.addInitScript({ source }); }); } diff --git a/src/rpc/client/clientHelper.ts b/src/rpc/client/clientHelper.ts index b5b6692587..53881ada27 100644 --- a/src/rpc/client/clientHelper.ts +++ b/src/rpc/client/clientHelper.ts @@ -15,8 +15,10 @@ * limitations under the License. */ -import { isUnderTest as commonIsUnderTest } from '../../helper'; +import { isUnderTest as commonIsUnderTest, helper } from '../../helper'; import * as types from './types'; +import * as fs from 'fs'; +import * as util from 'util'; const deprecatedHits = new Set(); export function deprecate(methodName: string, message: string) { @@ -38,3 +40,100 @@ export function envObjectToArray(env: types.Env): { name: string, value: string } return result; } + +export async function evaluationScript(fun: Function | string | { path?: string, content?: string }, arg?: any, addSourceUrl: boolean = true): Promise { + if (typeof fun === 'function') { + const source = fun.toString(); + const argString = Object.is(arg, undefined) ? 'undefined' : JSON.stringify(arg); + return `(${source})(${argString})`; + } + if (arg !== undefined) + throw new Error('Cannot evaluate a string with arguments'); + if (helper.isString(fun)) + return fun; + if (fun.content !== undefined) + return fun.content; + if (fun.path !== undefined) { + let source = await util.promisify(fs.readFile)(fun.path, 'utf8'); + if (addSourceUrl) + source += '//# sourceURL=' + fun.path.replace(/\n/g, ''); + return source; + } + throw new Error('Either path or content property must be present'); +} + +export function urlMatches(urlString: string, match: types.URLMatch | undefined): boolean { + if (match === undefined || match === '') + return true; + if (helper.isString(match)) + match = globToRegex(match); + if (helper.isRegExp(match)) + return match.test(urlString); + if (typeof match === 'string' && match === urlString) + return true; + const url = new URL(urlString); + if (typeof match === 'string') + return url.pathname === match; + + if (typeof match !== 'function') + throw new Error('url parameter should be string, RegExp or function'); + return match(url); +} + +const escapeGlobChars = new Set(['/', '$', '^', '+', '.', '(', ')', '=', '!', '|']); + +export function globToRegex(glob: string): RegExp { + const tokens = ['^']; + let inGroup; + for (let i = 0; i < glob.length; ++i) { + const c = glob[i]; + if (escapeGlobChars.has(c)) { + tokens.push('\\' + c); + continue; + } + if (c === '*') { + const beforeDeep = glob[i - 1]; + let starCount = 1; + while (glob[i + 1] === '*') { + starCount++; + i++; + } + const afterDeep = glob[i + 1]; + const isDeep = starCount > 1 && + (beforeDeep === '/' || beforeDeep === undefined) && + (afterDeep === '/' || afterDeep === undefined); + if (isDeep) { + tokens.push('((?:[^/]*(?:\/|$))*)'); + i++; + } else { + tokens.push('([^/]*)'); + } + continue; + } + + switch (c) { + case '?': + tokens.push('.'); + break; + case '{': + inGroup = true; + tokens.push('('); + break; + case '}': + inGroup = false; + tokens.push(')'); + break; + case ',': + if (inGroup) { + tokens.push('|'); + break; + } + tokens.push('\\' + c); + break; + default: + tokens.push(c); + } + } + tokens.push('$'); + return new RegExp(tokens.join('')); +} diff --git a/src/rpc/client/frame.ts b/src/rpc/client/frame.ts index dcaf97dbf2..2abfa54390 100644 --- a/src/rpc/client/frame.ts +++ b/src/rpc/client/frame.ts @@ -15,7 +15,7 @@ * limitations under the License. */ -import { helper, assert } from '../../helper'; +import { assert } from '../../helper'; import { FrameChannel, FrameInitializer, FrameNavigatedEvent, FrameGotoOptions, FrameWaitForSelectorOptions, FrameDispatchEventOptions, FrameSetContentOptions, FrameClickOptions, FrameDblclickOptions, FrameFillOptions, FrameFocusOptions, FrameTextContentOptions, FrameInnerTextOptions, FrameInnerHTMLOptions, FrameGetAttributeOptions, FrameHoverOptions, FrameSetInputFilesOptions, FrameTypeOptions, FramePressOptions, FrameCheckOptions, FrameUncheckOptions } from '../channels'; import { BrowserContext } from './browserContext'; import { ChannelOwner } from './channelOwner'; @@ -29,6 +29,7 @@ import { EventEmitter } from 'events'; import { Waiter } from './waiter'; import { Events } from './events'; import { LifecycleEvent, URLMatch, SelectOption, SelectOptionOptions, FilePayload, WaitForFunctionOptions, kLifecycleEvents } from './types'; +import { urlMatches } from './clientHelper'; const fsReadFileAsync = util.promisify(fs.readFile.bind(fs)); @@ -123,7 +124,7 @@ export class Frame extends ChannelOwner { if (event.error) return true; waiter.log(` navigated to "${event.url}"`); - return helper.urlMatches(event.url, options.url); + return urlMatches(event.url, options.url); }); if (navigatedEvent.error) { const e = new Error(navigatedEvent.error); diff --git a/src/rpc/client/page.ts b/src/rpc/client/page.ts index 5b93bac37a..b9b94eb93c 100644 --- a/src/rpc/client/page.ts +++ b/src/rpc/client/page.ts @@ -41,6 +41,7 @@ import { Waiter } from './waiter'; import * as fs from 'fs'; import * as util from 'util'; import { Size, URLMatch, Headers, LifecycleEvent, WaitForEventOptions, SelectOption, SelectOptionOptions, FilePayload, WaitForFunctionOptions } from './types'; +import { evaluationScript, urlMatches } from './clientHelper'; type PDFOptions = Omit & { width?: string | number, @@ -150,7 +151,7 @@ export class Page extends ChannelOwner { private _onRoute(route: Route, request: Request) { for (const {url, handler} of this._routes) { - if (helper.urlMatches(request.url(), url)) { + if (urlMatches(request.url(), url)) { handler(route, request); return; } @@ -202,7 +203,7 @@ export class Page extends ChannelOwner { return this.frames().find(f => { if (name) return f.name() === name; - return helper.urlMatches(f.url(), url); + return urlMatches(f.url(), url); }) || null; } @@ -330,7 +331,7 @@ export class Page extends ChannelOwner { async waitForRequest(urlOrPredicate: string | RegExp | ((r: Request) => boolean), options: { timeout?: number } = {}): Promise { const predicate = (request: Request) => { if (helper.isString(urlOrPredicate) || helper.isRegExp(urlOrPredicate)) - return helper.urlMatches(request.url(), urlOrPredicate); + return urlMatches(request.url(), urlOrPredicate); return urlOrPredicate(request); }; return this.waitForEvent(Events.Page.Request, { predicate, timeout: options.timeout }); @@ -339,7 +340,7 @@ export class Page extends ChannelOwner { async waitForResponse(urlOrPredicate: string | RegExp | ((r: Response) => boolean), options: { timeout?: number } = {}): Promise { const predicate = (response: Response) => { if (helper.isString(urlOrPredicate) || helper.isRegExp(urlOrPredicate)) - return helper.urlMatches(response.url(), urlOrPredicate); + return urlMatches(response.url(), urlOrPredicate); return urlOrPredicate(response); }; return this.waitForEvent(Events.Page.Response, { predicate, timeout: options.timeout }); @@ -402,7 +403,7 @@ export class Page extends ChannelOwner { async addInitScript(script: Function | string | { path?: string, content?: string }, arg?: any) { return this._wrapApiCall('page.addInitScript', async () => { - const source = await helper.evaluationScript(script, arg); + const source = await evaluationScript(script, arg); await this._channel.addInitScript({ source }); }); } diff --git a/src/rpc/client/selectors.ts b/src/rpc/client/selectors.ts index 1fac829cbf..92a3605015 100644 --- a/src/rpc/client/selectors.ts +++ b/src/rpc/client/selectors.ts @@ -16,8 +16,8 @@ import { SelectorsChannel, SelectorsInitializer } from '../channels'; import { ChannelOwner } from './channelOwner'; -import { helper } from '../../helper'; import { ElementHandle } from './elementHandle'; +import { evaluationScript } from './clientHelper'; export class Selectors extends ChannelOwner { static from(selectors: SelectorsChannel): Selectors { @@ -29,7 +29,7 @@ export class Selectors extends ChannelOwner { - const source = await helper.evaluationScript(script, undefined, false); + const source = await evaluationScript(script, undefined, false); await this._channel.register({ ...options, name, source }); } diff --git a/src/rpc/server/frameDispatcher.ts b/src/rpc/server/frameDispatcher.ts index 9a30a809a0..19a6815e0e 100644 --- a/src/rpc/server/frameDispatcher.ts +++ b/src/rpc/server/frameDispatcher.ts @@ -56,10 +56,6 @@ export class FrameDispatcher extends Dispatcher impleme return { response: lookupNullableDispatcher(await this._frame.goto(params.url, params)) }; } - async waitForNavigation(params: types.WaitForNavigationOptions): Promise<{ response?: ResponseChannel }> { - return { response: lookupNullableDispatcher(await this._frame.waitForNavigation(params)) }; - } - async frameElement(): Promise<{ element: ElementHandleChannel }> { return { element: new ElementHandleDispatcher(this._scope, await this._frame.frameElement()) }; } diff --git a/src/rpc/server/selectorsDispatcher.ts b/src/rpc/server/selectorsDispatcher.ts index c9af93b53a..f15c2536cd 100644 --- a/src/rpc/server/selectorsDispatcher.ts +++ b/src/rpc/server/selectorsDispatcher.ts @@ -26,7 +26,7 @@ export class SelectorsDispatcher extends Dispatcher { - await this._object.register(params.name, params.source, params); + await this._object.register(params.name, params.source, params.contentScript); } async createSelector(params: { name: string, handle: ElementHandleDispatcher }): Promise<{ value?: string }> { diff --git a/src/selectors.ts b/src/selectors.ts index ba237d900f..df40295c0a 100644 --- a/src/selectors.ts +++ b/src/selectors.ts @@ -45,14 +45,12 @@ export class Selectors { this._engines = new Map(); } - async register(name: string, script: string | Function | { path?: string, content?: string }, options: { contentScript?: boolean } = {}): Promise { - const { contentScript = false } = options; + async register(name: string, source: string, contentScript: boolean = false): Promise { if (!name.match(/^[a-zA-Z_0-9-]+$/)) throw new Error('Selector engine name may only contain [a-zA-Z0-9_] characters'); // Note: we keep 'zs' for future use. if (this._builtinEngines.has(name) || name === 'zs' || name === 'zs:light') throw new Error(`"${name}" is a predefined selector engine`); - const source = await helper.evaluationScript(script, undefined, false); if (this._engines.has(name)) throw new Error(`"${name}" selector engine has been already registered`); this._engines.set(name, { source, contentScript }); diff --git a/src/types.ts b/src/types.ts index f592c694d7..8cfa88205e 100644 --- a/src/types.ts +++ b/src/types.ts @@ -44,11 +44,6 @@ export type PointerActionWaitOptions = TimeoutOptions & { force?: boolean, }; -export type WaitForNavigationOptions = TimeoutOptions & { - waitUntil?: LifecycleEvent, - url?: URLMatch -}; - export type ElementScreenshotOptions = TimeoutOptions & { type?: 'png' | 'jpeg', quality?: number, diff --git a/test/interception.spec.ts b/test/interception.spec.ts index ff0d6601b4..d766001557 100644 --- a/test/interception.spec.ts +++ b/test/interception.spec.ts @@ -16,7 +16,7 @@ */ import './base.fixture'; -import { helper } from '../lib/helper'; +import { globToRegex } from '../lib/rpc/client/clientHelper'; import vm from 'vm'; it('should work with navigation', async({page, server}) => { @@ -71,20 +71,20 @@ it('should intercept after a service worker', async({browser, page, server, cont }); it('should work with glob', async() => { - expect(helper.globToRegex('**/*.js').test('https://localhost:8080/foo.js')).toBeTruthy(); - expect(helper.globToRegex('**/*.css').test('https://localhost:8080/foo.js')).toBeFalsy(); - expect(helper.globToRegex('*.js').test('https://localhost:8080/foo.js')).toBeFalsy(); - expect(helper.globToRegex('https://**/*.js').test('https://localhost:8080/foo.js')).toBeTruthy(); - expect(helper.globToRegex('http://localhost:8080/simple/path.js').test('http://localhost:8080/simple/path.js')).toBeTruthy(); - expect(helper.globToRegex('http://localhost:8080/?imple/path.js').test('http://localhost:8080/Simple/path.js')).toBeTruthy(); - expect(helper.globToRegex('**/{a,b}.js').test('https://localhost:8080/a.js')).toBeTruthy(); - expect(helper.globToRegex('**/{a,b}.js').test('https://localhost:8080/b.js')).toBeTruthy(); - expect(helper.globToRegex('**/{a,b}.js').test('https://localhost:8080/c.js')).toBeFalsy(); + expect(globToRegex('**/*.js').test('https://localhost:8080/foo.js')).toBeTruthy(); + expect(globToRegex('**/*.css').test('https://localhost:8080/foo.js')).toBeFalsy(); + expect(globToRegex('*.js').test('https://localhost:8080/foo.js')).toBeFalsy(); + expect(globToRegex('https://**/*.js').test('https://localhost:8080/foo.js')).toBeTruthy(); + expect(globToRegex('http://localhost:8080/simple/path.js').test('http://localhost:8080/simple/path.js')).toBeTruthy(); + expect(globToRegex('http://localhost:8080/?imple/path.js').test('http://localhost:8080/Simple/path.js')).toBeTruthy(); + expect(globToRegex('**/{a,b}.js').test('https://localhost:8080/a.js')).toBeTruthy(); + expect(globToRegex('**/{a,b}.js').test('https://localhost:8080/b.js')).toBeTruthy(); + expect(globToRegex('**/{a,b}.js').test('https://localhost:8080/c.js')).toBeFalsy(); - expect(helper.globToRegex('**/*.{png,jpg,jpeg}').test('https://localhost:8080/c.jpg')).toBeTruthy(); - expect(helper.globToRegex('**/*.{png,jpg,jpeg}').test('https://localhost:8080/c.jpeg')).toBeTruthy(); - expect(helper.globToRegex('**/*.{png,jpg,jpeg}').test('https://localhost:8080/c.png')).toBeTruthy(); - expect(helper.globToRegex('**/*.{png,jpg,jpeg}').test('https://localhost:8080/c.css')).toBeFalsy(); + expect(globToRegex('**/*.{png,jpg,jpeg}').test('https://localhost:8080/c.jpg')).toBeTruthy(); + expect(globToRegex('**/*.{png,jpg,jpeg}').test('https://localhost:8080/c.jpeg')).toBeTruthy(); + expect(globToRegex('**/*.{png,jpg,jpeg}').test('https://localhost:8080/c.png')).toBeTruthy(); + expect(globToRegex('**/*.{png,jpg,jpeg}').test('https://localhost:8080/c.css')).toBeFalsy(); }); it('should work with regular expression passed from a different context', async({page, server}) => {