Validate Content-Type by parsed media type instead of substring match (v1.x)#2444
Conversation
The Streamable HTTP server gate and the client response dispatch matched Content-Type with a substring search, which accepts values whose media type is different (e.g. `text/plain; a=application/json`) and rejects valid case variants like `Application/JSON`. Parse the header with the content-type package instead, falling back to the segment before the first `;` when the parameter section is malformed, so unambiguous values such as `application/json;` keep working. The 415/-32000 error shape and message bytes are unchanged. This mirrors the equivalent change on main.
🦋 Changeset detectedLatest commit: 7ecf52e The changes in this PR will be included in the next version bump. 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 |
commit: |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
I didn't find any bugs — the media-type parsing helper, transport changes, and test matrix all look correct — but this changes runtime validation behavior in the core Streamable HTTP transport (server 415 handling and client response dispatch), so it's worth a human maintainer's look for interop implications.
Extended reasoning...
Overview
This PR (a v1.x backport of #2441) replaces substring-based Content-Type checks with parsed media-type comparison. It adds a new shared helper (src/shared/mediaType.ts) built on the existing content-type dependency with a fallback for malformed parameter sections, applies it to POST Content-Type validation in WebStandardStreamableHTTPServerTransport and to response dispatch in the Streamable HTTP client, adds an ESLint no-restricted-syntax rule to prevent regressions, and includes a thorough unit/transport test matrix plus a changeset.
Security risks
The change is strictly a tightening of input validation: values like text/plain; a=application/json are now correctly rejected with 415 instead of being dispatched. No injection, auth, or data-exposure surface is introduced. The fallback parser for malformed parameter sections is conservative (returns no essence for joined duplicate headers), and I verified the only remaining substring check in src/ (the Accept header list check) is intentionally exempted with a lint disable.
Level of scrutiny
The logic and tests look correct, but this touches the core Streamable HTTP transport on both the server (request rejection path) and the client (response content-type dispatch). The client-side change means responses whose Content-Type merely contained the substring but is not exactly application/json or text/event-stream as a media type will now be treated as unexpected — stricter behavior that could in principle affect interop with non-conforming servers on the stable v1.x line. That behavior/compatibility judgment on a widely-used code path is one a maintainer should confirm, so I'm not shadow-approving.
Other factors
Test coverage is strong: a new helper unit matrix and transport-level tests pinning 415-without-dispatch for non-JSON media types, acceptance of parameters/case variants/malformed parameter sections, and rejection of joined duplicate headers. The PR mirrors an already-accepted change on main, which reduces design risk, but the backport target (v1.x patch release) raises the bar for behavioral changes.
2 participants
Backport of #2441 to the v1.x line.
Content-Type validation in the web-standard Streamable HTTP server transport matched the raw header as a substring (
ct.includes('application/json')), which misclassifies values whose media type is notapplication/json(for exampletext/plain; a=application/json) and wrongly rejects valid case variants. The client's response dispatch used the same pattern.Motivation and Context
Content-Type is a media type plus optional parameters (RFC 9110); comparisons must use the parsed media type. This mirrors the change on main: parse via the
content-typepackage (already a dependency, used correctly by the SSE transport), with a media-type fallback for malformed parameter sections so unambiguous values likeapplication/json;keep working. The Node transport wraps the web-standard transport, so one change covers both.How Has This Been Tested?
application/jsonare served; absent and joined duplicate headers answer 415.Breaking Changes
Stricter for non-conforming clients only; the 415 response body and error code bytes are unchanged. One leniency gain: case variants are now accepted per RFC 9110.
Types of changes
Checklist