[v1.x] fix: Update UriTemplate implementation to handle optional/omitted, out-of-order, and encoded query parameters by mgyarmathy · Pull Request #1083 · modelcontextprotocol/typescript-sdk

mgyarmathy · 2025-11-05T20:34:58Z

Note: This is the v1.x backport of #1396.

This PR addresses several limitations of the current UriTemplate#match implementation:

template matching fails when optional query parameters are not provided
template matching fails when query parameters are provided out of order from the sequence specified in the template
template matching does not decode uri-encoded query parameters

Omitted query parameters are parsed as an empty string per guidance of RFC 6570 Section 2.3.

Motivation and Context

How Has This Been Tested?

New test additions cover all of the cases mentioned in #1079 and #149 as well as a few additional potential edge cases I could come up with.

Breaking Changes

n/a

Types of changes

Bug fix (non-breaking change which fixes an issue)

Checklist

I have read the MCP Documentation
My code follows the repository's style guidelines
New and existing tests pass locally
I have added appropriate error handling

Additional context

pkg-pr-new · 2025-11-12T17:49:05Z

Open in StackBlitz

npm i https://pkg.pr.new/@modelcontextprotocol/sdk@1083

commit: 1fbe148

cedricziel · 2026-01-12T15:45:58Z

Team - any chance of revisiting this PR? - Not being able to use optional query parameters is kinda limiting.

mgyarmathy · 2026-01-16T18:50:06Z

I've updated the base branch for this so that the bugfix is backported to v1.x. The fix for v2.x is #1396

travisbreaks

Clean implementation. Separating path matching from query parameter extraction is the right approach for RFC 6570 query templates. The code correctly handles all the tricky cases: optional params, out-of-order params, URL-encoded values, and extra query params.

Test coverage is solid: 6 new test cases covering partial matches, reordered params, extra params, omitted params, nested paths with query params, and encoded values.

A few observations:

1. Missing query params default to empty string

if (value === undefined) {
    result[cleanName] = '';
}

An omitted optional query param returning '' (empty string) rather than undefined could be surprising to consumers. With undefined, callers can distinguish "param was omitted" from "param was explicitly set to empty." Consider returning undefined for truly absent params. The tests currently assert '', so this would be a design decision for the maintainers.

2. Extra query params silently ignored
The test "should still match if additional query parameters are provided" confirms that sort=desc is silently dropped. This is correct per RFC 6570 (the template only declares q and page), but worth a brief comment in the code for future readers.

3. Duplicate query param keys
If a URI has ?q=foo&q=bar, the Map will keep only the last value (bar). RFC 6570 does not define behavior for duplicate keys in query matching, so this is reasonable, but worth noting.

