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,19 @@
 
     match(uri: string): Variables | null {
         UriTemplate.validateLength(uri, MAX_TEMPLATE_LENGTH, 'URI');
+
+        // 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 +284,63 @@
             }
         }
 
+        // Only strip the URI fragment when the template has no {#var} operator;
+        // otherwise the fragment is part of what the path regex must capture.
+        const hasHashOperator = this.parts.some(p => typeof p !== 'string' && p.operator === '#');
+        let working = uri;
+        if (!hasHashOperator) {
+            const hashIndex = working.indexOf('#');
+            if (hashIndex !== -1) working = working.slice(0, hashIndex);
+        }
+
+        // Only split path/query when the template actually has {?..}/{&..}
+        // operators. Otherwise match the path regex against the full URI so
+        // {+var} can capture across '?' as it did before query-param support.
+        let pathPart = working;
+        let queryPart = '';
+        if (queryParts.length > 0) {
+            const queryIndex = working.indexOf('?');
+            if (queryIndex !== -1) {
+                pathPart = working.slice(0, queryIndex);
+                queryPart = working.slice(queryIndex + 1);
+            }
+        }
+
         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 (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,105 @@ 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 not throw on literal ? in a string segment (expand-only usage)', () => {
+            expect(() => new UriTemplate('/path?fixed=1')).not.toThrow();
+            expect(() => new UriTemplate('http://e.com/?literal').expand({})).not.toThrow();
+            expect(new UriTemplate('http://e.com/?literal').expand({})).toBe('http://e.com/?literal');
+        });
+
+        it('should let {+var} capture across ? when template has no query operators', () => {
+            const template = new UriTemplate('http://e.com{+rest}');
+            expect(template.match('http://e.com/search?q=hello')).toEqual({ rest: '/search?q=hello' });
+        });
+
+        it('should let {#var} capture the fragment', () => {
+            const template = new UriTemplate('/page{#section}');
+            expect(template.match('/page#intro')).toEqual({ section: '#intro' });
+        });
+
+        it('should accept templates using {?param} for query parameters', () => {
+            // The supported way to express query parameters
+            const template = new UriTemplate('/path{?fixed}');
+            expect(template.match('/path?fixed=1')).toEqual({ fixed: '1' });
+            expect(template.match('/path')).toEqual({});
+        });
+
+        it('should strip fragments before matching query parameters', () => {
+            const template = new UriTemplate('/path{?a}');
+            expect(template.match('/path?a=1#frag')).toEqual({ a: '1' });
+            expect(template.match('/path?a=1&b=2#frag')).toEqual({ a: '1' });
+        });
+
+        it('should strip fragments when the URI has no query string', () => {
+            const template = new UriTemplate('/path{?a}');
+            expect(template.match('/path#frag')).toEqual({});
+            expect(template.match('/path')).toEqual({});
+        });
     });
 
     describe('security and edge cases', () => {