Skip to content

url: skip plain prefixes in percent-escape checks - #66696

Open
rozzilla wants to merge 1 commit into
nodejs:mainfrom
rozzilla:perf/url-percent-scan
Open

rozzilla wants to merge 1 commit into
nodejs:mainfrom
rozzilla:perf/url-percent-scan

Conversation

@rozzilla

Copy link
Copy Markdown
Contributor

decodeFormComponent() already locates the first %, but hasPercentHex() scans from the beginning again. Pass that position to the helper so it skips the plain-text prefix. Replacing + with a space preserves the position; escape validation and decoding behavior are unchanged.

Constructor throughput on Apple M2/macOS, with rebuilt Node v27.0.0-pre binaries
against upstream a7a96c2448ab91ca079de5e64233aa79a400b00e:

Query value Throughput improvement
32 ASCII characters followed by %20 +17.2%
1,024 ASCII characters followed by %20 +139.4%
1,024 ASCII characters followed by %E2%82%AC +135.6%

No corrected-significant regressions were detected.

decodeFormComponent() already finds the first percent character, but
hasPercentHex() scans the plain prefix again. Start the check at the
known position to avoid repeating that work.

Replacing plus signs with spaces preserves character positions, so
escape validation and decoding behavior remain unchanged.

Signed-off-by: Roberto Bianchi <roberto.bianchi@spendesk.com>
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/performance
  • @nodejs/url

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. whatwg-url Issues and PRs related to the WHATWG URL implementation. labels Oct 13, 2026
@codecov

codecov Bot commented Oct 13, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.47%. Comparing base (a7a96c2) to head (b3259a3).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #66696      +/-   ##
==========================================
- Coverage   90.47%   90.47%   -0.01%     
==========================================
  Files         791      791              
  Lines      277088   277088              
  Branches    53286    53281       -5     
==========================================
- Hits       250703   250696       -7     
+ Misses      16805    16803       -2     
- Partials     9580     9589       +9     
Files with missing lines Coverage Δ
lib/internal/url.js 93.31% <100.00%> (ø)

... and 33 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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

Labels

needs-ci PRs that need a full CI run. whatwg-url Issues and PRs related to the WHATWG URL implementation.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants