Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/malformed-resource-uri-invalid-params.md
Original file line number Diff line number Diff line change
@@ -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.
5 changes: 5 additions & 0 deletions .changeset/probe-window-handler-restore.md
Original file line number Diff line number Diff line change
@@ -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.
6 changes: 6 additions & 0 deletions .changeset/standard-header-ows.md
Original file line number Diff line number Diff line change
@@ -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()`.
43 changes: 36 additions & 7 deletions packages/client/src/client/versionNegotiation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<ProbeWindow> {
const window = new ProbeWindow(transport);
Expand All @@ -197,18 +215,29 @@ 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;
if (pending !== undefined) {
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;
}

Expand All @@ -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. */
Expand Down
113 changes: 113 additions & 0 deletions packages/client/test/client/versionNegotiation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
});
38 changes: 35 additions & 3 deletions packages/core-internal/src/shared/inboundClassification.ts
Original file line number Diff line number Diff line change
Expand Up @@ -464,6 +464,25 @@ export const MCP_NAME_HEADER_SOURCE: Readonly<Record<string, 'name' | 'uri'>> =
'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
Expand Down Expand Up @@ -537,19 +556,20 @@ 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'
);
}
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'
);
Expand Down Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
10 changes: 9 additions & 1 deletion packages/server/src/server/mcp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()];
Expand Down
Loading
Loading