Skip to content

[v1.x] fix(client): merge requestInit headers case-insensitively - #3105

Open
Davien21 wants to merge 3 commits into
modelcontextprotocol:v1.xfrom
Davien21:fix-header-merge-case
Open

Davien21 wants to merge 3 commits into
modelcontextprotocol:v1.xfrom
Davien21:fix-header-merge-case

Conversation

@Davien21

Copy link
Copy Markdown

Fixes #2969

The 400 comes from the client header merge, not the server. _commonHeaders() in src/client/streamableHttp.ts and src/client/sse.ts spreads the transport headers and requestInit.headers into one plain object, so mcp-protocol-version and a caller's MCP-Protocol-Version both survive and the Headers constructor joins them.

Before: with requestInit.headers: { 'MCP-Protocol-Version': '2025-03-26' } and 2025-11-25 negotiated, the client sends mcp-protocol-version: 2025-11-25, 2025-03-26 and the server answers 400.
After: it sends 2025-03-26, the same as a lowercase mcp-protocol-version already did.

The fix builds a Headers from the transport values and set()s the caller's headers on top, so names compare case-insensitively. I kept the v1.x precedence where the caller's value wins. main (#2475) puts transport headers last instead, but that also changes which Authorization gets sent, which seemed too much for a v1.x patch. With this in, #2975's server-side change isn't needed for SDK clients.

Tested: a new e2e requirement, typescript:client-transport:http:custom-header-case, connects a real client to the SDK server with a mixed-case MCP-Protocol-Version in requestInit. On v1.x it fails with the same 400 as the issue; with the change it passes. The SSE client gets a unit test for the same merge. npm test (1859), npm run test:e2e (1136), npm run typecheck, npm run lint and npm run build pass.

_commonHeaders() in the Streamable HTTP and SSE clients spread the
transport headers and requestInit.headers into one plain object. Keys
that differed only in case (mcp-protocol-version vs
MCP-Protocol-Version) both survived, and the Headers constructor joined
them into "2025-11-25, 2025-03-26", which the server rejects with 400.

Build a Headers from the transport values and set() the caller's
headers on top. Header names now compare case-insensitively, and the
caller's value still wins, as it already did with a lowercase name.

Fixes modelcontextprotocol#2969
@Davien21
Davien21 requested a review from a team as a code owner October 26, 2026 14:04
@changeset-bot

changeset-bot Bot commented Oct 26, 2026 •

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 7009366

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

@pkg-pr-new

pkg-pr-new Bot commented Oct 26, 2026 •

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/@modelcontextprotocol/sdk@3105

commit: 7009366

@claude claude Bot added the v1 Issues / PRs related to v1.x label Oct 27, 2026
@arhancanli

Copy link
Copy Markdown

Checked the merge in plain Node (Headers semantics, same as the runtime this runs in): new Headers(transportHeaders) then new Headers(requestInit.headers).forEach(set) fixes the MCP-Protocol-Version vs mcp-protocol-version case as described.

One behaviour difference I could reproduce: when requestInit.headers is an array of tuples with the same name twice, the old normalizeHeaders went through Object.fromEntries, so the last pair won. The Headers constructor appends instead:

new Headers([['x-a','1'],['X-A','2']])            // x-a: '1, 2'
const m = new Headers({'mcp-protocol-version':'2025-11-25'});
new Headers([['MCP-Protocol-Version','a'],['mcp-protocol-version','b']]).forEach((v,k)=>m.set(k,v));
// mcp-protocol-version: 'a, b'

So a caller who passes duplicates in tuple form now sends a comma-joined value, which for mcp-protocol-version is the same 400 this PR fixes. That input is unusual, so it may be fine to leave; if you want to keep last-wins, build the caller side with normalizeHeaders first and then set each entry onto the Headers. A test with a tuple-array requestInit.headers would pin whichever behaviour you choose.

@Davien21

Davien21 commented Oct 29, 2026 •

Copy link
Copy Markdown
Author

Good catch @arhancanli, thank you.
Both transports now pass requestInit.headers through normalizeHeaders before setting each entry on the merged Headers, so a name repeated in tuple form keeps last-wins like before, and a case-only duplicate of a transport header still replaces it. Added a tuple-array test for SSE and Streamable HTTP that asserts the last value wins (0ca1f0b).

@arhancanli

Copy link
Copy Markdown

Read 0ca1f0b: routing both through normalizeHeaders and then merged.set per entry restores last-wins for tuple arrays, and since Headers.set is case-insensitive the [[MCP-Protocol-Version, a], [mcp-protocol-version, b]] case ends on b, which is what the new test asserts in both transports. That also covers a plain object with case-only duplicates ({A: 1, a: 2}), because Object.entries keeps insertion order.

One thing I could not tell from the diff: does normalizeHeaders still copy a Headers instance by iterating it (so values already comma-joined by the caller stay as one string), or does it read it differently? If it is the former, a test with a Headers input holding a repeated name would pin that path down too.

@Davien21

Copy link
Copy Markdown
Author

@arhancanli Yes, the former: normalizeHeaders copies a Headers instance with Object.fromEntries(headers.entries()), so a name the caller appended twice arrives as the single comma-joined value Headers already holds, same as before this PR. Added a test for that in both transports (7009366).

@arhancanli

Copy link
Copy Markdown

Read 7009366: both tests append X-Trace twice with different casing and assert get('x-trace') is a, b, so the Headers-instance path is pinned in SSE and Streamable HTTP. That matches the runtime semantics (Headers joins on append with ", " and entries() yields the joined string), so nothing further from my side on the header merge.

This branch has not been deployed

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

Labels

v1 Issues / PRs related to v1.x

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants