Skip to content

[v1.x] fix: Update UriTemplate implementation to handle optional/omitted, out-of-order, and encoded query parameters #1083

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

Open
mgyarmathy wants to merge 7 commits into
base: v1.x
Choose a base branch
from
+164 −11
Open

[v1.x] fix: Update UriTemplate implementation to handle optional/omitted, out-of-order, and encoded query parameters #1083

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
3caa7e4
update UriTemplate implementation to handle optional or out-of-order …
mgyarmathy Nov 5, 2025
0924abb
omit absent query params from match result
mgyarmathy Mar 27, 2026
e2a569c
add safe decodeURIComponent helper; add changeset entry
mgyarmathy Mar 27, 2026
749fc4b
handle malformed percent-encoding in UriTemplate.match()
mgyarmathy Mar 27, 2026
98e31bb
fix(core): add boundary assertion to hasLiteralQuery regex and addres…
mgyarmathy Mar 27, 2026
1fbe148
refactor(core): reject literal-? in templates at construction; fix fr…
mgyarmathy Mar 31, 2026
0e89da3
fix: make ?/# pre-processing conditional on template operators; don't…
mgyarmathy Apr 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/fix-uri-template-query-params.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@modelcontextprotocol/core': patch
---

Fix `UriTemplate.match()` to correctly handle optional, out-of-order, and URL-encoded query parameters. Previously, query parameters had to appear in the exact order specified in the template and omitted parameters would cause match failures. Omitted query parameters are now
absent from the result (rather than set to `''`), so callers can use `vars.param ?? defaultValue`.
53 changes: 45 additions & 8 deletions src/shared/uriTemplate.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,14 @@
const MAX_TEMPLATE_EXPRESSIONS = 10000;
const MAX_REGEX_LENGTH = 1000000; // 1MB

function safeDecode(s: string): string {
try {
return decodeURIComponent(s);
} catch {
return s;
}
}

