diff --git a/.changeset/uri-template-literal-encoding.md b/.changeset/uri-template-literal-encoding.md new file mode 100644 index 0000000000..9888ecc46e --- /dev/null +++ b/.changeset/uri-template-literal-encoding.md @@ -0,0 +1,11 @@ +--- +'@modelcontextprotocol/core-internal': patch +--- + +Pct-encode URI template literals per RFC 6570 §3.1, so a resource template whose literal text carries a character the URI grammar does not allow stays routable. + +`UriTemplate` copied literal runs verbatim, in both `expand()` and the pattern `match()` builds. §3.1 requires a literal outside the reserved/unreserved sets — `ucschar` (`café`), a space — to be pct-encoded as UTF-8 on expansion, so `expand()` returned a string that is not a valid RFC 3986 URI. + +The consequence was a resource that could be listed but never read. `resources/read` resolves the requested URI through `new URL()`, which pct-encodes it, and the raw literal in the pattern could not match that: a template such as `file:///docs/café/{name}` answered `-32602 Resource not found` for every URI a client could send, encoded or not. + +Literals are now encoded in both directions, with existing `%XX` triplets passing through unchanged so an already-encoded literal is not encoded twice. diff --git a/packages/core-internal/src/shared/uriTemplate.ts b/packages/core-internal/src/shared/uriTemplate.ts index 5ffe213acd..927480129e 100644 --- a/packages/core-internal/src/shared/uriTemplate.ts +++ b/packages/core-internal/src/shared/uriTemplate.ts @@ -24,6 +24,44 @@ export class UriTemplate { throw new Error(`${context} exceeds maximum length of ${max} characters (got ${str.length})`); } } + + /** + * Percent-encodes a literal run per RFC 6570 §3.1. + * + * A template may be written with characters the URI grammar does not allow — + * `ucschar` (`file:///docs/café/`) or a space — and §3.1 requires those to be + * pct-encoded as UTF-8 when the template is expanded. Reserved and unreserved + * characters are structural and stay as written, and existing `%XX` triplets + * pass through unchanged so an already-encoded literal is not encoded twice + * (`encodeURI` alone would turn `caf%C3%A9` into `caf%25C3%25A9`). + * + * Applied once in the constructor, so both directions read the same literal: + * `expand()` emits the encoded form and `match()` builds its pattern from it. + * An expanded URI therefore matches the template it came from — as does the + * pct-encoded URI a `new URL()` round-trip produces. + */ + private static encodeLiteral(text: string): string { + let result = ''; + let last = 0; + for (const match of text.matchAll(/%[0-9A-Fa-f]{2}/g)) { + result += UriTemplate.encodeLiteralRun(text.slice(last, match.index)) + match[0]; + last = match.index + match[0].length; + } + return result + UriTemplate.encodeLiteralRun(text.slice(last)); + } + + /** + * Encodes one run of literal text that holds no `%XX` triplet. + * + * `encodeURI` escapes `[` and `]`, which RFC 3986 reserves for an IPv6 host + * literal — §3.1 leaves reserved characters to the template author, and + * `new URL('http://[::1]/x')` keeps them — so they are restored here. The + * restore runs per gap rather than over the whole literal, so a `%5B` the + * author wrote themselves still passes through as a triplet. + */ + private static encodeLiteralRun(text: string): string { + return encodeURI(text).replaceAll('%5B', '[').replaceAll('%5D', ']'); + } private readonly template: string; private readonly parts: Array; @@ -34,7 +72,9 @@ export class UriTemplate { constructor(template: string) { UriTemplate.validateLength(template, MAX_TEMPLATE_LENGTH, 'Template'); this.template = template; - this.parts = this.parse(template); + // Literals are encoded once here rather than on every expand()/match(): + // they come from the template, so the result is the same every time. + this.parts = this.parse(template).map(part => (typeof part === 'string' ? UriTemplate.encodeLiteral(part) : part)); } toString(): string { @@ -259,6 +299,9 @@ export class UriTemplate { for (const part of this.parts) { if (typeof part === 'string') { + // Already encoded in the constructor, so the pattern lines up with + // what expand() emits and with the URI the server resolves through + // `new URL()` (RFC 6570 §3.1). pattern += this.escapeRegExp(part); } else { const patterns = this.partToRegExp(part); diff --git a/packages/core-internal/test/shared/uriTemplate.test.ts b/packages/core-internal/test/shared/uriTemplate.test.ts index bfc3237872..a4563ec8bc 100644 --- a/packages/core-internal/test/shared/uriTemplate.test.ts +++ b/packages/core-internal/test/shared/uriTemplate.test.ts @@ -85,6 +85,47 @@ describe('UriTemplate', () => { }); }); + describe('literal encoding (RFC 6570 §3.1)', () => { + it('should pct-encode a literal the URI grammar does not allow', () => { + // uritemplate-test, "Literal Encoding" + expect(new UriTemplate('café/{v}').expand({ v: 'value' })).toBe('caf%C3%A9/value'); + expect(new UriTemplate('file:///my docs/{v}').expand({ v: 'a.txt' })).toBe('file:///my%20docs/a.txt'); + }); + + it('should keep reserved and unreserved literal characters as written', () => { + expect(new UriTemplate('file:///a-b_c~d.e/{v}').expand({ v: 'x' })).toBe('file:///a-b_c~d.e/x'); + expect(new UriTemplate('http://x.test/p?q=1&r=2#{v}').expand({ v: 'x' })).toBe('http://x.test/p?q=1&r=2#x'); + }); + + it('should keep the brackets of an IPv6 host literal', () => { + // encodeURI escapes [ and ], but RFC 3986 reserves them for the host and + // `new URL()` keeps them, so the pattern has to keep them too. + const template = new UriTemplate('http://[::1]:8080/docs/{name}'); + expect(new URL('http://[::1]:8080/docs/a.txt').href).toBe('http://[::1]:8080/docs/a.txt'); + expect(template.expand({ name: 'a.txt' })).toBe('http://[::1]:8080/docs/a.txt'); + expect(template.match('http://[::1]:8080/docs/a.txt')).toEqual({ name: 'a.txt' }); + }); + + it('should not encode an already-encoded literal twice', () => { + expect(new UriTemplate('file:///docs/caf%C3%A9/{v}').expand({ v: 'a.txt' })).toBe('file:///docs/caf%C3%A9/a.txt'); + // a %5B the author wrote themselves stays a triplet + expect(new UriTemplate('file:///docs/%5Bx%5D/{v}').expand({ v: 'a.txt' })).toBe('file:///docs/%5Bx%5D/a.txt'); + }); + + it('should match the pct-encoded form a URL round-trip produces', () => { + const template = new UriTemplate('file:///docs/café/{name}'); + expect(new URL('file:///docs/café/a.txt').href).toBe('file:///docs/caf%C3%A9/a.txt'); + expect(template.match('file:///docs/caf%C3%A9/a.txt')).toEqual({ name: 'a.txt' }); + }); + + it('should round-trip expand through match', () => { + for (const source of ['file:///docs/café/{name}', 'file:///my docs/{name}']) { + const template = new UriTemplate(source); + expect(template.match(template.expand({ name: 'a.txt' }))).toEqual({ name: 'a.txt' }); + } + }); + }); + describe('matching', () => { it('should match simple strings and extract variables', () => { const template = new UriTemplate('http://example.com/users/{username}'); diff --git a/packages/server/test/server/resourceTemplateLiteralEncoding.test.ts b/packages/server/test/server/resourceTemplateLiteralEncoding.test.ts new file mode 100644 index 0000000000..af64e588f1 --- /dev/null +++ b/packages/server/test/server/resourceTemplateLiteralEncoding.test.ts @@ -0,0 +1,84 @@ +/** + * A resource template whose literal text carries characters the URI grammar does not + * allow — `ucschar` (`café`) or a space — stays routable. + * + * `resources/read` resolves the requested URI through `new URL()` before matching, so + * the template has to line up with the pct-encoded form (RFC 6570 §3.1), whichever form + * the client sent. + */ +import type { JSONRPCRequest, MessageClassification } from '@modelcontextprotocol/core-internal'; +import { + CLIENT_CAPABILITIES_META_KEY, + CLIENT_INFO_META_KEY, + PROTOCOL_VERSION_META_KEY, + setNegotiatedProtocolVersion +} from '@modelcontextprotocol/core-internal'; +import { describe, expect, it } from 'vitest'; + +import { invoke } from '../../src/server/invoke'; +import { McpServer, ResourceTemplate } from '../../src/server/mcp'; + +const MODERN_REVISION = '2026-07-28'; +const MODERN: MessageClassification = { era: 'modern', revision: MODERN_REVISION }; + +const ENVELOPE = { + [PROTOCOL_VERSION_META_KEY]: MODERN_REVISION, + [CLIENT_INFO_META_KEY]: { name: 'literal-encoding-client', version: '1.0.0' }, + [CLIENT_CAPABILITIES_META_KEY]: {} +}; + +const modernRequest = (method: string, params: Record = {}): JSONRPCRequest => + ({ + jsonrpc: '2.0', + id: 1, + method, + params: { ...params, _meta: ENVELOPE } + }) as JSONRPCRequest; + +function buildMcpServer(uriTemplate: string): McpServer { + const mcpServer = new McpServer({ name: 'literal-encoding-server', version: '1.0.0' }); + mcpServer.registerResource('docs', new ResourceTemplate(uriTemplate, { list: undefined }), {}, async (uri, { name }) => ({ + contents: [{ uri: uri.href, text: `contents of ${String(name)}` }] + })); + return mcpServer; +} + +async function modernBody(uriTemplate: string, method: string, params: Record = {}): Promise> { + const mcpServer = buildMcpServer(uriTemplate); + setNegotiatedProtocolVersion(mcpServer.server, MODERN_REVISION); + const response = await invoke(mcpServer, modernRequest(method, params), { classification: MODERN }); + return (await response.json()) as Record; +} + +describe('a resource template with a non-ASCII literal', () => { + it('is advertised with the template spelled as registered', async () => { + const body = (await modernBody('file:///docs/café/{name}', 'resources/templates/list')) as { + result: { resourceTemplates: { uriTemplate: string }[] }; + }; + expect(body.result.resourceTemplates[0]?.uriTemplate).toBe('file:///docs/café/{name}'); + }); + + it.each([ + ['pct-encoded', 'file:///docs/caf%C3%A9/a.txt'], + ['raw', 'file:///docs/café/a.txt'] + ])('is read when the client sends the %s URI', async (_form, uri) => { + const body = (await modernBody('file:///docs/café/{name}', 'resources/read', { uri })) as { + result?: { contents: { uri: string; text: string }[] }; + error?: unknown; + }; + expect(body.error).toBeUndefined(); + expect(body.result?.contents[0]).toMatchObject({ + uri: 'file:///docs/caf%C3%A9/a.txt', + text: 'contents of a.txt' + }); + }); + + it('is read when the literal carries a space', async () => { + const body = (await modernBody('file:///my docs/{name}', 'resources/read', { uri: 'file:///my%20docs/a.txt' })) as { + result?: { contents: { text: string }[] }; + error?: unknown; + }; + expect(body.error).toBeUndefined(); + expect(body.result?.contents[0]?.text).toBe('contents of a.txt'); + }); +});