From fdde9493eada0099baeb62556634ee244fe79dcb Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Mon, 25 Jan 2021 14:49:51 -0800 Subject: [PATCH] fix: don't parse potentially invalid urls in event handlers (#5090) --- src/cli/traceViewer/snapshotRouter.ts | 11 +++++------ src/cli/traceViewer/traceViewer.ts | 2 +- src/client/clientHelper.ts | 13 +++++++++++-- src/server/network.ts | 16 +++++++++++----- src/server/page.ts | 4 +++- src/trace/harTracer.ts | 4 +++- src/trace/snapshotter.ts | 15 +++------------ test/page-goto.spec.ts | 5 +++++ 8 files changed, 42 insertions(+), 28 deletions(-) diff --git a/src/cli/traceViewer/snapshotRouter.ts b/src/cli/traceViewer/snapshotRouter.ts index 6cf781826aeb0..83bc2ad60f2a7 100644 --- a/src/cli/traceViewer/snapshotRouter.ts +++ b/src/cli/traceViewer/snapshotRouter.ts @@ -18,6 +18,7 @@ import * as fs from 'fs'; import * as path from 'path'; import * as util from 'util'; import type { Route } from '../../..'; +import { parsedURL } from '../../client/clientHelper'; import type { FrameSnapshot, NetworkResourceTraceEvent, PageSnapshot } from '../../trace/traceTypes'; import { ContextEntry } from './traceModel'; @@ -112,11 +113,9 @@ export class SnapshotRouter { } function removeHash(url: string) { - try { - const u = new URL(url); - u.hash = ''; - return u.toString(); - } catch (e) { + const u = parsedURL(url); + if (!u) return url; - } + u.hash = ''; + return u.toString(); } diff --git a/src/cli/traceViewer/traceViewer.ts b/src/cli/traceViewer/traceViewer.ts index 3594d9d9f270f..3ae602614b025 100644 --- a/src/cli/traceViewer/traceViewer.ts +++ b/src/cli/traceViewer/traceViewer.ts @@ -132,8 +132,8 @@ class TraceViewer { this._document.snapshotRouter.route(route); return; } - const url = new URL(request.url()); try { + const url = new URL(request.url()); if (this._document && request.url().includes('action-preview')) { const fullPath = url.pathname.substring('/action-preview/'.length); const actionId = fullPath.substring(0, fullPath.indexOf('.png')); diff --git a/src/client/clientHelper.ts b/src/client/clientHelper.ts index c0a44e5afe11d..f4e464eef436d 100644 --- a/src/client/clientHelper.ts +++ b/src/client/clientHelper.ts @@ -58,6 +58,14 @@ export async function evaluationScript(fun: Function | string | { path?: string, throw new Error('Either path or content property must be present'); } +export function parsedURL(url: string): URL | null { + try { + return new URL(url); + } catch (e) { + return null; + } +} + export function urlMatches(urlString: string, match: types.URLMatch | undefined): boolean { if (match === undefined || match === '') return true; @@ -67,10 +75,11 @@ export function urlMatches(urlString: string, match: types.URLMatch | undefined) return match.test(urlString); if (typeof match === 'string' && match === urlString) return true; - const url = new URL(urlString); + const url = parsedURL(urlString); + if (!url) + return false; 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); diff --git a/src/server/network.ts b/src/server/network.ts index e452fae4d3a59..d1ea76c40c77c 100644 --- a/src/server/network.ts +++ b/src/server/network.ts @@ -61,12 +61,18 @@ export function rewriteCookies(cookies: types.SetNetworkCookieParam[]): types.Se }); } -function stripFragmentFromUrl(url: string): string { - if (!url.indexOf('#')) +export function parsedURL(url: string): URL | null { + try { + return new URL(url); + } catch (e) { + return null; + } +} + +export function stripFragmentFromUrl(url: string): string { + if (!url.includes('#')) return url; - const parsed = new URL(url); - parsed.hash = ''; - return parsed.href; + return url.substring(0, url.indexOf('#')); } export class Request { diff --git a/src/server/page.ts b/src/server/page.ts index 5296e35d62360..400c4500a13ce 100644 --- a/src/server/page.ts +++ b/src/server/page.ts @@ -481,7 +481,9 @@ export class Page extends EventEmitter { const url = frame.url(); if (!url.startsWith('http')) return; - this._browserContext.addVisitedOrigin(new URL(url).origin); + const purl = network.parsedURL(url); + if (purl) + this._browserContext.addVisitedOrigin(purl.origin); } allBindings() { diff --git a/src/trace/harTracer.ts b/src/trace/harTracer.ts index 38dccbbdf6643..39d64aa2f0cd2 100644 --- a/src/trace/harTracer.ts +++ b/src/trace/harTracer.ts @@ -134,7 +134,9 @@ class HarContextTracer { private _onRequest(page: Page, request: network.Request) { const pageEntry = this._pageEntries.get(page)!; - const url = new URL(request.url()); + const url = network.parsedURL(request.url()); + if (!url) + return; const harEntry: har.Entry = { pageref: pageEntry.id, diff --git a/src/trace/snapshotter.ts b/src/trace/snapshotter.ts index ed86ab86e0d48..f758d23b6432c 100644 --- a/src/trace/snapshotter.ts +++ b/src/trace/snapshotter.ts @@ -18,6 +18,7 @@ import { BrowserContext } from '../server/browserContext'; import { Page } from '../server/page'; import * as network from '../server/network'; import { helper, RegisteredListener } from '../server/helper'; +import { stripFragmentFromUrl } from '../server/network'; import { Progress, runAbortableTask } from '../server/progress'; import { debugLogger } from '../utils/debugLogger'; import { Frame } from '../server/frames'; @@ -115,7 +116,7 @@ export class Snapshotter { return frameResult; const frameSnapshot = { frameId: frame._id, - url: removeHash(frame.url()), + url: stripFragmentFromUrl(frame.url()), html: 'Snapshot is not available', resourceOverrides: [], }; @@ -190,7 +191,7 @@ export class Snapshotter { const snapshot: FrameSnapshot = { frameId: frame._id, - url: removeHash(frame.url()), + url: stripFragmentFromUrl(frame.url()), html: data.html, resourceOverrides: [], }; @@ -216,16 +217,6 @@ export class Snapshotter { } } -function removeHash(url: string) { - try { - const u = new URL(url); - u.hash = ''; - return u.toString(); - } catch (e) { - return url; - } -} - type FrameSnapshotAndMapping = { snapshot: FrameSnapshot, mapping: Map, diff --git a/test/page-goto.spec.ts b/test/page-goto.spec.ts index bd1edeadfb81f..385b55b64d28b 100644 --- a/test/page-goto.spec.ts +++ b/test/page-goto.spec.ts @@ -494,3 +494,8 @@ it('should report raw buffer for main resource', (test, { browserName, platform const body = await response.body(); expect(body.toString()).toBe('Ü (lowercase ü)'); }); + +it('should not throw unhandled rejections on invalid url', async ({page, server}) => { + const e = await page.goto('https://www.youtube Panel Title.com/').catch(e => e); + expect(e.toString()).toContain('Panel Title'); +});