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

.changeset/fix-uri-template-query-params.md

@@ -0,0 +1,5 @@
+---
+'@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`.

packages/core/src/shared/uriTemplate.ts

@@ -7,6 +7,14 @@ const MAX_VARIABLE_LENGTH = 1_000_000; // 1MB
 const MAX_TEMPLATE_EXPRESSIONS = 10_000;
 const MAX_REGEX_LENGTH = 1_000_000; // 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.
@@ -97,7 +105,7 @@ export class UriTemplate {
         return expr
             .slice(operator.length)
             .split(',')
-            .map(name => name.replace('*', '').trim())
+            .map(name => name.replaceAll('*', '').trim())
             .filter(name => name.length > 0);
     }
 
@@ -254,12 +262,25 @@ export class UriTemplate {
 
     match(uri: string): Variables | null {
         UriTemplate.validateLength(uri, MAX_TEMPLATE_LENGTH, 'URI');
+
+        // Check whether any literal string segment in the template contains a
+        // '?'. If so, the template author has written a manual query-string
+        // prefix (e.g. `/path?fixed=1{&var}`) and we cannot simply split the
+        // URI at its first '?' — the path regex itself needs to consume past it.
+        const hasLiteralQuery = this.parts.some(part => typeof part === 'string' && part.includes('?'));
+
+        // 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) {
@@ -269,22 +290,69 @@ export class UriTemplate {
             }
         }
 
-        pattern += '$';
-        UriTemplate.validateLength(pattern, MAX_REGEX_LENGTH, 'Generated regex pattern');
-        const regex = new RegExp(pattern);
-        const match = uri.match(regex);
-
-        if (!match) return null;
+        let match: RegExpMatchArray | null;
+        let queryPart: string;
+
+        if (hasLiteralQuery) {
+            // Match the path regex against the full URI. The lookahead
+            // assertion ensures the literal portion ends exactly at a
+            // query-param separator, fragment, or end-of-string, so a
+            // template like `?id=1` does not spuriously prefix-match
+            // `?id=100`.
+            pattern += '(?=[&#]|$)';
+            UriTemplate.validateLength(pattern, MAX_REGEX_LENGTH, 'Generated regex pattern');
+            const regex = new RegExp(pattern);
+            match = uri.match(regex);
+            if (!match) return null;
+            // Everything after the match is the remaining query string to
+            // parse for {?...}/{&...} expressions. Strip any fragment and
+            // the leading `&` separator first.
+            let rest = uri.slice(match[0].length);
+            const hashIndex = rest.indexOf('#');
+            if (hashIndex !== -1) rest = rest.slice(0, hashIndex);
+            queryPart = rest.replace(/^&/, '');
+        } else {
+            // Split URI into path and query parts at the first '?'
+            const queryIndex = uri.indexOf('?');
+            const pathPart = queryIndex === -1 ? uri : uri.slice(0, queryIndex);
+            queryPart = queryIndex === -1 ? '' : uri.slice(queryIndex + 1);
+
+            pattern += '$';
+            UriTemplate.validateLength(pattern, MAX_REGEX_LENGTH, 'Generated regex pattern');
+            const regex = new RegExp(pattern);
+            match = pathPart.match(regex);
+            if (!match) return null;
+        }
 
         const result: Variables = {};
-        for (const [i, name_] of names.entries()) {
-            const { name, exploded } = name_!;
-            const value = match[i + 1]!;
-            const cleanName = name.replace('*', '');
 
+        for (const [i, { name, exploded }] of names.entries()) {
+            const value = match[i + 1]!;
+            const cleanName = name.replaceAll('*', '');
             result[cleanName] = exploded && value.includes(',') ? value.split(',') : 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.replaceAll('*', '');
+                const value = queryParams.get(cleanName);
+                if (value === undefined) continue;
+                result[cleanName] = exploded && value.includes(',') ? value.split(',') : value;
+            }
+        }
+
         return result;
     }
 }

packages/core/test/shared/uriTemplate.test.ts

@@ -191,11 +191,115 @@ 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']);
+        });
+
+        it('should not throw on malformed percent-encoding in query parameters', () => {
+            const template = new UriTemplate('/search{?q}');
+            expect(template.match('/search?q=100%')).toEqual({ q: '100%' });
+            expect(template.match('/search?q=%ZZ')).toEqual({ q: '%ZZ' });
+        });
+
+        it('should match templates with a literal ? followed by {&...} continuation', () => {
+            const template = new UriTemplate('/path?static=1{&dynamic}');
+            const match = template.match('/path?static=1&dynamic=hello');
+            expect(match).toEqual({ dynamic: 'hello' });
+            expect(template.variableNames).toEqual(['dynamic']);
+        });
+
+        it('should match templates with literal ? when continuation param is absent', () => {
+            const template = new UriTemplate('/path?static=1{&dynamic}');
+            const match = template.match('/path?static=1');
+            expect(match).toEqual({});
+        });
+
+        it('should match templates with literal ? and multiple continuation params', () => {
+            const template = new UriTemplate('/api?v=2{&key,page}');
+            const match = template.match('/api?v=2&page=3&key=abc');
+            expect(match).toEqual({ key: 'abc', page: '3' });
+        });
+
+        it('should match path variables combined with literal ? and {&...}', () => {
+            const template = new UriTemplate('/api/{version}?format=json{&key}');
+            const match = template.match('/api/v1?format=json&key=secret');
+            expect(match).toEqual({ version: 'v1', key: 'secret' });
+        });
+
+        it('should not prefix-match literal query values with literal ?', () => {
+            // `?id=1` must not match `?id=100`
+            const template = new UriTemplate('/path?id=1{&extra}');
+            expect(template.match('/path?id=100')).toBeNull();
+            expect(template.match('/path?id=1')).toEqual({});
+            expect(template.match('/path?id=1&extra=x')).toEqual({ extra: 'x' });
+        });
+
+        it('should require a proper separator after the literal ? portion', () => {
+            // malformed URI missing `&` between params must not match
+            const template = new UriTemplate('/path?a=1{&b}');
+            expect(template.match('/path?a=1b=2')).toBeNull();
+        });
+
+        it('should ignore fragments after the literal ? portion', () => {
+            const template = new UriTemplate('/path?a=1{&b}');
+            expect(template.match('/path?a=1#section')).toEqual({});
+            expect(template.match('/path?a=1&b=foo#section')).toEqual({ b: 'foo' });
+        });
     });
 
     describe('security and edge cases', () => {