diff --git a/.changeset/content-type-media-type-validation.md b/.changeset/content-type-media-type-validation.md new file mode 100644 index 0000000000..9f820a64ad --- /dev/null +++ b/.changeset/content-type-media-type-validation.md @@ -0,0 +1,6 @@ +--- +'@modelcontextprotocol/sdk': patch +--- + +Validate `Content-Type` by its parsed media type instead of a substring match. POSTs to the Streamable HTTP server transport whose media type is not `application/json` are now rejected with `415 Unsupported Media Type` (previously any value containing the substring passed, and +valid case variants were wrongly rejected). Values with parameters (`application/json; charset=utf-8`, including malformed parameter sections like `application/json;`) continue to work, and the client's response dispatch uses the same parsed comparison. diff --git a/eslint.config.mjs b/eslint.config.mjs index dd06491eef..0bac3745a8 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -23,6 +23,23 @@ export default tseslint.config( { ignores: ['src/spec.types.ts'] }, + { + files: ['src/**/*.ts'], + rules: { + 'no-restricted-syntax': [ + 'error', + { + selector: + ":matches(CallExpression[callee.property.name='includes'], CallExpression[callee.property.name='indexOf'], " + + "CallExpression[callee.property.name='startsWith'])[arguments.0.value='application/json']", + message: + "Substring-matching 'application/json' misclassifies Content-Type values whose media type is different " + + "(e.g. 'text/plain; a=application/json') and mishandles parameters and case. " + + 'Parse the media type instead: isJsonContentType() from src/shared/mediaType.js.' + } + ] + } + }, { files: ['src/client/**/*.ts', 'src/server/**/*.ts'], ignores: ['**/*.test.ts'], diff --git a/src/client/streamableHttp.ts b/src/client/streamableHttp.ts index 736587973d..5ec6fdde8d 100644 --- a/src/client/streamableHttp.ts +++ b/src/client/streamableHttp.ts @@ -1,3 +1,4 @@ +import { mediaTypeEssence } from '../shared/mediaType.js'; import { Transport, FetchLike, createFetchWithInit, normalizeHeaders } from '../shared/transport.js'; import { isInitializedNotification, isJSONRPCRequest, isJSONRPCResultResponse, JSONRPCMessage, JSONRPCMessageSchema } from '../types.js'; import { auth, AuthResult, extractWWWAuthenticateParams, OAuthClientProvider, UnauthorizedError } from './auth.js'; @@ -572,16 +573,17 @@ export class StreamableHTTPClientTransport implements Transport { const hasRequests = messages.filter(msg => 'method' in msg && 'id' in msg && msg.id !== undefined).length > 0; - // Check the response type + // Check the response type (parsed media type — see mediaTypeEssence) const contentType = response.headers.get('content-type'); + const responseMediaType = mediaTypeEssence(contentType); if (hasRequests) { - if (contentType?.includes('text/event-stream')) { + if (responseMediaType === 'text/event-stream') { // Handle SSE stream responses for requests // We use the same handler as standalone streams, which now supports // reconnection with the last event ID this._handleSseStream(response.body, { onresumptiontoken }, false); - } else if (contentType?.includes('application/json')) { + } else if (responseMediaType === 'application/json') { // For non-streaming servers, we might get direct JSON responses const data = await response.json(); const responseMessages = Array.isArray(data) diff --git a/src/server/webStandardStreamableHttp.ts b/src/server/webStandardStreamableHttp.ts index 1f528427c8..5721d3e370 100644 --- a/src/server/webStandardStreamableHttp.ts +++ b/src/server/webStandardStreamableHttp.ts @@ -7,6 +7,7 @@ * For Node.js Express/HTTP compatibility, use `StreamableHTTPServerTransport` which wraps this transport. */ +import { isJsonContentType } from '../shared/mediaType.js'; import { Transport } from '../shared/transport.js'; import { AuthInfo } from './auth/types.js'; import { @@ -599,6 +600,8 @@ export class WebStandardStreamableHTTPServerTransport implements Transport { // Validate the Accept header const acceptHeader = req.headers.get('accept'); // The client MUST include an Accept header, listing both application/json and text/event-stream as supported content types. + // Accept is a comma-separated list, so a substring check is the intended semantics here (unlike Content-Type below). + // eslint-disable-next-line no-restricted-syntax if (!acceptHeader?.includes('application/json') || !acceptHeader.includes('text/event-stream')) { this.onerror?.(new Error('Not Acceptable: Client must accept both application/json and text/event-stream')); return this.createJsonErrorResponse( @@ -608,8 +611,9 @@ export class WebStandardStreamableHTTPServerTransport implements Transport { ); } + // Parsed media type, never a substring match — see isJsonContentType. const ct = req.headers.get('content-type'); - if (!ct || !ct.includes('application/json')) { + if (!isJsonContentType(ct)) { this.onerror?.(new Error('Unsupported Media Type: Content-Type must be application/json')); return this.createJsonErrorResponse(415, -32000, 'Unsupported Media Type: Content-Type must be application/json'); } diff --git a/src/shared/mediaType.ts b/src/shared/mediaType.ts new file mode 100644 index 0000000000..aaa7a3533f --- /dev/null +++ b/src/shared/mediaType.ts @@ -0,0 +1,57 @@ +import contentType from 'content-type'; + +/** + * Extracts the media type (the lowercased `type/subtype` pair, without + * parameters) from a raw `Content-Type` header value, or `undefined` when the + * header is missing or empty. + * + * Content-Type comparisons must use the parsed media type, never a substring + * search of the raw header: a value like `text/plain; a=application/json` + * contains the substring `application/json` but its media type is + * `text/plain`, and case variants or parameters make naive string comparison + * wrong in both directions. + * + * "Essence" is the WHATWG MIME Sniffing standard's term for the bare + * `type/subtype` pair (https://mimesniff.spec.whatwg.org/#mime-type-essence); + * the Fetch standard's request classification is defined against it + * (https://fetch.spec.whatwg.org/#cors-safelisted-request-header). + * + * Parsing is RFC 9110 (`content-type` package) first. When the parameter + * section is malformed (`application/json;`, `application/json; charset=`), + * browsers and most HTTP stacks still derive the media type from the segment + * before the first `;` — the fallback matches that widely-implemented + * behavior, so a header whose media type is unambiguous is not rejected for + * a sloppy parameter section. + */ +export function mediaTypeEssence(header: string | null | undefined): string | undefined { + if (!header) { + return undefined; + } + try { + return contentType.parse(header).type; + } catch { + const essence = (header.split(';', 1)[0] ?? '').trim().toLowerCase(); + // A comma in the parameter tail of an unparseable value indicates + // joined duplicate headers — ambiguous, so no essence at all (keeps + // duplicate-header handling uniform whether or not the first copy + // carries parameters). + if (essence === '' || header.slice(essence.length).includes(',')) { + return undefined; + } + return essence; + } +} + +/** + * Whether a raw `Content-Type` header value denotes `application/json`. + * Parameters (for example `charset=utf-8`) are allowed and ignored; malformed + * parameter sections do not reject a header whose media type is unambiguously + * `application/json` (see `mediaTypeEssence` for the exact grammar). + */ +export function isJsonContentType(header: string | null | undefined): boolean { + // Fast path: the exact literal is what SDK clients send on every POST. + if (header === 'application/json') { + return true; + } + return mediaTypeEssence(header) === 'application/json'; +} diff --git a/test/server/contentTypeValidation.test.ts b/test/server/contentTypeValidation.test.ts new file mode 100644 index 0000000000..a0aed78e7b --- /dev/null +++ b/test/server/contentTypeValidation.test.ts @@ -0,0 +1,115 @@ +/** + * Content-Type validation at the Streamable HTTP entry — the parsed media + * type decides, never a substring search of the raw header. + * + * The shape pinned here: `Content-Type: text/plain; a=application/json` + * contains the substring `application/json`, but its media type is + * `text/plain` — the transport must answer it (and any other non-JSON media + * type) with 415 before the body is dispatched, while values whose media type + * is `application/json` keep working regardless of parameters or case. + */ +import { z } from 'zod'; +import { McpServer } from '../../src/server/mcp.js'; +import { WebStandardStreamableHTTPServerTransport } from '../../src/server/webStandardStreamableHttp.js'; +import { CallToolResult } from '../../src/types.js'; + +const executed: string[] = []; + +function factory(): McpServer { + const mcpServer = new McpServer({ name: 'ct-fixture', version: '1.0.0' }); + mcpServer.tool('run', 'records each dispatch', { cmd: z.string() }, async ({ cmd }): Promise => { + executed.push(cmd); + return { content: [{ type: 'text', text: `ran: ${cmd}` }] }; + }); + return mcpServer; +} + +const TOOL_CALL = { + jsonrpc: '2.0', + id: 1, + method: 'tools/call', + params: { name: 'run', arguments: { cmd: 'hello' } } +}; + +function postRequest(body: unknown, headers: Record): Request { + return new Request('http://127.0.0.1/mcp', { + method: 'POST', + headers: { + Accept: 'application/json, text/event-stream', + ...headers + }, + body: JSON.stringify(body) + }); +} + +async function sendToTransport(request: Request): Promise { + const server = factory(); + const transport = new WebStandardStreamableHTTPServerTransport({ + sessionIdGenerator: undefined, + enableJsonResponse: true + }); + await server.connect(transport); + const response = await transport.handleRequest(request); + await server.close(); + return response; +} + +async function postToTransport(headers: Record): Promise { + return sendToTransport(postRequest(TOOL_CALL, headers)); +} + +beforeEach(() => { + executed.length = 0; +}); + +describe('WebStandardStreamableHTTPServerTransport Content-Type validation', () => { + it('serves an application/json POST (control)', async () => { + const response = await postToTransport({ 'Content-Type': 'application/json' }); + expect(response.status).toBe(200); + expect(executed).toEqual(['hello']); + }); + + it('accepts application/json with parameters', async () => { + const response = await postToTransport({ 'Content-Type': 'application/json; charset=utf-8' }); + expect(response.status).toBe(200); + expect(executed).toEqual(['hello']); + }); + + it('rejects text/plain with 415', async () => { + const response = await postToTransport({ 'Content-Type': 'text/plain' }); + expect(response.status).toBe(415); + expect(executed).toEqual([]); + }); + + it('rejects a non-JSON media type whose parameters contain `application/json` and does not dispatch', async () => { + const response = await postToTransport({ 'Content-Type': 'text/plain; a=application/json' }); + expect(response.status).toBe(415); + const body = (await response.json()) as { error: { code: number; message: string } }; + expect(body.error.code).toBe(-32000); + expect(body.error.message).toContain('Content-Type must be application/json'); + expect(executed).toEqual([]); + }); + + it('rejects a POST with no Content-Type header with 415', async () => { + // A string body makes Request auto-attach `text/plain;charset=UTF-8`; + // delete it so this actually exercises the absent-header branch. + const request = postRequest(TOOL_CALL, {}); + request.headers.delete('content-type'); + expect(request.headers.get('content-type')).toBeNull(); + const response = await sendToTransport(request); + expect(response.status).toBe(415); + expect(executed).toEqual([]); + }); + + it('accepts an unambiguous media type with a malformed parameter section (trailing semicolon)', async () => { + const response = await postToTransport({ 'Content-Type': 'application/json;' }); + expect(response.status).toBe(200); + expect(executed).toEqual(['hello']); + }); + + it('rejects joined duplicate Content-Type headers', async () => { + const response = await postToTransport({ 'Content-Type': 'application/json, application/json' }); + expect(response.status).toBe(415); + expect(executed).toEqual([]); + }); +}); diff --git a/test/shared/mediaType.test.ts b/test/shared/mediaType.test.ts new file mode 100644 index 0000000000..98d8f77109 --- /dev/null +++ b/test/shared/mediaType.test.ts @@ -0,0 +1,66 @@ +import { isJsonContentType, mediaTypeEssence } from '../../src/shared/mediaType.js'; + +describe('mediaTypeEssence', () => { + it('parses well-formed headers', () => { + expect(mediaTypeEssence('application/json')).toBe('application/json'); + expect(mediaTypeEssence('text/event-stream; charset=utf-8')).toBe('text/event-stream'); + expect(mediaTypeEssence('Application/JSON; charset=utf-8')).toBe('application/json'); + }); + + it('falls back to the pre-parameter segment for malformed parameter sections', () => { + expect(mediaTypeEssence('application/json;')).toBe('application/json'); + expect(mediaTypeEssence('application/json; charset=')).toBe('application/json'); + expect(mediaTypeEssence('text/plain;')).toBe('text/plain'); + }); + + it('returns undefined for missing or empty headers', () => { + expect(mediaTypeEssence(null)).toBeUndefined(); + expect(mediaTypeEssence(undefined)).toBeUndefined(); + expect(mediaTypeEssence('')).toBeUndefined(); + expect(mediaTypeEssence(' ')).toBeUndefined(); + expect(mediaTypeEssence(';charset=utf-8')).toBeUndefined(); + }); + + it('yields no essence for joined duplicate headers, with or without parameters', () => { + // Headers.get() joins repeated headers with ', '. Without parameters + // the comma lands in the first segment; with parameters it hides in + // the tail — both must behave the same. + expect(mediaTypeEssence('application/json, application/json')).toBe('application/json, application/json'); + expect(mediaTypeEssence('application/json; charset=utf-8, text/plain')).toBeUndefined(); + expect(mediaTypeEssence('application/json; charset=utf-8, application/json')).toBeUndefined(); + }); +}); + +describe('isJsonContentType', () => { + it('accepts application/json with or without parameters', () => { + expect(isJsonContentType('application/json')).toBe(true); + expect(isJsonContentType('application/json; charset=utf-8')).toBe(true); + expect(isJsonContentType('Application/JSON')).toBe(true); + expect(isJsonContentType(' application/json ; charset=utf-8')).toBe(true); + }); + + it('accepts unambiguous media types with malformed parameter sections', () => { + expect(isJsonContentType('application/json;')).toBe(true); + expect(isJsonContentType('application/json; charset=')).toBe(true); + expect(isJsonContentType('application/json; charset=utf-8; charset=x')).toBe(true); + }); + + it('never matches on substrings: parameters and sibling types are not application/json', () => { + expect(isJsonContentType('text/plain; a=application/json')).toBe(false); + expect(isJsonContentType('text/plain;')).toBe(false); + expect(isJsonContentType('text/plain, application/json')).toBe(false); + expect(isJsonContentType('application/json, application/json')).toBe(false); + expect(isJsonContentType('application/json; charset=utf-8, text/plain')).toBe(false); + expect(isJsonContentType('text/plain; charset=utf-8, application/json')).toBe(false); + expect(isJsonContentType('application/json-patch+json')).toBe(false); + expect(isJsonContentType('application/jsonp')).toBe(false); + }); + + it('rejects missing, empty, and non-JSON types', () => { + expect(isJsonContentType(null)).toBe(false); + expect(isJsonContentType(undefined)).toBe(false); + expect(isJsonContentType('')).toBe(false); + expect(isJsonContentType('text/plain')).toBe(false); + expect(isJsonContentType('multipart/form-data')).toBe(false); + }); +});