export class UriTemplate {
/**
* Returns true if the given string contains any URI template expressions.
Expand Down Expand Up @@ -247,12 +255,24 @@

match(uri: string): Variables | null {
UriTemplate.validateLength(uri, MAX_TEMPLATE_LENGTH, 'URI');

// 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);

Check failure on line 262 in src/shared/uriTemplate.ts

View check run for this annotation

Claude / Claude Code Review

Literal ? in template causes match regression with {&var} operator 

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.
Copy link
Copy Markdown

@claude claude Bot Mar 27, 2026

Choose a reason for hiding this comment

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

🔴 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

  1. Template: /search?static=true{&q}
  2. parse() produces: ["/search?static=true", {operator: "&", names: ["q"]}]
  3. match("/search?static=true&q=test") is called
  4. Line 260: queryIndex = uri.indexOf("?") → index 7 (the literal ? after "search")
  5. pathPart = "/search", queryPart = "static=true&q=test"
  6. Loop over parts: string "/search?static=true" → escapeRegExp → "/search\\?static\\=true" added to path regex
  7. Expression {&q} has operator & → pushed to queryParts, not included in regex
  8. Final regex: ^/search\?static\=true$
  9. pathPart.match(regex) → "/search".match(/^\/search\?static\=true$/) → null
  10. 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 ?.

Copy link
Copy Markdown
Contributor

@felixweinberger felixweinberger Mar 27, 2026

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Contributor Author

@mgyarmathy mgyarmathy Mar 27, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Contributor Author

@mgyarmathy mgyarmathy Mar 27, 2026

Choose a reason for hiding this comment

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

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


// Build regex pattern for path (non-query) parts
let pattern = '^';
const names: Array<{ name: string; exploded: boolean }> = [];
const queryParts: Array<{ name: string; exploded: boolean }> = [];

for (const part of this.parts) {
if (typeof part === 'string') {
pattern += this.escapeRegExp(part);
} else if (part.operator === '?' || part.operator === '&') {
for (const name of part.names) {
queryParts.push({ name, exploded: part.exploded });
}
} else {
const patterns = this.partToRegExp(part);
for (const { pattern: partPattern, name } of patterns) {
Expand All @@ -265,20 +285,37 @@
pattern += '$';
UriTemplate.validateLength(pattern, MAX_REGEX_LENGTH, 'Generated regex pattern');
const regex = new RegExp(pattern);
const match = uri.match(regex);
const match = pathPart.match(regex);

if (!match) return null;

const result: Variables = {};
for (let i = 0; i < names.length; i++) {
const { name, exploded } = names[i];
const value = match[i + 1];

for (const [i, { name, exploded }] of names.entries()) {
const value = match[i + 1]!;
const cleanName = name.replace('*', '');
result[cleanName] = exploded && value.includes(',') ? value.split(',') : value;
}

if (exploded && value.includes(',')) {
result[cleanName] = value.split(',');
} else {
result[cleanName] = value;
if (queryParts.length > 0) {
const queryParams = new Map<string, string>();
if (queryPart) {
for (const pair of queryPart.split('&')) {
const equalIndex = pair.indexOf('=');
if (equalIndex !== -1) {
const key = safeDecode(pair.slice(0, equalIndex));
const value = safeDecode(pair.slice(equalIndex + 1));
queryParams.set(key, value);
}
}
}

for (const { name, exploded } of queryParts) {
const cleanName = name.replace('*', '');
const value = queryParams.get(cleanName);

if (value === undefined) continue;
result[cleanName] = exploded && value.includes(',') ? value.split(',') : value;
}
}

Expand Down
53 changes: 53 additions & 0 deletions test/shared/uriTemplate.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,11 +191,64 @@ describe('UriTemplate', () => {
expect(template.variableNames).toEqual(['q', 'page']);
});

it('should handle partial query parameter matches correctly', () => {
const template = new UriTemplate('/search{?q,page}');
const match = template.match('/search?q=test');
expect(match).toEqual({ q: 'test' });
expect(template.variableNames).toEqual(['q', 'page']);
});

it('should match multiple query parameters if provided in a different order', () => {
const template = new UriTemplate('/search{?q,page}');
const match = template.match('/search?page=1&q=test');
expect(match).toEqual({ q: 'test', page: '1' });
expect(template.variableNames).toEqual(['q', 'page']);
});

it('should still match if additional query parameters are provided', () => {
const template = new UriTemplate('/search{?q,page}');
const match = template.match('/search?q=test&page=1&sort=desc');
expect(match).toEqual({ q: 'test', page: '1' });
expect(template.variableNames).toEqual(['q', 'page']);
});

it('should match omitted query parameters', () => {
const template = new UriTemplate('/search{?q,page}');
const match = template.match('/search');
expect(match).toEqual({});
expect(template.variableNames).toEqual(['q', 'page']);
});

it('should distinguish absent from empty query parameters', () => {
const template = new UriTemplate('/search{?q,page}');
const match = template.match('/search?q=');
expect(match).toEqual({ q: '' });
});

it('should match nested path segments with query parameters', () => {
const template = new UriTemplate('/api/{version}/{resource}{?apiKey,q,p,sort}');
const match = template.match('/api/v1/users?apiKey=testkey&q=user');
expect(match).toEqual({
version: 'v1',
resource: 'users',
apiKey: 'testkey',
q: 'user'
});
expect(template.variableNames).toEqual(['version', 'resource', 'apiKey', 'q', 'p', 'sort']);
});

it('should handle partial matches correctly', () => {
const template = new UriTemplate('/users/{id}');
expect(template.match('/users/123/extra')).toBeNull();
expect(template.match('/users')).toBeNull();
});

it('should handle encoded query parameters', () => {
const template = new UriTemplate('/search{?q}');
const match = template.match('/search?q=hello%20world');
expect(match).toEqual({ q: 'hello world' });
expect(template.variableNames).toEqual(['q']);
});
});

describe('security and edge cases', () => {
Expand Down
Loading