Skip to content

fix(core): avoid quadratic backtracking in path parameter substitution - #888

Open
Dhirenderchoudhary wants to merge 2 commits into
corsairdev:mainfrom
Dhirenderchoudhary:fix/redos-path-substitution
Open

fix(core): avoid quadratic backtracking in path parameter substitution#888
Dhirenderchoudhary wants to merge 2 commits into
corsairdev:mainfrom
Dhirenderchoudhary:fix/redos-path-substitution

Conversation

@Dhirenderchoudhary

@Dhirenderchoudhary Dhirenderchoudhary commented Aug 21, 2026

Copy link
Copy Markdown
Collaborator

Description

getUrl substitutes path parameters with a lazy quantifier:

.replace(/{(.*?)}/g, …)

.*? backtracks from every { in the string, so the scan is quadratic in the
number of unmatched opening braces. CodeQL flags it as a polynomial regular
expression on uncontrolled data.

This replaces the lazy quantifier with an explicit negated class:

.replace(/\{([^{}]*)\}/g, …)

[^{}]* cannot cross a brace, so there is nothing to backtrack over and the
scan is linear. It matches exactly the same well-formed {placeholder} tokens.

Impact

Every plugin request goes through getUrl, and options.url is built from
endpoint paths that interpolate caller-supplied values. A value carrying many
unmatched { characters stalls the event loop before the request is even sent.

Tests

packages/corsair/tests/request-path-substitution.test.ts — 6 tests driving the
real request() with a stubbed fetch, asserting the resolved URL.

Five cover substitution behaviour so the change is provably semantics-preserving:
single placeholder, multiple placeholders, unmatched placeholder left untouched,
value encoding, and {api-version}.

The sixth is the regression guard: it builds the hostile input above and asserts
substitution completes in under 1s. With the lazy quantifier restored it fails
at 22,664 ms; the other five still pass, which is what confirms the fix changes
performance and not behaviour.

pnpm --filter corsair test -- tests/request-path-substitution.test.ts

Checklist

Before submitting your PR, please verify the following:

  • I have run pnpm lint and all checks pass
  • I have run pnpm typecheck and there are no TypeScript errors
  • I have run pnpm build and all packages build successfully
  • I have run pnpm test and all tests pass
  • I have added or updated tests where applicable
  • I have added or updated necessary documentation

Screenshots / Demos (if applicable)

Additional Notes

Summary by CodeRabbit

  • Bug Fixes

    • Improved URL path-parameter replacement for malformed inputs with many unmatched braces, preventing slowdowns.
    • Preserved correct substitution and encoding of valid path parameters, including API version placeholders.
  • Tests

    • Added coverage for single and multiple parameters, empty and nested placeholders, unmatched braces, encoded values, API version handling, and large malformed inputs.

@vercel

vercel Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

@Dhirenderchoudhary is attempting to deploy a commit to the corsair Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai

coderabbitai Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: e8767480-281d-477f-8529-5bfd8755ff46

📥 Commits

Reviewing files that changed from the base of the PR and between 33c5b3a and 5cc4173.

📒 Files selected for processing (1)
  • packages/corsair/tests/request-path-substitution.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 6 remain after this review.


📝 Walkthrough

Walkthrough

The URL builder now matches brace-delimited placeholders without crossing braces. Tests cover substitution behavior, encoding, API-version replacement, malformed placeholders, and large unclosed-brace input.

Changes

URL substitution

Layer / File(s) Summary
Brace-safe placeholder matching
packages/corsair/async-core/request.ts
getUrl uses /\{([^{}]*)\}/g to replace placeholders without repeated backtracking on malformed brace-heavy input.
Substitution behavior and performance validation
packages/corsair/tests/request-path-substitution.test.ts
Tests mock fetch, capture generated URLs, verify placeholder substitution and encoding, and check processing time for 200,000 unclosed braces.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 5cc41

The PR changes path-parameter matching to avoid pathological backtracking while preserving substitution behavior, with focused regression coverage. No actionable merge-blocking risk remains beyond normal checks and review.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: preventing quadratic backtracking in core path parameter substitution.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added the core Changes in packages/corsair label Aug 21, 2026
@Dhirenderchoudhary
Dhirenderchoudhary marked this pull request as ready for review August 21, 2026 15:27
@greptile-apps

greptile-apps Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR replaces the path-parameter substitution regex with a brace-bounded matcher to avoid quadratic backtracking while retaining normal placeholder behavior.

  • Updates the shared async-core request URL substitution logic.
  • Adds request-level coverage for substitution, malformed braces, encoding, API versions, nested braces, and adversarial input performance.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
packages/corsair/async-core/request.ts Replaces the backtracking-prone placeholder matcher with a linear brace-bounded expression.
packages/corsair/tests/request-path-substitution.test.ts Adds comprehensive request-level regression coverage for URL path substitution and malformed input performance.

Reviews (2): Last reviewed commit: "test(core): document substitution edge c..." | Re-trigger Greptile

@Dhirenderchoudhary

Copy link
Copy Markdown
Collaborator Author

@greptile

@Dhirenderchoudhary Dhirenderchoudhary self-assigned this Aug 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core Changes in packages/corsair

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant