Validate Content-Type by parsed media type instead of substring match (v1.x) (#2444)
This commit is contained in:
parent
369513df7b
commit
69749aa508
7 changed files with 271 additions and 4 deletions
6
.changeset/content-type-media-type-validation.md
Normal file
6
.changeset/content-type-media-type-validation.md
Normal file
|
|
@ -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.
|
||||
|
|
@ -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'],
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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');
|
||||
}
|
||||
|
|
|
|||
57
src/shared/mediaType.ts
Normal file
57
src/shared/mediaType.ts
Normal file
|
|
@ -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';
|
||||
}
|
||||
115
test/server/contentTypeValidation.test.ts
Normal file
115
test/server/contentTypeValidation.test.ts
Normal file
|
|
@ -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<CallToolResult> => {
|
||||
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<string, string>): 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<Response> {
|
||||
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<string, string>): Promise<Response> {
|
||||
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([]);
|
||||
});
|
||||
});
|
||||
66
test/shared/mediaType.test.ts
Normal file
66
test/shared/mediaType.test.ts
Normal file
|
|
@ -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);
|
||||
});
|
||||
});
|
||||
Loading…
Reference in a new issue