diff --git a/.changeset/uri-template-match-decodes-extracted-values.md b/.changeset/uri-template-match-decodes-extracted-values.md new file mode 100644 index 0000000000..b81a3135e7 --- /dev/null +++ b/.changeset/uri-template-match-decodes-extracted-values.md @@ -0,0 +1,10 @@ +--- +'@modelcontextprotocol/core-internal': patch +--- + +`UriTemplate.match()` now percent-decodes every value it extracts from a URI, undoing the encoding that `expand()` applied for the same operator. `expand()` percent-encodes spaces, slashes, non-ASCII characters, and friends; `match()` was returning the still-encoded substring, so a handler routed through `ResourceTemplate` received the wrong string for any resource whose template variable contains a reserved or non-ASCII character. + +- `decodeURIComponent` is used for the default, `.`, `/`, `?`, and `&` operators. +- `decodeURI` is used for `+` and `#`, matching the lighter `encodeURI` that `expand()` applies for those reserved-character operators. +- Each element of an exploded array is decoded independently. +- A malformed escape sequence (e.g. `%ZZ`) is returned unchanged rather than thrown, so a value that was already raw on the wire does not raise out of `match()`. diff --git a/packages/core-internal/src/shared/uriTemplate.ts b/packages/core-internal/src/shared/uriTemplate.ts index 5ffe213acd..fadc8670c8 100644 --- a/packages/core-internal/src/shared/uriTemplate.ts +++ b/packages/core-internal/src/shared/uriTemplate.ts @@ -202,8 +202,8 @@ export class UriTemplate { operator: string; names: string[]; exploded: boolean; - }): Array<{ pattern: string; name: string }> { - const patterns: Array<{ pattern: string; name: string }> = []; + }): Array<{ pattern: string; name: string; operator: string }> { + const patterns: Array<{ pattern: string; name: string; operator: string }> = []; // Validate variable name length for matching for (const name of part.names) { @@ -216,7 +216,8 @@ export class UriTemplate { const prefix = i === 0 ? '\\' + part.operator : '&'; patterns.push({ pattern: prefix + this.escapeRegExp(name) + '=([^&]+)', - name + name, + operator: part.operator }); } return patterns; @@ -248,23 +249,44 @@ export class UriTemplate { } } - patterns.push({ pattern, name }); + patterns.push({ pattern, name, operator: part.operator }); return patterns; } + /** + * Decodes a value that {@link match} extracted from a URI, undoing the + * encoding that {@link expand} applies for the same operator. `+` and `#` + * use {@link encodeURI}, which preserves a wider set of reserved + * characters, so the matching inverse is {@link decodeURI}; every other + * operator uses {@link encodeURIComponent}, whose inverse is + * {@link decodeURIComponent}. + * + * Malformed escape sequences are returned unchanged rather than thrown, + * so a value that was already raw on the wire (or that was constructed + * from a string the caller already decoded) does not raise out of + * {@link match}. + */ + private decodeExtractedValue(value: string, operator: string): string { + try { + return operator === '+' || operator === '#' ? decodeURI(value) : decodeURIComponent(value); + } catch { + return value; + } + } + match(uri: string): Variables | null { UriTemplate.validateLength(uri, MAX_TEMPLATE_LENGTH, 'URI'); let pattern = '^'; - const names: Array<{ name: string; exploded: boolean }> = []; + const names: Array<{ name: string; exploded: boolean; operator: string }> = []; for (const part of this.parts) { if (typeof part === 'string') { pattern += this.escapeRegExp(part); } else { const patterns = this.partToRegExp(part); - for (const { pattern: partPattern, name } of patterns) { + for (const { pattern: partPattern, name, operator } of patterns) { pattern += partPattern; - names.push({ name, exploded: part.exploded }); + names.push({ name, exploded: part.exploded, operator }); } } } @@ -278,11 +300,14 @@ export class UriTemplate { const result: Variables = {}; for (const [i, name_] of names.entries()) { - const { name, exploded } = name_!; + const { name, exploded, operator } = name_!; const value = match[i + 1]!; const cleanName = name.replace('*', ''); - result[cleanName] = exploded && value.includes(',') ? value.split(',') : value; + const raw = exploded && value.includes(',') ? value.split(',') : value; + result[cleanName] = Array.isArray(raw) + ? raw.map(v => this.decodeExtractedValue(v, operator)) + : this.decodeExtractedValue(raw, operator); } return result; diff --git a/packages/core-internal/test/shared/uriTemplate.test.ts b/packages/core-internal/test/shared/uriTemplate.test.ts index bfc3237872..edb8856837 100644 --- a/packages/core-internal/test/shared/uriTemplate.test.ts +++ b/packages/core-internal/test/shared/uriTemplate.test.ts @@ -198,6 +198,75 @@ describe('UriTemplate', () => { }); }); + describe('percent-decoding on match', () => { + // See https://github.com/modelcontextprotocol/typescript-sdk/issues/2728 + // match() previously returned the still-encoded substring, so the + // round trip (match(expand(v)) === v) failed for any value containing + // a reserved or non-ASCII character. + + it('round-trips a single value through the default operator', () => { + for (const value of ['My File.txt', 'a b', 'a/b', 'a?b', 'a&b', 'a=b', '100%', 'ΓΌ', 'a#b']) { + const template = new UriTemplate('file:///{path}'); + const expanded = template.expand({ path: value }); + expect(template.match(expanded)).toEqual({ path: value }); + } + }); + + it('round-trips multiple variables through the default operator', () => { + const template = new UriTemplate('x://h/{a}/{b}'); + const expanded = template.expand({ a: 'a#b', b: 'a#b' }); + expect(expanded).toBe('x://h/a%23b/a%23b'); + expect(template.match(expanded)).toEqual({ a: 'a#b', b: 'a#b' }); + }); + + it('round-trips values through the + reserved operator using decodeURI', () => { + // + uses encodeURI on expand, which leaves '/', ':' and friends + // unencoded. decodeURI is the matching inverse, and a percent + // escape such as %20 must still be decoded back to a space. + // The greedy .+ in the matcher relies on a literal suffix + // (here '/here') to backtrack, so include one for the test. + const template = new UriTemplate('http://example.com/{+path}/here'); + const expanded = template.expand({ path: 'a b/c' }); + expect(expanded).toBe('http://example.com/a%20b/c/here'); + expect(template.match(expanded)).toEqual({ path: 'a b/c' }); + }); + + it('round-trips a value through a /-prefixed path operator', () => { + const template = new UriTemplate('{/path}'); + expect(template.match(template.expand({ path: 'a/b c' }))).toEqual({ path: 'a/b c' }); + }); + + it('round-trips a value through a .-prefixed label operator', () => { + const template = new UriTemplate('X{.var}'); + expect(template.match(template.expand({ var: 'a b' }))).toEqual({ var: 'a b' }); + }); + + it('round-trips a value through a ?-prefixed form-style query operator', () => { + const template = new UriTemplate('X{?q}'); + expect(template.match(template.expand({ q: 'a b&c' }))).toEqual({ q: 'a b&c' }); + }); + + it('round-trips a value through a &-prefixed form-continuation operator', () => { + const template = new UriTemplate('X{&q}'); + expect(template.match(template.expand({ q: 'a b&c' }))).toEqual({ q: 'a b&c' }); + }); + + it('decodes each element of an exploded default-operator array', () => { + // Default-operator exploded values are joined by ',' on both the + // expand and the match sides, so the round trip is exact. + const template = new UriTemplate('{list*}'); + const expanded = template.expand({ list: ['a b', 'c/d', 'e#f'] }); + expect(template.match(expanded)).toEqual({ list: ['a b', 'c/d', 'e#f'] }); + }); + + it('passes a malformed escape through unchanged instead of throwing', () => { + // '%ZZ' is not a valid percent escape. The wire shape is whatever + // the server sent; the matcher should not reject it. + const template = new UriTemplate('file:///{path}'); + expect(template.match('file:///a%ZZb')).toEqual({ path: 'a%ZZb' }); + }); + }); + describe('security and edge cases', () => { it('should handle extremely long input strings', () => { const longString = 'x'.repeat(100_000);