Skip to content

fix(server): validate Content-Type by parsed media type instead of substring match#2441

Merged
felixweinberger merged 7 commits into
mainfrom
fweinberger/content-type-media-type-validation
Jul 6, 2026
Merged

fix(server): validate Content-Type by parsed media type instead of substring match#2441
felixweinberger merged 7 commits into
mainfrom
fweinberger/content-type-media-type-validation

Conversation

@felixweinberger

Copy link
Copy Markdown
Contributor

Content-Type validation in the Streamable HTTP server transport, the createMcpHandler entry, and the hono adapter's JSON pre-parse matched the raw header as a substring (ct.includes('application/json')). That misclassifies values whose media type is not application/json (for example text/plain; a=application/json), wrongly rejects valid case variants like Application/JSON, and the 2026-07-28 entry performed no Content-Type validation at all.

Motivation and Context

Content-Type is a media type plus optional parameters (RFC 9110); comparisons must use the parsed media type. This parses the header (via the content-type package the legacy SSE transport already used, with a media-type fallback for malformed parameter sections so unambiguous values like application/json; keep working) and compares the media type itself at every entry. A new isJsonContentType helper is exported for transport and framework-adapter authors, the client's response dispatch uses the same comparison, and an ESLint rule bans the substring pattern from reappearing.

How Has This Been Tested?

  • New tests pin the behavior on both eras and the direct-transport wiring: non-JSON media types answer 415 without dispatch; parameters, case variants, and malformed-parameter values whose media type is application/json are served; absent and duplicated headers answer 415.
  • Full test suites, typecheck, lint, and the conformance suites (2025 + 2026 eras) pass unchanged.

Breaking Changes

Stricter for non-conforming clients only: POSTs whose Content-Type media type is not application/json now answer 415 on every entry (previously any value containing the substring passed, and the 2026-07-28 entry did not check at all — see the migration guide note). SDK clients are unaffected. One leniency gain: case variants are now accepted per RFC 9110.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

The Content-Type checks in the Streamable HTTP server transport and the
hono adapter matched the raw header as a substring, accepting values
whose media type is not application/json (e.g.
'text/plain; a=application/json') while rejecting valid case variants;
the 2026-07-28 entry did not validate Content-Type at all. Parse the
header (content-type package, with a media-type fallback for malformed
parameter sections) and compare the media type at every entry. Export
isJsonContentType for adapter authors, migrate the client's response
dispatch to the same comparison, and add an ESLint rule banning the
substring pattern.
@felixweinberger felixweinberger requested a review from a team as a code owner July 6, 2026 17:57
@changeset-bot

changeset-bot Bot commented Jul 6, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: b2546a1

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages
Name Type
@modelcontextprotocol/server Patch
@modelcontextprotocol/client Patch
@modelcontextprotocol/hono Patch
@modelcontextprotocol/node Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@pkg-pr-new

pkg-pr-new Bot commented Jul 6, 2026

Copy link
Copy Markdown

Open in StackBlitz

@modelcontextprotocol/client

npm i https://pkg.pr.new/@modelcontextprotocol/client@2441

@modelcontextprotocol/codemod

npm i https://pkg.pr.new/@modelcontextprotocol/codemod@2441

@modelcontextprotocol/core

npm i https://pkg.pr.new/@modelcontextprotocol/core@2441

@modelcontextprotocol/server

npm i https://pkg.pr.new/@modelcontextprotocol/server@2441

@modelcontextprotocol/server-legacy

npm i https://pkg.pr.new/@modelcontextprotocol/server-legacy@2441

@modelcontextprotocol/express

npm i https://pkg.pr.new/@modelcontextprotocol/express@2441

@modelcontextprotocol/fastify

npm i https://pkg.pr.new/@modelcontextprotocol/fastify@2441

@modelcontextprotocol/hono

npm i https://pkg.pr.new/@modelcontextprotocol/hono@2441

@modelcontextprotocol/node

npm i https://pkg.pr.new/@modelcontextprotocol/node@2441

commit: b2546a1

* 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 {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment thread packages/core-internal/src/shared/mediaType.ts
felixweinberger and others added 2 commits July 6, 2026 18:31
The essence fallback took everything before the first ';', so a joined
duplicate header was rejected when the first copy had no parameters but
accepted when it did. Bail out of the fallback when the parameter tail
of an unparseable value contains a comma, and cite the WHATWG essence
definition in the module docs.

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The joined-duplicate-header asymmetry from the earlier inline comment is now handled in mediaTypeEssence() and pinned by tests, and I didn't find further issues — but since this adds a new public export (isJsonContentType), a new core-internal runtime dependency, and stricter 415 behavior on the 2026-07-28 entry, it deserves a maintainer's judgment call rather than an automated approval.

Extended reasoning...

Overview

The PR replaces substring-matching of application/json with parsed-media-type comparison across the server transport (WebStandardStreamableHTTPServerTransport), createMcpHandler (which previously performed no Content-Type check on the modern leg), the hono adapter's JSON pre-parse, and the client's response dispatch. It introduces packages/core-internal/src/shared/mediaType.ts (mediaTypeEssence / isJsonContentType), exports isJsonContentType on the public surface, adds the content-type package as a core-internal runtime dependency (moved to the runtimeShared catalog), adds an ESLint no-restricted-syntax rule banning the substring pattern, and includes migration-guide and changeset prose plus new tests for both eras and the direct transport wiring.

Security risks

Low. The change is strictly tightening: non-JSON media types (including values that merely contain the substring) now answer 415 before dispatch, and the earlier concern — that the malformed-parameter fallback could accept a joined duplicate header whose first copy carried parameters — is resolved: the fallback now returns undefined when a comma appears in the parameter tail, with tests pinning both duplicate-header shapes. The Accept-header substring check is intentionally retained (comma-separated list semantics) with an explicit eslint-disable and comment. No auth, crypto, or data-exposure surface is touched.

Level of scrutiny

This is production-critical HTTP entry code and a behavior change with mild breaking potential: hand-rolled clients that omitted or mis-set Content-Type on the 2026-07-28 entry now get 415 where they previously got served. The repo's conventions place the burden of proof on new public exports (isJsonContentType is now public API, justified by the hono adapter and example callsites in the same PR) and on new dependencies in the runtime-neutral core package — both are reasonable here (content-type is tiny and dependency-free, and the legacy SSE transport already used it), but they are exactly the kind of API-surface and posture decisions a maintainer should sign off on rather than a bot.

Other factors

Test coverage is thorough: the new mediaType.test.ts and contentTypeValidation.test.ts pin the substring-misclassification shape, parameter/case variants, malformed parameter sections, missing headers, joined duplicates, and both legacy and modern legs including the direct-transport wiring. The changeset and migration-guide prose match the implemented behavior, including the note that custom compositions of classifyInboundRequest / PerRequestHTTPServerTransport must apply the check themselves. The prior automated inline finding has been addressed in the current revision, so no outstanding review comments remain from this system.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants