Skip to content

fix(glob): throw on page.route() for unbalanced braces in glob patterns#40442

Open
Thavamani13 wants to merge 2 commits intomicrosoft:mainfrom
Thavamani13:fix-40422-v2
Open

fix(glob): throw on page.route() for unbalanced braces in glob patterns#40442
Thavamani13 wants to merge 2 commits intomicrosoft:mainfrom
Thavamani13:fix-40422-v2

Conversation

@Thavamani13
Copy link
Copy Markdown

Summary

  • globToRegexPattern() now throws a descriptive error for nested {, unmatched }, and unclosed {
  • RouteHandler and WebSocketRouteHandler constructors call resolveGlobToRegexPattern() eagerly so the error surfaces immediately when page.route() / context.route() is called, not silently at request-match time
  • Added test covering all three invalid cases

Fixes #40422

Validates glob patterns eagerly in RouteHandler and WebSocketRouteHandler
constructors so an invalid pattern like '{foo' throws immediately when
page.route() / context.route() is called, rather than silently aborting
requests at match time via a SyntaxError from new RegExp().

Fixes: microsoft#40422
constructor(baseURL: string | undefined, url: URLMatch, handler: WebSocketRouteHandlerCallback) {
this._baseURL = baseURL;
this.url = url;
if (isString(url))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's move this to the dispatcher / server side, so it also applies to our language ports.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved it @Skn0tt Please check

Move glob pattern validation from the client-side WebSocketRouteHandler
constructor to setWebSocketInterceptionPatterns in both BrowserContext and
Page dispatchers, so the error also surfaces for language ports.
@Skn0tt Skn0tt requested a review from yury-s April 27, 2026 16:24
@github-actions
Copy link
Copy Markdown
Contributor

Test results for "tests 1"

9 flaky ⚠️ [chromium-library] › library/chromium/connect-over-cdp.spec.ts:449 › should be able to connect via localhost `@ubuntu-22.04-chromium-tip-of-tree`
⚠️ [chromium-library] › library/chromium/connect-over-cdp.spec.ts:449 › should be able to connect via localhost `@chromium-ubuntu-22.04-arm-node20`
⚠️ [chromium-library] › library/video.spec.ts:647 › screencast › should capture full viewport `@chromium-ubuntu-22.04-arm-node20`
⚠️ [chromium-library] › library/chromium/connect-over-cdp.spec.ts:449 › should be able to connect via localhost `@chromium-ubuntu-22.04-node24`
⚠️ [chromium-library] › library/chromium/connect-over-cdp.spec.ts:449 › should be able to connect via localhost `@chromium-ubuntu-22.04-node20`
⚠️ [chromium-library] › library/video.spec.ts:719 › screencast › should work with video+trace `@chromium-ubuntu-22.04-node20`
⚠️ [chromium-library] › library/chromium/connect-over-cdp.spec.ts:449 › should be able to connect via localhost `@chromium-ubuntu-22.04-node22`
⚠️ [firefox-library] › library/inspector/cli-codegen-1.spec.ts:1080 › cli codegen › should not throw csp directive violation errors `@firefox-ubuntu-22.04-node20`
⚠️ [firefox-page] › page/page-wait-for-function.spec.ts:104 › should work with strict CSP policy `@firefox-ubuntu-22.04-node20`

41467 passed, 847 skipped


Merge workflow run.

@github-actions
Copy link
Copy Markdown
Contributor

Test results for "MCP"

1 failed
❌ [chrome] › mcp/dashboard.spec.ts:231 › should switch screencast to -s session on show --annotate @mcp-windows-latest

6651 passed, 927 skipped


Merge workflow run.

this._baseURL = baseURL;
this._times = times;
this.url = url;
if (isString(url))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_updateInterceptionPatterns is called next line at both constructions sites and it does the glob validation on the server, do we still need to duplicate the logic here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

page.route() silently aborts navigation when glob contains unbalanced { or }

3 participants