4. v2 port (PR #1396)
This PR targets v1.x. PR #1396 ports the same fix to v2 but has merge conflicts. If this lands on v1.x, the v2 port should be straightforward to rebase.

This has been open since October with no review. It addresses a real bug with a clean fix and good tests. Would be great to see it merged.

felixweinberger

The v2 version of this fix is up at #1785 with one change: absent query params are omitted from the result rather than set to '', so expand(match(uri)) round-trips correctly. Could you take a look and apply the same change here? Feedback on the approach welcome.

…query parameters

Absent query parameters are now omitted from the result object rather than set to empty string, so callers can use `vars.param ?? default`. This also distinguishes 'param absent' from 'param present but empty' (e.g. ?q= returns {q: ''}). Also removes dead code: the isQuery field was always false since query parts go to a separate array, and the (?:\\?.*)?$ regex suffix was unreachable since pathPart already excludes the query string.

changeset-bot · 2026-03-27T15:03:06Z

🦋 Changeset detected

Latest commit: 1fbe148

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

mgyarmathy · 2026-03-27T15:09:20Z

@felixweinberger I've applied the same changes as #1785 -- everything included there looks more clean and safe.

felixweinberger · 2026-03-27T17:18:36Z

@claude review

claude · 2026-03-27T17:24:14Z

src/shared/uriTemplate.ts

+        // Split URI into path and query parts
+        const queryIndex = uri.indexOf('?');
+        const pathPart = queryIndex === -1 ? uri : uri.slice(0, queryIndex);
+        const queryPart = queryIndex === -1 ? '' : uri.slice(queryIndex + 1);


🔴 The new match() unconditionally splits the URI at the first ? (line 260) to separate path from query, which breaks templates using the {&var} (form-continuation) operator. For example, /search?static=true{&q} fails to match /search?static=true&q=test because pathPart becomes /search while the path regex expects ^/search\?static\=true$. This is a regression from previously working behavior where the regex was applied to the full URI.

Extended reasoning...

What the bug is

The match() method introduced a URI-splitting strategy at line 260 that unconditionally splits the input URI at the first ? character. The path portion (before ?) is matched against a regex built from non-query template parts, while query parameters (after ?) are parsed separately with a Map. This breaks any template that contains a literal ? character in its string parts, which is exactly the use case for the RFC 6570 {&var} (form-continuation) operator.

How it manifests

The {&var} operator exists specifically for templates where a literal ? already appears in the template string, and additional query parameters should be appended with &. For template /search?static=true{&q}, the parser produces two parts: the string /search?static=true and the expression {&q}. The string part (including its literal ?) gets escaped into the path regex as ^/search\?static\=true$. However, the URI /search?static=true&q=test is split at the first ?, yielding pathPart="/search" and queryPart="static=true&q=test". The regex ^/search\?static\=true$ is then matched against just "/search", which obviously fails, and the method returns null.

Step-by-step proof

Template: /search?static=true{&q}

parse() produces: ["/search?static=true", {operator: "&", names: ["q"]}]

match("/search?static=true&q=test") is called

Line 260: queryIndex = uri.indexOf("?") → index 7 (the literal ? after "search")

pathPart = "/search", queryPart = "static=true&q=test"

Loop over parts: string "/search?static=true" → escapeRegExp → "/search\\?static\\=true" added to path regex

Expression {&q} has operator & → pushed to queryParts, not included in regex

Final regex: ^/search\?static\=true$

pathPart.match(regex) → "/search".match(/^\/search\?static\=true$/) → null

Method returns null — match fails

The even simpler case X{&var} matching X&var=test also fails: the URI has no ? so pathPart="X&var=test" but the regex is ^X$.

Impact

This completely breaks {&var} template matching, which is part of the RFC 6570 specification. The {&var} operator is the canonical way to express "append additional query parameters to a URI that already has some." While {?var} (which introduces the ?) is more common in MCP, the {&var} operator is a valid and supported pattern that previously worked correctly.

How to fix

The split logic needs to account for literal ? characters in template string parts. One approach: count the number of literal ? characters in string parts, and split the URI at the Nth+1 ? (where N is the count of literal ? chars in string parts). Alternatively, detect whether the template contains any {&var} expressions and, if so, include the literal query string in the path regex rather than splitting at the first ?.

I hit the same issue on #1785 and fixed it in 01bcc47 by checking whether the template itself has a literal ? before deciding where to split.

Makes sense. I copied over your commit in 749fc4b and added an extra spec you introduced.

I had to use .replace(/\*/g, '') instead of replaceAll('*', '') since v1.x targets es2020

felixweinberger

Requesting another turn on the automated review finding

Per review feedback on #1785: since #1083 backports this fix to v1.x, users migrating v1.x-latest to v2 won't see a behavior change here.

…s edge cases backporting from modelcontextprotocol@7e25ee3

…agment handling - Remove the hasLiteralQuery special-case branch from match(). Four rounds of review found bugs in it; the two-code-path design was inherently fragile. - Reject any literal '?' in template string segments at construction time with a clear error pointing at the {?param} alternative. No real-world MCP resource templates use this pattern. - Fix fragment handling: strip #fragment before splitting path/query, so /path?a=1#frag correctly extracts {a:'1'} instead of {a:'1#frag'}. - Replace 7 hasLiteralQuery tests with construction-validation tests and fragment-stripping tests. Net: -33 lines of match() logic, one code path, correct by construction. ported from modelcontextprotocol@8a9aefb

mgyarmathy · 2026-03-31T19:02:27Z

@felixweinberger I'm continuing to backport your commits from #1785. Let me know if I can assist you in any other way.

mgyarmathy requested a review from a team as a code owner November 5, 2025 20:34

mgyarmathy force-pushed the 1079-uritemplate-query-params branch 2 times, most recently from 5e2623c to 5950c61 Compare November 5, 2025 20:42

mgyarmathy changed the title ~~Update UriTemplate implementation to handle optional/omitted or out-of-order query parameters~~ Update UriTemplate implementation to handle optional/omitted, out-of-order, and encoded query parameters Nov 5, 2025

mgyarmathy changed the title ~~Update UriTemplate implementation to handle optional/omitted, out-of-order, and encoded query parameters~~ fix: Update UriTemplate implementation to handle optional/omitted, out-of-order, and encoded query parameters Nov 5, 2025

mgyarmathy force-pushed the 1079-uritemplate-query-params branch from 5950c61 to ee8c7f6 Compare November 12, 2025 17:46

mgyarmathy force-pushed the 1079-uritemplate-query-params branch 2 times, most recently from c7b3331 to 6e5ff20 Compare November 17, 2025 19:14

mgyarmathy force-pushed the 1079-uritemplate-query-params branch from 6e5ff20 to 6ec58ad Compare December 17, 2025 20:51

mgyarmathy marked this pull request as draft December 17, 2025 20:52

mgyarmathy marked this pull request as ready for review December 17, 2025 20:52

This was referenced Dec 17, 2025

# MCP SDK ResourceTemplate URI Validation Issue: RFC 6570 Template Matching Behavior #1079

Open

Unordered and partial query parameter matching #149

Open

cedricziel mentioned this pull request Jan 12, 2026

fix: ResourceTemplate URI matching with SDK PR#1083 for query parameters cedricziel/aha-mcp#197

Merged

mgyarmathy changed the base branch from main to v1.x January 16, 2026 18:38

mgyarmathy mentioned this pull request Jan 16, 2026

[v2] fix: Update UriTemplate implementation to handle optional/omitted, out-of-order, and encoded query parameters #1396

Closed

5 tasks

mgyarmathy changed the title ~~fix: Update UriTemplate implementation to handle optional/omitted, out-of-order, and encoded query parameters~~ [v1.x] fix: Update UriTemplate implementation to handle optional/omitted, out-of-order, and encoded query parameters Jan 16, 2026

travisbreaks reviewed Mar 19, 2026

View reviewed changes

felixweinberger mentioned this pull request Mar 23, 2026

Add pathVariableNames and queryVariableNames getters in UriTemplate #609

Closed

9 tasks

felixweinberger requested changes Mar 27, 2026

View reviewed changes

mgyarmathy added 2 commits March 27, 2026 09:56

update UriTemplate implementation to handle optional or out-of-order …

3caa7e4

…query parameters

mgyarmathy force-pushed the 1079-uritemplate-query-params branch from 6ec58ad to 0924abb Compare March 27, 2026 15:03

add safe decodeURIComponent helper; add changeset entry

e2a569c

mgyarmathy mentioned this pull request Mar 27, 2026

fix: UriTemplate.match() handles optional, out-of-order, and encoded query parameters #1785

Open

6 tasks

claude bot reviewed Mar 27, 2026

View reviewed changes

felixweinberger requested changes Mar 27, 2026

View reviewed changes

felixweinberger added a commit that referenced this pull request Mar 27, 2026

docs: drop migration.md entry since fix is backported to v1.x

79d782e

Per review feedback on #1785: since #1083 backports this fix to v1.x, users migrating v1.x-latest to v2 won't see a behavior change here.

handle malformed percent-encoding in UriTemplate.match()

749fc4b

mgyarmathy force-pushed the 1079-uritemplate-query-params branch from ad9c488 to 749fc4b Compare March 27, 2026 18:38

fix(core): add boundary assertion to hasLiteralQuery regex and addres…

98e31bb

…s edge cases backporting from modelcontextprotocol@7e25ee3

mgyarmathy force-pushed the 1079-uritemplate-query-params branch from b66e6c6 to 98e31bb Compare March 27, 2026 20:58

Conversation

mgyarmathy commented Nov 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation and Context

How Has This Been Tested?

Breaking Changes

Types of changes

Checklist

Additional context

Uh oh!

pkg-pr-new bot commented Nov 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cedricziel commented Jan 12, 2026

Uh oh!

mgyarmathy commented Jan 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

travisbreaks left a comment

Choose a reason for hiding this comment

Uh oh!

felixweinberger left a comment

Choose a reason for hiding this comment

Uh oh!

changeset-bot bot commented Mar 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

mgyarmathy commented Mar 27, 2026

Uh oh!

felixweinberger commented Mar 27, 2026

Uh oh!

claude bot Mar 27, 2026

Choose a reason for hiding this comment

What the bug is

How it manifests

Step-by-step proof

Impact

How to fix

Uh oh!

felixweinberger Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

mgyarmathy Mar 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mgyarmathy Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

felixweinberger left a comment

Choose a reason for hiding this comment

Uh oh!

mgyarmathy commented Mar 31, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mgyarmathy commented Nov 5, 2025 •

edited

Loading

pkg-pr-new bot commented Nov 12, 2025 •

edited

Loading

mgyarmathy commented Jan 16, 2026 •

edited

Loading

changeset-bot bot commented Mar 27, 2026 •

edited

Loading

mgyarmathy Mar 27, 2026 •

edited

Loading