feat(rpc): client-side parameters validation (#3069)

Author: dgozman

src/page.ts

@@ -387,9 +387,9 @@ export class Page extends EventEmitter {
 
   async emulateMedia(options: { media?: types.MediaType | null, colorScheme?: types.ColorScheme | null }) {
     if (options.media !== undefined)
-      assert(options.media === null || types.mediaTypes.has(options.media), 'media: expected one of (screen|print)');
+      assert(options.media === null || types.mediaTypes.has(options.media), 'media: expected one of (screen|print|null)');
     if (options.colorScheme !== undefined)
-      assert(options.colorScheme === null || types.colorSchemes.has(options.colorScheme), 'colorScheme: expected one of (dark|light|no-preference)');
+      assert(options.colorScheme === null || types.colorSchemes.has(options.colorScheme), 'colorScheme: expected one of (dark|light|no-preference|null)');
     if (options.media !== undefined)
       this._state.mediaType = options.media;
     if (options.colorScheme !== undefined)

src/rpc/channels.ts

@@ -620,8 +620,8 @@ export type PageCloseParams = {
 };
 export type PageCloseResult = void;
 export type PageEmulateMediaParams = {
-  media?: 'screen' | 'print' | 'reset',
-  colorScheme?: 'dark' | 'light' | 'no-preference' | 'reset',
+  media?: 'screen' | 'print' | 'null',
+  colorScheme?: 'dark' | 'light' | 'no-preference' | 'null',
 };
 export type PageEmulateMediaResult = void;
 export type PageExposeBindingParams = {

src/rpc/client/browserType.ts

@@ -23,6 +23,7 @@ import { BrowserServer } from './browserServer';
 import { LoggerSink } from '../../loggerSink';
 import { headersObjectToArray, envObjectToArray } from '../serializers';
 import { serializeArgument } from './jsHandle';
+import { assert } from '../../helper';
 
 type FirefoxPrefsOptions = { firefoxUserPrefs?: { [key: string]: string | number | boolean } };
 
@@ -48,6 +49,8 @@ export class BrowserType extends ChannelOwner<BrowserTypeChannel, BrowserTypeIni
     const logger = options.logger;
     options = { ...options, logger: undefined };
     return this._wrapApiCall('browserType.launch', async () => {
+      assert(!(options as any).userDataDir, 'userDataDir option is not supported in `browserType.launch`. Use `browserType.launchPersistentContext` instead');
+      assert(!(options as any).port, 'Cannot specify a port without launching as a server.');
       const launchOptions: BrowserTypeLaunchParams = {
         ...options,
         ignoreDefaultArgs: Array.isArray(options.ignoreDefaultArgs) ? options.ignoreDefaultArgs : undefined,

src/rpc/client/channelOwner.ts

@@ -66,7 +66,7 @@ export abstract class ChannelOwner<T extends Channel = Channel, Initializer = {}
           return obj.addListener;
         if (prop === 'removeEventListener')
           return obj.removeListener;
-        return (params: any) => this._connection.sendMessageToServer({ guid, method: String(prop), params });
+        return (params: any) => this._connection.sendMessageToServer(this._type, guid, String(prop), params);
       },
     });
     (this._channel as any)._object = this;

src/rpc/client/connection.ts

@@ -38,6 +38,7 @@ import { ChromiumBrowser } from './chromiumBrowser';
 import { ChromiumBrowserContext } from './chromiumBrowserContext';
 import { Selectors } from './selectors';
 import { Stream } from './stream';
+import { validateParams } from './validator';
 
 class Root extends ChannelOwner<Channel, {}> {
   constructor(connection: Connection) {
@@ -63,9 +64,10 @@ export class Connection {
     return new Promise(f => this._waitingForObject.set(guid, f));
   }
 
-  async sendMessageToServer(message: { guid: string, method: string, params: any }): Promise<any> {
+  async sendMessageToServer(type: string, guid: string, method: string, params: any): Promise<any> {
     const id = ++this._lastId;
-    const converted = { id, ...message, params: this._replaceChannelsWithGuids(message.params) };
+    const validated = method === 'debugScopeState' ? params : validateParams(type, method, params);
+    const converted = { id, guid, method, params: validated };
     debug('pw:channel:command')(converted);
     this.onmessage(converted);
     return new Promise((resolve, reject) => this._callbacks.set(id, { resolve, reject }));
@@ -97,22 +99,6 @@ export class Connection {
     object._channel.emit(method, this._replaceGuidsWithChannels(params));
   }
 
-  private _replaceChannelsWithGuids(payload: any): any {
-    if (!payload)
-      return payload;
-    if (Array.isArray(payload))
-      return payload.map(p => this._replaceChannelsWithGuids(p));
-    if (payload._object instanceof ChannelOwner)
-      return { guid: payload._object._guid };
-    if (typeof payload === 'object') {
-      const result: any = {};
-      for (const key of Object.keys(payload))
-        result[key] = this._replaceChannelsWithGuids(payload[key]);
-      return result;
-    }
-    return payload;
-  }
-
   private _replaceGuidsWithChannels(payload: any): any {
     if (!payload)
       return payload;

src/rpc/client/elementHandle.ts

@@ -123,7 +123,7 @@ export class ElementHandle<T extends Node = Node> extends JSHandle<T> {
     });
   }
 
-  async selectText(options: types.TimeoutOptions): Promise<void> {
+  async selectText(options: types.TimeoutOptions = {}): Promise<void> {
     return this._wrapApiCall('elementHandle.selectText', async () => {
       await this._elementChannel.selectText(options);
     });

src/rpc/client/frame.ts

@@ -15,7 +15,7 @@
  * limitations under the License.
  */
 
-import { assertMaxArguments, helper } from '../../helper';
+import { assertMaxArguments, helper, assert } from '../../helper';
 import * as types from '../../types';
 import { FrameChannel, FrameInitializer, FrameNavigatedEvent } from '../channels';
 import { BrowserContext } from './browserContext';
@@ -89,7 +89,8 @@ export class Frame extends ChannelOwner<FrameChannel, FrameInitializer> {
 
   async goto(url: string, options: GotoOptions = {}): Promise<network.Response | null> {
     return this._wrapApiCall(this._apiName('goto'), async () => {
-      return network.Response.fromNullable((await this._channel.goto({ url, ...options })).response);
+      const waitUntil = verifyLoadState('waitUntil', options.waitUntil === undefined ? 'load' : options.waitUntil);
+      return network.Response.fromNullable((await this._channel.goto({ url, ...options, waitUntil })).response);
     });
   }
 
@@ -188,6 +189,10 @@ export class Frame extends ChannelOwner<FrameChannel, FrameInitializer> {
 
   async waitForSelector(selector: string, options: types.WaitForElementOptions = {}): Promise<ElementHandle<Element> | null> {
     return this._wrapApiCall(this._apiName('waitForSelector'), async () => {
+      if ((options as any).visibility)
+        throw new Error('options.visibility is not supported, did you mean options.state?');
+      if ((options as any).waitFor && (options as any).waitFor !== 'visible')
+        throw new Error('options.waitFor is not supported, did you mean options.state?');
       const result = await this._channel.waitForSelector({ selector, ...options });
       return ElementHandle.fromNullable(result.element) as ElementHandle<Element> | null;
     });
@@ -234,7 +239,8 @@ export class Frame extends ChannelOwner<FrameChannel, FrameInitializer> {
 
   async setContent(html: string, options: types.NavigateOptions = {}): Promise<void> {
     return this._wrapApiCall(this._apiName('setContent'), async () => {
-      await this._channel.setContent({ html, ...options });
+      const waitUntil = verifyLoadState('waitUntil', options.waitUntil === undefined ? 'load' : options.waitUntil);
+      await this._channel.setContent({ html, ...options, waitUntil });
     });
   }
 
@@ -272,9 +278,11 @@ export class Frame extends ChannelOwner<FrameChannel, FrameInitializer> {
   async addStyleTag(options: { url?: string; path?: string; content?: string; }): Promise<ElementHandle> {
     return this._wrapApiCall(this._apiName('addStyleTag'), async () => {
       const copy = { ...options };
-      if (copy.path)
+      if (copy.path) {
         copy.content = (await fsReadFileAsync(copy.path)).toString();
-      return ElementHandle.from((await this._channel.addStyleTag({ ...options })).element);
+        copy.content += '/*# sourceURL=' + copy.path.replace(/\n/g, '') + '*/';
+      }
+      return ElementHandle.from((await this._channel.addStyleTag({ ...copy })).element);
     });
   }
 
@@ -378,6 +386,8 @@ export class Frame extends ChannelOwner<FrameChannel, FrameInitializer> {
   async waitForFunction<R>(pageFunction: Func1<void, R>, arg?: any, options?: types.WaitForFunctionOptions): Promise<SmartHandle<R>>;
   async waitForFunction<R, Arg>(pageFunction: Func1<Arg, R>, arg: Arg, options: types.WaitForFunctionOptions = {}): Promise<SmartHandle<R>> {
     return this._wrapApiCall(this._apiName('waitForFunction'), async () => {
+      if (typeof options.polling === 'string')
+        assert(options.polling === 'raf', 'Unknown polling option: ' + options.polling);
       const result = await this._channel.waitForFunction({
         ...options,
         pollingInterval: options.polling === 'raf' ? undefined : options.polling,
@@ -396,7 +406,7 @@ export class Frame extends ChannelOwner<FrameChannel, FrameInitializer> {
   }
 }
 
-function verifyLoadState(name: string, waitUntil: types.LifecycleEvent): types.LifecycleEvent {
+export function verifyLoadState(name: string, waitUntil: types.LifecycleEvent): types.LifecycleEvent {
   if (waitUntil as unknown === 'networkidle0')
     waitUntil = 'networkidle';
   if (!types.kLifecycleEvents.has(waitUntil))

src/rpc/client/page.ts

@@ -29,7 +29,7 @@ import { Dialog } from './dialog';
 import { Download } from './download';
 import { ElementHandle } from './elementHandle';
 import { Worker } from './worker';
-import { Frame, FunctionWithSource, GotoOptions } from './frame';
+import { Frame, FunctionWithSource, GotoOptions, verifyLoadState } from './frame';
 import { Keyboard, Mouse } from './input';
 import { Func1, FuncOn, SmartHandle, serializeArgument, parseResult } from './jsHandle';
 import { Request, Response, Route, RouteHandler } from './network';
@@ -300,7 +300,8 @@ export class Page extends ChannelOwner<PageChannel, PageInitializer> {
 
   async reload(options: types.NavigateOptions = {}): Promise<Response | null> {
     return this._wrapApiCall('page.reload', async () => {
-      return Response.fromNullable((await this._channel.reload(options)).response);
+      const waitUntil = verifyLoadState('waitUntil', options.waitUntil === undefined ? 'load' : options.waitUntil);
+      return Response.fromNullable((await this._channel.reload({ ...options, waitUntil })).response);
     });
   }
 
@@ -346,21 +347,23 @@ export class Page extends ChannelOwner<PageChannel, PageInitializer> {
 
   async goBack(options: types.NavigateOptions = {}): Promise<Response | null> {
     return this._wrapApiCall('page.goBack', async () => {
-      return Response.fromNullable((await this._channel.goBack(options)).response);
+      const waitUntil = verifyLoadState('waitUntil', options.waitUntil === undefined ? 'load' : options.waitUntil);
+      return Response.fromNullable((await this._channel.goBack({ ...options, waitUntil })).response);
     });
   }
 
   async goForward(options: types.NavigateOptions = {}): Promise<Response | null> {
     return this._wrapApiCall('page.goForward', async () => {
-      return Response.fromNullable((await this._channel.goForward(options)).response);
+      const waitUntil = verifyLoadState('waitUntil', options.waitUntil === undefined ? 'load' : options.waitUntil);
+      return Response.fromNullable((await this._channel.goForward({ ...options, waitUntil })).response);
     });
   }
 
   async emulateMedia(options: { media?: types.MediaType | null, colorScheme?: types.ColorScheme | null }) {
     return this._wrapApiCall('page.emulateMedia', async () => {
       await this._channel.emulateMedia({
-        media: options.media === null ? 'reset' : options.media,
-        colorScheme: options.colorScheme === null ? 'reset' : options.colorScheme,
+        media: options.media === null ? 'null' : options.media,
+        colorScheme: options.colorScheme === null ? 'null' : options.colorScheme,
       });
     });
   }