From 7e697354de95111ca2c70a12ac9f5d3ec96b56c3 Mon Sep 17 00:00:00 2001 From: Matt <77928207+mattzcarey@users.noreply.github.com> Date: Tue, 7 Jul 2026 13:48:48 +0100 Subject: [PATCH 1/3] fix(server): return Invalid Params for malformed resource URIs (#2451) --- .../malformed-resource-uri-invalid-params.md | 5 ++ packages/server/src/server/mcp.ts | 10 ++- test/integration/test/server/mcp.test.ts | 74 +++++++++++++++++++ 3 files changed, 88 insertions(+), 1 deletion(-) create mode 100644 .changeset/malformed-resource-uri-invalid-params.md diff --git a/.changeset/malformed-resource-uri-invalid-params.md b/.changeset/malformed-resource-uri-invalid-params.md new file mode 100644 index 0000000000..565de8bad2 --- /dev/null +++ b/.changeset/malformed-resource-uri-invalid-params.md @@ -0,0 +1,5 @@ +--- +'@modelcontextprotocol/server': patch +--- + +Return JSON-RPC Invalid Params with the original URI and an `invalid_uri` reason when `resources/read` receives a syntactically malformed URI. diff --git a/packages/server/src/server/mcp.ts b/packages/server/src/server/mcp.ts index e0d10b5a8e..d2e40181e4 100644 --- a/packages/server/src/server/mcp.ts +++ b/packages/server/src/server/mcp.ts @@ -476,7 +476,15 @@ export class McpServer { }); this.server.setRequestHandler('resources/read', async (request, ctx) => { - const uri = new URL(request.params.uri); + let uri: URL; + try { + uri = new URL(request.params.uri); + } catch { + throw new ProtocolError(ProtocolErrorCode.InvalidParams, `Resource URI ${request.params.uri} is invalid`, { + uri: request.params.uri, + reason: 'invalid_uri' + }); + } // First check for exact resource match const resource = this._registeredResources[uri.toString()]; diff --git a/test/integration/test/server/mcp.test.ts b/test/integration/test/server/mcp.test.ts index 2597c14f84..4b9a3865f0 100644 --- a/test/integration/test/server/mcp.test.ts +++ b/test/integration/test/server/mcp.test.ts @@ -2743,6 +2743,80 @@ describe('Zod v4', () => { }); }); + test('should echo the exact requested URI for nonexistent resources', async () => { + const mcpServer = new McpServer({ + name: 'test server', + version: '1.0' + }); + const client = new Client({ + name: 'test client', + version: '1.0' + }); + const requestedUri = 'HTTP://example.com:80/docs/../missing file.txt'; + mcpServer.registerResource('test', 'test://resource', {}, async () => ({ + contents: [ + { + uri: 'test://resource', + text: 'Test content' + } + ] + })); + + const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); + + await Promise.all([client.connect(clientTransport), mcpServer.server.connect(serverTransport)]); + + await expect( + client.request({ + method: 'resources/read', + params: { + uri: requestedUri + } + }) + ).rejects.toMatchObject({ + code: ProtocolErrorCode.InvalidParams, + message: expect.stringContaining('not found'), + data: { uri: requestedUri } + }); + }); + + test('should return invalid params for syntactically invalid resource URIs', async () => { + const mcpServer = new McpServer({ + name: 'test server', + version: '1.0' + }); + const client = new Client({ + name: 'test client', + version: '1.0' + }); + const requestedUri = 'not a valid URI'; + mcpServer.registerResource('test', 'test://resource', {}, async () => ({ + contents: [ + { + uri: 'test://resource', + text: 'Test content' + } + ] + })); + + const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); + + await Promise.all([client.connect(clientTransport), mcpServer.server.connect(serverTransport)]); + + await expect( + client.request({ + method: 'resources/read', + params: { + uri: requestedUri + } + }) + ).rejects.toMatchObject({ + code: ProtocolErrorCode.InvalidParams, + message: expect.stringContaining('invalid'), + data: { uri: requestedUri, reason: 'invalid_uri' } + }); + }); + /*** * Test: ProtocolError for Disabled Resource */ From 0ab5d1471d6c7375878316df2930fca77eee1d2a Mon Sep 17 00:00:00 2001 From: Matt <77928207+mattzcarey@users.noreply.github.com> Date: Tue, 7 Jul 2026 14:22:45 +0100 Subject: [PATCH 2/3] fix(server): trim OWS from standard MCP headers (#2453) --- .changeset/standard-header-ows.md | 6 +++ .../src/shared/inboundClassification.ts | 38 ++++++++++++++-- .../shared/standardHeaderValidation.test.ts | 44 +++++++++++++++++++ 3 files changed, 85 insertions(+), 3 deletions(-) create mode 100644 .changeset/standard-header-ows.md diff --git a/.changeset/standard-header-ows.md b/.changeset/standard-header-ows.md new file mode 100644 index 0000000000..4426652246 --- /dev/null +++ b/.changeset/standard-header-ows.md @@ -0,0 +1,6 @@ +--- +'@modelcontextprotocol/server': patch +'@modelcontextprotocol/core-internal': patch +--- + +Strip RFC 9110 optional whitespace around inbound `MCP-Protocol-Version`, `Mcp-Method`, and `Mcp-Name` values before classifying and validating modern HTTP requests. This keeps valid requests portable across Fetch runtimes that expose raw leading or trailing SP/HTAB through `Headers.get()`. diff --git a/packages/core-internal/src/shared/inboundClassification.ts b/packages/core-internal/src/shared/inboundClassification.ts index 35f694cae1..22883cf488 100644 --- a/packages/core-internal/src/shared/inboundClassification.ts +++ b/packages/core-internal/src/shared/inboundClassification.ts @@ -464,6 +464,25 @@ export const MCP_NAME_HEADER_SOURCE: Readonly> = 'resources/read': 'uri' }; +/** Strip RFC 9110 optional whitespace (SP / HTAB) around a field value in linear time. */ +function stripHttpOws(value: string): string { + let start = 0; + while (start < value.length) { + const code = value.codePointAt(start); + if (code !== 0x09 && code !== 0x20) break; + start += 1; + } + + let end = value.length; + while (end > start) { + const code = value.codePointAt(end - 1); + if (code !== 0x09 && code !== 0x20) break; + end -= 1; + } + + return start === 0 && end === value.length ? value : value.slice(start, end); +} + /** * SEP-2243 standard-header server-side validation, evaluated by the HTTP * entry on a modern-classified request immediately after @@ -537,11 +556,12 @@ export function validateStandardRequestHeaders(request: InboundHttpRequest, rout ); } - const decoded = decodeMcpParamValue(request.mcpNameHeader); + const normalizedNameHeader = stripHttpOws(request.mcpNameHeader); + const decoded = decodeMcpParamValue(normalizedNameHeader); if (decoded === undefined) { return crossCheckMismatch( 'name-header-invalid-encoding', - request.mcpNameHeader, + normalizedNameHeader, 'the Mcp-Name header carries an invalid Base64 sentinel value', 'standard-header-validation' ); @@ -549,7 +569,7 @@ export function validateStandardRequestHeaders(request: InboundHttpRequest, rout if (bodyValue !== undefined && decoded !== bodyValue) { return crossCheckMismatch( 'name-header-mismatch', - request.mcpNameHeader, + normalizedNameHeader, `the body carries params.${sourceField}="${bodyValue}" but the Mcp-Name header names "${decoded}"`, 'standard-header-validation' ); @@ -818,6 +838,18 @@ function classifyNotificationBody(request: InboundHttpRequest, body: JSONRPCNoti * `modern`) or a ladder rejection; it never throws. */ export function classifyInboundRequest(request: InboundHttpRequest): InboundClassificationOutcome { + // RFC 9110 §5.5: field parsing excludes optional whitespace around a + // field value. Fetch implementations normally perform this normalization, + // but transport-neutral callers and some runtimes can expose raw OWS. + request = { + ...request, + ...(request.protocolVersionHeader !== undefined && { + protocolVersionHeader: stripHttpOws(request.protocolVersionHeader) + }), + ...(request.mcpMethodHeader !== undefined && { mcpMethodHeader: stripHttpOws(request.mcpMethodHeader) }), + ...(request.mcpNameHeader !== undefined && { mcpNameHeader: stripHttpOws(request.mcpNameHeader) }) + }; + if (request.httpMethod.toUpperCase() !== 'POST') { // Body-less 2025-era session operations (and any other non-POST // method): the modern era is POST-only. diff --git a/packages/core-internal/test/shared/standardHeaderValidation.test.ts b/packages/core-internal/test/shared/standardHeaderValidation.test.ts index 8a150375a9..99de31457b 100644 --- a/packages/core-internal/test/shared/standardHeaderValidation.test.ts +++ b/packages/core-internal/test/shared/standardHeaderValidation.test.ts @@ -157,6 +157,50 @@ describe('SEP-2243 standard-header validation (Mcp-Name presence and cross-check expectRejection(validateStandardRequestHeaders(request, route), 'name-header-invalid-encoding'); }); + test('raw HTTP OWS is stripped from standard MCP header values', () => { + const { request } = modernPost( + 'tools/call', + { name: 'echo', arguments: {} }, + { mcpMethod: '\t tools/call ', mcpName: ' echo \t' } + ); + request.protocolVersionHeader = ` ${MODERN}\t`; + const classified = classifyInboundRequest(request); + expect(classified.kind).toBe('modern'); + if (classified.kind !== 'modern') { + throw new Error(`expected a modern route, got ${classified.kind}`); + } + expect(validateStandardRequestHeaders(request, classified)).toBeUndefined(); + }); + + test('long OWS runs are stripped without changing an encoded name', () => { + const ows = '\t '.repeat(50_000); + const name = ' echo '; + const { request } = modernPost( + 'tools/call', + { name, arguments: {} }, + { + mcpMethod: `${ows}tools/call${ows}`, + mcpName: `${ows}${encodeMcpParamValue(name)}${ows}` + } + ); + request.protocolVersionHeader = `${ows}${MODERN}${ows}`; + + const classified = classifyInboundRequest(request); + expect(classified.kind).toBe('modern'); + if (classified.kind !== 'modern') { + throw new Error(`expected a modern route, got ${classified.kind}`); + } + expect(validateStandardRequestHeaders(request, classified)).toBeUndefined(); + }); + + test('whitespace outside RFC 9110 OWS is not stripped', () => { + const { request } = modernPost('tools/call', { name: 'echo', arguments: {} }, { mcpMethod: 'tools/call', mcpName: 'echo' }); + request.mcpMethodHeader = '\u00a0tools/call'; + const classified = classifyInboundRequest(request); + expect(classified.kind).toBe('reject'); + expect((classified as InboundLadderRejection).cell).toBe('method-header-mismatch'); + }); + test('a matching Mcp-Name on a prompts/get passes', () => { const { request, route } = modernPost('prompts/get', { name: 'greeting' }, { mcpMethod: 'prompts/get', mcpName: 'greeting' }); expect(validateStandardRequestHeaders(request, route)).toBeUndefined(); From cc70c5e6a9f9b1c15dcba0bdd019a479b81375de Mon Sep 17 00:00:00 2001 From: Felix Weinberger <3823880+felixweinberger@users.noreply.github.com> Date: Tue, 7 Jul 2026 15:09:41 +0100 Subject: [PATCH 3/3] fix(client): preserve pre-set transport handlers across the version-negotiation probe window (#2455) --- .changeset/probe-window-handler-restore.md | 5 + .../client/src/client/versionNegotiation.ts | 43 +++++-- .../test/client/versionNegotiation.test.ts | 113 ++++++++++++++++++ 3 files changed, 154 insertions(+), 7 deletions(-) create mode 100644 .changeset/probe-window-handler-restore.md diff --git a/.changeset/probe-window-handler-restore.md b/.changeset/probe-window-handler-restore.md new file mode 100644 index 0000000000..6256ba05e6 --- /dev/null +++ b/.changeset/probe-window-handler-restore.md @@ -0,0 +1,5 @@ +--- +'@modelcontextprotocol/client': patch +--- + +Version negotiation no longer discards transport handlers the caller set before `connect()`. The probe window now saves any pre-set `onmessage`/`onerror`/`onclose`, forwards error and close events to them while the probe is in flight, and restores them when the window closes — so `Protocol.connect()` chains them exactly as it does on a plain connect. Previously, connecting with `versionNegotiation` silently cleared pre-set handlers (e.g. an `onerror` used to detect session-expiry auth failures), leaving them permanently detached for the life of the connection. diff --git a/packages/client/src/client/versionNegotiation.ts b/packages/client/src/client/versionNegotiation.ts index d438ff7a81..628d5d7bc1 100644 --- a/packages/client/src/client/versionNegotiation.ts +++ b/packages/client/src/client/versionNegotiation.ts @@ -171,12 +171,30 @@ type RawProbeReply = * starts the transport; `release()` detaches them and arms a one-shot `start()` * pass-through so the subsequent Protocol connect (which always starts its * transport) takes over the already-started channel without a double-start error. + * + * Handlers a caller pre-set on the transport before `connect()` are saved on + * open and restored on detach, so `Protocol.connect()` finds and chains them + * exactly as it would on a plain (non-negotiating) connect. Error and close + * events are forwarded to the saved handlers during the window, so negotiation + * does not change what a pre-attached observer sees of the transport + * lifecycle. (Inbound messages are deliberately NOT forwarded — the window's + * drop-guard is a module invariant, and the probe reply has no plain-connect + * counterpart; a pre-set `onmessage` is restored at detach and chained by + * `Protocol.connect()` like the others.) */ class ProbeWindow { private _pending: { id: string; resolve: (reply: RawProbeReply) => void } | undefined; private _probeCounter = 0; + private readonly _savedOnMessage: Transport['onmessage']; + private readonly _savedOnError: Transport['onerror']; + private readonly _savedOnClose: Transport['onclose']; + private _closeDelivered = false; - private constructor(private readonly _transport: Transport) {} + private constructor(private readonly _transport: Transport) { + this._savedOnMessage = _transport.onmessage; + this._savedOnError = _transport.onerror; + this._savedOnClose = _transport.onclose; + } static async open(transport: Transport): Promise { const window = new ProbeWindow(transport); @@ -197,9 +215,10 @@ class ProbeWindow { } // Probe-window guard: drop everything else with zero bytes written back (see module doc). }; - transport.onerror = () => { + transport.onerror = error => { // Out-of-band transport errors are not necessarily fatal; the probe // resolves via a send failure, the close signal, or the timeout. + window._savedOnError?.(error); }; transport.onclose = () => { const pending = window._pending; @@ -207,8 +226,18 @@ class ProbeWindow { window._pending = undefined; pending.resolve({ kind: 'closed' }); } + // Forward exactly once: after a mid-window close is delivered, the + // restored handler must not re-deliver it when cleanup paths call + // `transport.close()` again. + window._closeDelivered = true; + window._savedOnClose?.(); }; - await transport.start(); + try { + await transport.start(); + } catch (error) { + window.detach(); + throw error; + } return window; } @@ -235,12 +264,12 @@ class ProbeWindow { }); } - /** Detach the window's handlers, leaving the transport's own `start` untouched. */ + /** Detach the window's handlers, restoring any the caller pre-set, leaving the transport's own `start` untouched. */ detach(): void { this._pending = undefined; - this._transport.onmessage = undefined; - this._transport.onerror = undefined; - this._transport.onclose = undefined; + this._transport.onmessage = this._savedOnMessage; + this._transport.onerror = this._savedOnError; + this._transport.onclose = this._closeDelivered ? undefined : this._savedOnClose; } /** Detach the handlers and arm the one-shot `start()` pass-through for the `Protocol.connect()` handover. */ diff --git a/packages/client/test/client/versionNegotiation.test.ts b/packages/client/test/client/versionNegotiation.test.ts index 86d901e24a..698bccd6ee 100644 --- a/packages/client/test/client/versionNegotiation.test.ts +++ b/packages/client/test/client/versionNegotiation.test.ts @@ -782,3 +782,116 @@ describe('probe send-error classification', () => { expect(requests(transport.sent).some(r => r.method === 'initialize')).toBe(false); }); }); + +/* ------------------------------------------------------------------------- * + * Probe window handler preservation: handlers pre-set on the transport + * before connect() survive negotiation exactly as they survive a plain + * connect — Protocol.connect() must find and chain them after the window. + * ------------------------------------------------------------------------- */ + +describe('probe window preserves pre-set transport handlers', () => { + test('pre-set onerror/onclose are restored after a modern negotiation and reachable through the Protocol chain', async () => { + const transport = new ScriptedTransport(modernServerScript()); + const seenErrors: Error[] = []; + let closed = 0; + transport.onerror = error => { + seenErrors.push(error); + }; + transport.onclose = () => { + closed++; + }; + + const client = new Client({ name: 'c', version: '0' }, { versionNegotiation: { mode: 'auto' } }); + await client.connect(transport); + + // The window restored the handler, so Protocol.connect chained it: + // post-connect transport errors still reach the pre-set observer. + const boom = new Error('post-connect transport error'); + transport.onerror?.(boom); + expect(seenErrors).toContain(boom); + + await client.close(); + expect(closed).toBeGreaterThan(0); + }); + + test('pre-set onerror survives the legacy fallback path too', async () => { + const transport = new ScriptedTransport(legacyServerScript); + const seenErrors: Error[] = []; + transport.onerror = error => { + seenErrors.push(error); + }; + + const client = new Client({ name: 'c', version: '0' }, { versionNegotiation: { mode: 'auto' } }); + await client.connect(transport); + expect(client.getNegotiatedProtocolVersion()).toBe('2025-11-25'); + + const boom = new Error('post-fallback transport error'); + transport.onerror?.(boom); + expect(seenErrors).toContain(boom); + + await client.close(); + }); + + test('failed negotiation (pin mode, no fallback) restores handlers via the detach path — onclose fires exactly once', async () => { + const transport = new ScriptedTransport(legacyServerScript); + const presetOnError = (_error: Error) => {}; + let closes = 0; + transport.onerror = presetOnError; + transport.onclose = () => { + closes++; + }; + + const client = new Client({ name: 'c', version: '0' }, { versionNegotiation: { mode: { pin: MODERN } } }); + await expect(client.connect(transport)).rejects.toThrow(); + + // detach() restored the pre-set handlers, and Client's cleanup close + // delivered the close event exactly once. + expect(transport.onerror).toBe(presetOnError); + expect(closes).toBe(1); + }); + + test('a mid-probe transport close reaches the pre-set onclose exactly once (no re-delivery from cleanup close)', async () => { + const script: Script = (message, t) => { + if (!isJSONRPCRequest(message)) return; + if (message.method === 'server/discover') { + t.onclose?.(); + return; + } + legacyServerScript(message, t); + }; + const transport = new ScriptedTransport(script); + let closes = 0; + transport.onclose = () => { + closes++; + }; + + const client = new Client({ name: 'c', version: '0' }, { versionNegotiation: { mode: 'auto' } }); + await client.connect(transport).catch(() => undefined); + + expect(closes).toBe(1); + }); + + test('transport errors DURING the probe window are forwarded to the pre-set handler', async () => { + const duringProbe = new Error('mid-probe transport error'); + const script: Script = (message, t) => { + if (!isJSONRPCRequest(message)) return; + if (message.method === 'server/discover') { + t.onerror?.(duringProbe); + t.reply({ jsonrpc: '2.0', id: message.id, error: { code: -32_601, message: 'Method not found' } }); + return; + } + legacyServerScript(message, t); + }; + const transport = new ScriptedTransport(script); + const seenErrors: Error[] = []; + transport.onerror = error => { + seenErrors.push(error); + }; + + const client = new Client({ name: 'c', version: '0' }, { versionNegotiation: { mode: 'auto' } }); + await client.connect(transport); + + expect(seenErrors).toContain(duringProbe); + await client.close(); + }); +});