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_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 @@
         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 @@
 
     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,60 @@
             }
         }
 
-        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 without a trailing
+            // anchor, then treat everything after the match as the remaining
+            // query string to parse for {?...}/{&...} expressions.
+            UriTemplate.validateLength(pattern, MAX_REGEX_LENGTH, 'Generated regex pattern');
+            const regex = new RegExp(pattern);
+            match = uri.match(regex);
+            if (!match) return null;
+            queryPart = uri.slice(match[0].length).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,95 @@ 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' });
+        });
     });
 
     describe('security and edge cases', () => {