Skip to content

perf: validate absolute URLs without throwing - #460

Merged
mcollina merged 2 commits into
delvedor:mainfrom
gurgunday:perf/bad-absolute-url-no-stack
Oct 8, 2026
Merged

mcollina merged 2 commits into
delvedor:mainfrom
gurgunday:perf/bad-absolute-url-no-stack

Conversation

@gurgunday

@gurgunday gurgunday commented Sep 28, 2026 •

Copy link
Copy Markdown
Collaborator

getPathFromAbsoluteUrl() validates absolute URLs with new URL(), which throws on malformed input. The error is always caught and discarded

use URL.parse instead

as we support node 20, we have to check the existence of the function

┌─────────────────────────────────────────────┬─────────────┬─────────────┬────────────────────┐
│                    case                     │  base (ns)  │  new (ns)   │       change       │
├─────────────────────────────────────────────┼─────────────┼─────────────┼────────────────────┤
│ malformed http://[invalid/hello/world       │ 2100–2203   │ 100–103     │ −95%, ~21× faster  │
├─────────────────────────────────────────────┼─────────────┼─────────────┼────────────────────┤
│ valid http://example.com/r10/42/x           │ 178.0–181.1 │ 180.1–187.6 │ +~1–3% (see below) │
├─────────────────────────────────────────────┼─────────────┼─────────────┼────────────────────┤
│ valid https://example.com:8080/r10/42/x?a=1 │ 259.5–268.5 │ 258.7–265.6 │ no change          │
├─────────────────────────────────────────────┼─────────────┼─────────────┼────────────────────┤
│ relative /r10/42/x (control)                │ 49.7–53.2   │ 50.3–50.8   │ no change          │
└─────────────────────────────────────────────┴─────────────┴─────────────┴────────────────────┘

@gurgunday

Copy link
Copy Markdown
Collaborator Author

Cc @mcollina

@gurgunday
gurgunday force-pushed the perf/bad-absolute-url-no-stack branch 2 times, most recently from 34f444c to 81bf387 Compare October 6, 2026 21:01
@gurgunday gurgunday changed the title perf: skip stack trace capture for malformed absolute URLs perf: validate absolute URLs without throwing Oct 6, 2026

@mcollina mcollina left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

can you rewrite the tests to not have inlined JS as string? Store those in a fixtures folder

getPathFromAbsoluteUrl() validated absolute URLs with new URL(), which
throws on malformed input. The error was always caught and discarded,
so allocating it and capturing its stack trace was wasted work on an
attacker-controlled path.

Use URL.parse(), which returns null instead of throwing. It is only
available since Node.js 20.18.0, so fall back to URL.canParse() followed
by new URL() on older versions.

Routing a malformed absolute URL goes from ~2150ns to ~100ns (-95%),
with no measurable change for valid absolute or relative requests.

Signed-off-by: Gürgün Dayıoğlu <hey@gurgun.day>
Signed-off-by: Gürgün Dayıoğlu <hey@gurgun.day>
@gurgunday
gurgunday force-pushed the perf/bad-absolute-url-no-stack branch from 81bf387 to f969947 Compare October 8, 2026 11:48
@gurgunday

gurgunday commented Oct 8, 2026 •

Copy link
Copy Markdown
Collaborator Author

can you rewrite the tests to not have inlined JS as string? Store those in a fixtures folder

@mcollina no idea how it slipped in 😄, fixed

we don't need inline, probably the agent had an issue and cheated

@mcollina mcollina left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

lgtm

@mcollina
mcollina merged commit b2cdb40 into delvedor:main Oct 8, 2026
14 checks passed
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.

2 participants