Skip to content

feat: RFC 6570 URI templates with operator-aware security #2356

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
maxisbey wants to merge 47 commits into
base: main
Choose a base branch
from

fix: reject {expr}{+var} adjacency to close ReDoS gap

19822fb
Select commit
Loading
Failed to load commit list.
Draft

feat: RFC 6570 URI templates with operator-aware security #2356

fix: reject {expr}{+var} adjacency to close ReDoS gap
19822fb
Select commit
Loading
Failed to load commit list.
Claude / Claude Code Review completed Mar 27, 2026 in 23m 31s

Code review found 1 important issue

Found 4 candidates, confirmed 3. See review comments for details.

Details

Severity Count
🔴 Important 1
🟡 Nit 2
🟣 Pre-existing 0
Severity File:Line Issue
🔴 Important src/mcp/shared/uri_template.py:840-869 ReDoS: adjacent non-reserved expressions cause O(n^2) backtracking
🟡 Nit docs/migration.md:551 Migration doc says Four behaviors but lists five
🟡 Nit docs/server/resources.md:380-382 Third doc example missing ServerRequestContext and Server imports

Annotations

Check failure on line 869 in src/mcp/shared/uri_template.py

See this annotation in the file changed.

@claude claude / Claude Code Review

ReDoS: adjacent non-reserved expressions cause O(n^2) backtracking 

ReDoS gap: `_check_ambiguous_adjacency` rejects reserved-adjacent and explode-adjacent patterns but misses two adjacent non-reserved expressions like `{a}{b}suffix`. The resulting regex `([^/?#&,]*)([^/?#&,]*)suffix` has identical character classes in both groups, causing O(n²) backtracking (~16s at `max_uri_length=65536`) when the suffix fails to match. Fix: reject any two path expressions immediately adjacent without a disambiguating literal separator.

Check warning on line 551 in docs/migration.md

See this annotation in the file changed.

@claude claude / Claude Code Review

Migration doc says Four behaviors but lists five 

Line 551 says "Four behaviors have changed:" but the section lists five bolded items (Path-safety checks, Template literals regex-escaped, Query parameters match leniently, Malformed templates fail at decoration time, Static URIs with Context-only handlers now error). Should say "Five" instead of "Four".

Check warning on line 382 in docs/server/resources.md

See this annotation in the file changed.

@claude claude / Claude Code Review

Third doc example missing ServerRequestContext and Server imports 

The third low-level server code example ("Listing templates", lines 379-401) uses `ServerRequestContext[Any]` on line 386 and `Server(...)` on line 396, but the import block (lines 380-382) only imports `Any` and types from `mcp.types`. Missing `from mcp.server.context import ServerRequestContext` and `from mcp.server.lowlevel import Server`. Users copying this example verbatim will get `NameError`. The first two low-level examples correctly import both.
View more details on Claude