From a790308d7fceab964fdb6b6346948582a3fb3f5f Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Thu, 15 Oct 2026 12:44:47 +0200 Subject: [PATCH] fix(resources): match RFC 6570 operators and prefer specific templates Templates using an RFC 6570 operator compiled to null, so they were registered and listed in resources/templates/list but resources/read never routed to them. Template matching was also first-registered-wins, so x://{a}/{b} shadowed a later x://files/{name}. Template matching moves to src/uri-template.ts and now supports {+var} (may contain '/'), {/a,b} optional path segments and trailing {?a,b}{&c} query parameters (order-tolerant, optional, unknown or repeated parameters do not match). Any other template (#, ., ; operators, multi-variable {a,b}, * and :n modifiers) throws at registration with the template in the message. Level-1 {name} matching and decoding are unchanged. When several templates match, the one with the most literal characters wins, ties going to registration order. Fixes #303 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01UF4S3w7iA83V9DVsasLJzM --- README.md | 11 ++- src/decorators/meta.ts | 4 + src/handlers.ts | 78 ++-------------- src/uri-template.ts | 152 ++++++++++++++++++++++++++++++++ test/resource-templates.test.ts | 115 +++++++++++++++++++++++- 5 files changed, 285 insertions(+), 75 deletions(-) create mode 100644 src/uri-template.ts diff --git a/README.md b/README.md index bfd15b3..37ba7c3 100644 --- a/README.md +++ b/README.md @@ -2280,7 +2280,16 @@ app.mcpAddTool({ Resource and prompt handlers also receive the same context object as tool handlers, enabling access to HTTP context and authorization information: -When `uriPattern` contains `{name}` placeholders (an RFC 6570 level-1 template), `resources/read` for a concrete URI such as `context://data/42` is routed to it, and the values are available as `context.uriParams` (`{ id: '42' }`). Each placeholder matches a single non-empty segment that does not contain `/`; an exact `uri` registration always takes precedence over a template. +When `uriPattern` is an RFC 6570 template, `resources/read` for a concrete URI such as `context://data/42` is routed to it, and the percent-decoded values are available as `context.uriParams` (`{ id: '42' }`). The supported expressions are: + +| Expression | Example | Matches | +|------------|---------|---------| +| `{name}` | `x://docs/{id}` | a single non-empty segment that does not contain `/` | +| `{+name}` | `files://{+path}` | a non-empty value that may contain `/` (but not `?` or `#`) | +| `{/a,b}` | `x://items{/id}` | optional `/value` segments | +| `{?a,b}`, `{&c}` | `search://docs{?q,limit}{&page}` | optional query parameters in any order; must come last in the template, and unknown or repeated parameters do not match | + +Registering a template that uses anything else (the `#`, `.` and `;` operators, `{a,b}` with several variables, or the `*` and `:n` modifiers) throws. An exact `uri` registration always takes precedence over a template; when several templates match, the one with the most literal characters wins (ties go to the first registered), so `x://files/{name}` wins over `x://{a}/{b}` for `x://files/a`. Values such as `{+path}` are not sanitized: validate them (for example against `..` segments) before using them as file paths. ```typescript // Resource handler with context diff --git a/src/decorators/meta.ts b/src/decorators/meta.ts index 9c40468..5813dc2 100644 --- a/src/decorators/meta.ts +++ b/src/decorators/meta.ts @@ -11,6 +11,7 @@ import type { ResourceUnsubscribeHandler } from '../types.ts' import { callRegisteredTool } from '../handlers.ts' +import { isTemplateUri, compileUriTemplate } from '../uri-template.ts' import { schemaToArguments, validateToolSchema, isTypeBoxSchema } from '../validation/index.ts' import type { JsonSchemaValidator } from '../validation/json-schema-validator.ts' @@ -97,6 +98,9 @@ const mcpDecoratorsPlugin: FastifyPluginAsync = async (app throw new Error('Resource definition must have a uri or uriPattern') } + // Throws for templates resources/read could never route to + if (isTemplateUri(uriPattern)) compileUriTemplate(uriPattern) + // Convert uriPattern to uri for the definition const resourceDefinition = { ...definition, diff --git a/src/handlers.ts b/src/handlers.ts index 985240c..d8c8f84 100644 --- a/src/handlers.ts +++ b/src/handlers.ts @@ -51,6 +51,7 @@ import type { JsonSchemaValidator } from './validation/json-schema-validator.ts' import { sanitizeToolParams, assessToolSecurity } from './security.ts' import { MCP_ATTR, type SpanAttributeValue } from './telemetry-constants.ts' import { isStdioRequest } from './stdio-marker.ts' +import { isTemplateUri, compileUriTemplate, matchUriTemplate } from './uri-template.ts' // Lazy-loaded telemetry module — only imported when a tracer is configured let _telemetry: typeof import('./telemetry.ts') | undefined @@ -281,73 +282,6 @@ async function handleToolsList (request: JSONRPCRequest, dependencies: HandlerDe return createResponse(request.id, result) } -const URI_TEMPLATE_REGEX = /\{[^}]+\}/ - -function isTemplateUri (uri: string): boolean { - return URI_TEMPLATE_REGEX.test(uri) -} - -interface CompiledUriTemplate { - regex: RegExp - names: string[] -} - -const compiledUriTemplates = new Map() - -/** - * Compile an RFC 6570 level-1 template (`{name}` placeholders only) into an - * anchored regex. Each placeholder matches one non-empty, slash-bounded - * segment; everything else must match literally. Templates using operators - * (`{+x}`, `{?x}`, ...) are not supported and never match. - */ -function compileUriTemplate (template: string): CompiledUriTemplate | null { - let compiled = compiledUriTemplates.get(template) - if (compiled !== undefined) return compiled - - const names: string[] = [] - let source = '^' - let lastIndex = 0 - let supported = true - for (const match of template.matchAll(/\{([^}]*)\}/g)) { - const name = match[1] - if (!/^[A-Za-z0-9_]+$/.test(name)) { - supported = false - break - } - source += escapeRegExp(template.slice(lastIndex, match.index)) + '([^/]+)' - names.push(name) - lastIndex = match.index + match[0].length - } - source += escapeRegExp(template.slice(lastIndex)) + '$' - - compiled = supported ? { regex: new RegExp(source), names } : null - compiledUriTemplates.set(template, compiled) - return compiled -} - -function escapeRegExp (value: string): string { - return value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') -} - -function matchUriTemplate (template: string, uri: string): Record | null { - const compiled = compileUriTemplate(template) - if (!compiled) return null - - const match = compiled.regex.exec(uri) - if (!match) return null - - const params: Record = {} - compiled.names.forEach((name, i) => { - const raw = match[i + 1] - try { - params[name] = decodeURIComponent(raw) - } catch { - params[name] = raw - } - }) - return params -} - // Fields shared by the spec Resource and ResourceTemplate shapes. Anything else // on a registered definition (uriPattern, uriSchema, ...) is a registration // input and must not be listed: uriSchema would leak validation patterns. @@ -779,16 +713,20 @@ async function handleResourcesRead ( } } - // Otherwise look for a registered template the concrete URI expands from + // Otherwise look for a registered template the concrete URI expands from, + // preferring the one with the most literal characters (ties: registration order) let uriParams: Record | undefined if (!resource) { + let bestLiteralLength = -1 for (const candidate of resources.values()) { if (!isTemplateUri(candidate.definition.uri)) continue - const params = matchUriTemplate(candidate.definition.uri, uri) + const compiled = compileUriTemplate(candidate.definition.uri) + if (compiled.literalLength <= bestLiteralLength) continue + const params = matchUriTemplate(compiled, uri) if (params) { resource = candidate uriParams = params - break + bestLiteralLength = compiled.literalLength } } } diff --git a/src/uri-template.ts b/src/uri-template.ts new file mode 100644 index 0000000..2cfb45c --- /dev/null +++ b/src/uri-template.ts @@ -0,0 +1,152 @@ +const URI_TEMPLATE_REGEX = /\{[^}]+\}/ +const VARNAME_REGEX = /^[A-Za-z0-9_]+$/ + +export function isTemplateUri (uri: string): boolean { + return URI_TEMPLATE_REGEX.test(uri) +} + +export interface CompiledUriTemplate { + // Anchored regex for the URI, or for the part before '?' when the template + // has query expressions + regex: RegExp + names: string[] + // Variables of the trailing {?...}{&...} expressions, null when there are none + queryNames: string[] | null + // Number of literal characters, used to prefer the most specific template + literalLength: number +} + +const compiledUriTemplates = new Map() + +/** + * Compile the supported subset of RFC 6570 into a matcher: + * + * - `{name}`: one non-empty segment without `/` + * - `{+name}`: one non-empty value that may contain `/` (but not `?` or `#`) + * - `{/a,b}`: optional `/value` path segments + * - `{?a,b}{&c}`: optional query parameters, in any order; these must + * trail the template + * + * Anything else (other operators, prefix/explode modifiers, multi-variable + * `{a,b}` or `{+a,b}`) throws, so a template that could never be read is + * rejected at registration instead of being listed. + */ +export function compileUriTemplate (template: string): CompiledUriTemplate { + let compiled = compiledUriTemplates.get(template) + if (compiled !== undefined) return compiled + + const unsupported = (reason: string) => new Error(`Unsupported URI template '${template}': ${reason}`) + + const names: string[] = [] + let queryNames: string[] | null = null + let source = '^' + let literalLength = 0 + let lastIndex = 0 + for (const match of template.matchAll(/\{([^}]*)\}/g)) { + const body = match[1] + const literal = template.slice(lastIndex, match.index) + lastIndex = match.index + match[0].length + literalLength += literal.length + + const operator = /^[+#./;?&=,!@|]/.test(body) ? body[0] : '' + const vars = body.slice(operator.length).split(',') + for (const name of vars) { + if (!VARNAME_REGEX.test(name)) { + throw unsupported(`expression '{${body}}' must list variable names made of letters, digits and '_' (modifiers are not supported)`) + } + } + + if (queryNames !== null && (literal !== '' || (operator !== '?' && operator !== '&'))) { + throw unsupported('query expressions {?...} and {&...} must come last') + } + + switch (operator) { + case '': + case '+': + if (vars.length > 1) { + throw unsupported(`expression '{${body}}' must have a single variable`) + } + source += escapeRegExp(literal) + (operator === '+' ? '([^?#]+)' : '([^/]+)') + names.push(vars[0]) + break + case '/': + source += escapeRegExp(literal) + vars.map(() => '(?:/([^/?#]+))?').join('') + names.push(...vars) + break + case '?': + if (queryNames !== null || template.slice(0, match.index).includes('?')) { + throw unsupported(`expression '{${body}}' follows another query component`) + } + source += escapeRegExp(literal) + queryNames = [...vars] + break + case '&': + if (queryNames === null) { + throw unsupported(`expression '{${body}}' must follow a {?...} expression`) + } + queryNames.push(...vars) + break + default: + throw unsupported(`operator '${operator}' is not supported`) + } + } + + const tail = template.slice(lastIndex) + if (queryNames !== null && tail !== '') { + throw unsupported('query expressions {?...} and {&...} must come last') + } + source += escapeRegExp(tail) + '$' + literalLength += tail.length + + compiled = { regex: new RegExp(source), names, queryNames, literalLength } + compiledUriTemplates.set(template, compiled) + return compiled +} + +function escapeRegExp (value: string): string { + return value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') +} + +function decode (raw: string): string { + try { + return decodeURIComponent(raw) + } catch { + return raw + } +} + +export function matchUriTemplate (compiled: CompiledUriTemplate, uri: string): Record | null { + let path = uri + let query: string | undefined + if (compiled.queryNames !== null) { + const index = uri.indexOf('?') + if (index !== -1) { + path = uri.slice(0, index) + query = uri.slice(index + 1) + } + } + + const match = compiled.regex.exec(path) + if (!match) return null + + const params: Record = {} + compiled.names.forEach((name, i) => { + const raw = match[i + 1] + if (raw !== undefined) params[name] = decode(raw) + }) + + if (query !== undefined) { + // Order-tolerant: each declared parameter may appear at most once, and + // any parameter the template does not declare means no match + const seen = new Set() + for (const pair of query.split('&')) { + const eq = pair.indexOf('=') + const name = eq === -1 ? pair : pair.slice(0, eq) + if (!compiled.queryNames!.includes(name) || seen.has(name)) return null + seen.add(name) + params[name] = decode(eq === -1 ? '' : pair.slice(eq + 1)) + } + } + + return params +} diff --git a/test/resource-templates.test.ts b/test/resource-templates.test.ts index 578d333..991e3f8 100644 --- a/test/resource-templates.test.ts +++ b/test/resource-templates.test.ts @@ -1,6 +1,7 @@ import { test, describe } from 'node:test' import type { TestContext } from 'node:test' import Fastify from 'fastify' +import type { FastifyInstance } from 'fastify' import { Type } from '@sinclair/typebox' import mcpPlugin from '../src/index.ts' import type { @@ -281,17 +282,123 @@ describe('Resource Templates', () => { t.assert.strictEqual(result.contents[0].text, 'concrete') }) - test('unsupported template operators never match', async (t: TestContext) => { + async function setup (t: TestContext) { const app = Fastify() t.after(() => app.close()) await app.register(mcpPlugin) await app.ready() + return app + } - app.mcpAddResource({ name: 'path', uriPattern: 'files://{+path}' }, async (uri) => ({ - contents: [{ uri, text: 'ok' }] + function addEcho (app: FastifyInstance, name: string, uriPattern: string) { + app.mcpAddResource({ name, uriPattern }, async (uri, context) => ({ + contents: [{ uri, text: JSON.stringify({ name, params: context.uriParams }) }] })) + } + + async function readEcho (app: ReturnType, uri: string) { + const body = await read(app, uri) + if (body.error) return body.error + const content = (body.result as ReadResourceResult).contents[0] + return 'text' in content ? JSON.parse(content.text) : undefined + } + + test('{+var} reserved expansion matches values containing slashes', async (t: TestContext) => { + const app = await setup(t) + addEcho(app, 'file', 'files://{+path}') + + t.assert.deepStrictEqual(await readEcho(app, 'files://a/b/c.txt'), { name: 'file', params: { path: 'a/b/c.txt' } }) + t.assert.deepStrictEqual(await readEcho(app, 'files://a%20b/c'), { name: 'file', params: { path: 'a b/c' } }) + t.assert.strictEqual((await readEcho(app, 'files://')).code, -32002) + }) + + test('{/var} path segment expansion is optional and does not span slashes', async (t: TestContext) => { + const app = await setup(t) + addEcho(app, 'items', 'x://items{/id,rev}') + + t.assert.deepStrictEqual(await readEcho(app, 'x://items/42'), { name: 'items', params: { id: '42' } }) + t.assert.deepStrictEqual(await readEcho(app, 'x://items/42/7'), { name: 'items', params: { id: '42', rev: '7' } }) + t.assert.deepStrictEqual(await readEcho(app, 'x://items'), { name: 'items', params: {} }) + t.assert.strictEqual((await readEcho(app, 'x://items/1/2/3')).code, -32002) + t.assert.strictEqual((await readEcho(app, 'x://items/')).code, -32002) + }) + + test('{?a,b} and {&c} query expansions match in any order and are optional', async (t: TestContext) => { + const app = await setup(t) + addEcho(app, 'search', 'search://docs{?q,limit}{&page}') + + t.assert.deepStrictEqual( + await readEcho(app, 'search://docs?page=2&limit=5&q=hello%20world'), + { name: 'search', params: { q: 'hello world', limit: '5', page: '2' } } + ) + t.assert.deepStrictEqual(await readEcho(app, 'search://docs?q='), { name: 'search', params: { q: '' } }) + t.assert.deepStrictEqual(await readEcho(app, 'search://docs'), { name: 'search', params: {} }) + + // Not an expansion of the template: unknown or repeated parameters + t.assert.strictEqual((await readEcho(app, 'search://docs?other=1')).code, -32002) + t.assert.strictEqual((await readEcho(app, 'search://docs?q=a&q=b')).code, -32002) + t.assert.strictEqual((await readEcho(app, 'search://docsx?q=a')).code, -32002) + }) + + test('query expansions combine with path expressions', async (t: TestContext) => { + const app = await setup(t) + addEcho(app, 'file', 'files://{+path}{?rev}') + addEcho(app, 'user', 'users://{id}{?fields}') + + t.assert.deepStrictEqual(await readEcho(app, 'files://a/b?rev=3'), { name: 'file', params: { path: 'a/b', rev: '3' } }) + t.assert.deepStrictEqual(await readEcho(app, 'files://a/b'), { name: 'file', params: { path: 'a/b' } }) + t.assert.deepStrictEqual(await readEcho(app, 'users://7?fields=name'), { name: 'user', params: { id: '7', fields: 'name' } }) + t.assert.strictEqual((await readEcho(app, 'users://7/8?fields=name')).code, -32002) + }) + + test('unsupported templates are rejected at registration', async (t: TestContext) => { + const app = await setup(t) + + for (const uriPattern of [ + 'x://doc{#section}', + 'x://file{.ext}', + 'x://map{;x,y}', + 'x://{a,b}', + 'x://{+a,b}', + 'x://{list*}', + 'x://{name:3}', + 'x://{?q}/more', + 'x://s?v=1{&q}', + 'x://s{&q}', + 'x://{a-b}', + 'x://{}x{=a}' + ]) { + t.assert.throws( + () => app.mcpAddResource({ name: 'bad', uriPattern }), + (err: Error) => err.message.includes(`'${uriPattern}'`), + uriPattern + ) + } + + const response = await app.inject({ + method: 'POST', + url: '/mcp', + payload: { jsonrpc: JSONRPC_VERSION, id: 1, method: 'resources/templates/list' } satisfies JSONRPCRequest + }) + const result = (response.json() as JSONRPCResultResponse).result as ListResourceTemplatesResult + t.assert.deepStrictEqual(result.resourceTemplates, []) + }) + + test('prefers the template with the most literal characters', async (t: TestContext) => { + const app = await setup(t) + addEcho(app, 'generic', 'x://{a}/{b}') + addEcho(app, 'files', 'x://files/{name}') + + t.assert.deepStrictEqual(await readEcho(app, 'x://files/a'), { name: 'files', params: { name: 'a' } }) + t.assert.deepStrictEqual(await readEcho(app, 'x://other/a'), { name: 'generic', params: { a: 'other', b: 'a' } }) + }) + + test('equally specific templates are tried in registration order', async (t: TestContext) => { + const app = await setup(t) + addEcho(app, 'first', 'x://{a}/b') + addEcho(app, 'second', 'x://a/{b}') - t.assert.strictEqual((await read(app, 'files://a')).error?.code, -32002) + t.assert.deepStrictEqual(await readEcho(app, 'x://a/b'), { name: 'first', params: { a: 'a' } }) }) })