fix(client): send POST bodies with Content-Length instead of chunked encoding - #1877
Open
ump45nose wants to merge 1 commit into
Open
fix(client): send POST bodies with Content-Length instead of chunked encoding#1877ump45nose wants to merge 1 commit into
ump45nose wants to merge 1 commit into
Conversation
…encoding On modern .NET, `McpHttpClient.CreatePostBodyContent` built the request body with `JsonContent.Create(...)`. `JsonContent` serializes lazily and cannot report a length, so `HttpClient` had no `Content-Length` to send and fell back to `Transfer-Encoding: chunked`. Hosts that reject chunked request bodies then fail the POST: the local Azure Functions Python worker answers with `400 "'Transfer-Encoding: chunked' header can not be used when content object is not specified"`, so an MCP server that works when deployed does not work when run locally. The netstandard2.0 branch already buffered the payload into a `ByteArrayContent`, which does set `Content-Length`, so the wire format also depended on the target framework. Use the buffered path on every target framework: serialize to UTF-8 bytes and wrap them in a `ByteArrayContent`. That removes the TFM split and makes the encoding uniform, at the cost of holding one serialized message in memory, which the netstandard path already did. Add a regression test that drives a real Streamable HTTP POST through `HttpClientTransport` and asserts, from the message handler, that the request declares a `Content-Length` matching the bytes actually sent and is not sent with chunked encoding. The values are captured in the handler because `McpHttpClient` disposes the content once `SendAsync` returns. Fixes modelcontextprotocol#932
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Buffer the Streamable HTTP POST body so it goes out with a
Content-Lengthinstead ofTransfer-Encoding: chunked.McpHttpClient.CreatePostBodyContentbuilt the body withJsonContent.Create(...)on modern .NET.JsonContentserializes lazily and cannot report a length, soHttpClienthad no length to send and fell back to chunked encoding. Hosts that reject chunked request bodies then fail the POST — the local Azure Functions Python worker answers400 "'Transfer-Encoding: chunked' header can not be used when content object is not specified"— which is the failure tracked in #816 and consolidated as #932.The netstandard2.0 branch already buffered the payload into a
ByteArrayContent, which does setContent-Length, so the wire format also depended on the target framework. This PR uses the buffered path on every target framework, which removes the TFM split entirely.Changes
src/ModelContextProtocol.Core/Client/McpHttpClient.cs— drop the#if NET/#elsesplit; serialize withJsonSerializer.SerializeToUtf8Bytesand wrap the bytes in aByteArrayContentfor all target frameworks. NoContent-Typechange: it is still the bareapplication/jsonvalue froms_applicationJsonContentType.tests/ModelContextProtocol.Tests/Transport/HttpClientTransportTests.cs— regression test that drives a real Streamable HTTP POST throughHttpClientTransportand asserts the request declares aContent-Lengthmatching the bytes actually sent, and is not sent with chunked encoding.Tradeoff: one serialized message is held in memory per request instead of being streamed. The netstandard2.0 path already paid that cost, and request bodies here are single JSON-RPC messages.
Testing
Build — all target frameworks, warnings as errors:
New test, red on unfixed source and green with the fix. With
McpHttpClient.csreverted toupstream/mainbut the test kept:With the fix:
Observed request properties from the message handler, before and after:
Content.Headers.ContentLengthupstream/mainnull(→ chunked on the wire)JsonContent54ByteArrayContentThe values are captured inside the handler because
McpHttpClient.SendAsyncdisposes the content once it returns; reading them afterwards throwsObjectDisposedException.Full test project on net10.0:
The 5 skips are the pre-existing Docker/external-service tests.
Other target frameworks (environment limit, please note). Only the .NET 10 runtime is installed here, so
dotnet testcannot launch the net8.0/net9.0 test hosts. I ran those TFMs with the xUnit v3 in-process runner instead and both pass 17/17 forHttpClientTransportTests:The net9.0 run executed on the .NET 10 runtime rather than a real .NET 9 runtime, so treat genuine net9.0 execution as covered by CI.
netstandard2.0compiles clean in the build above but has no test host.Not verified: no end-to-end run against a locally hosted Azure Functions MCP server; the wire-format property is asserted at the request level instead.
Notes on the issue state
#932 still carries
needs confirmation. The change follows the root cause and the preferred default described in the maintainer comment on the issue — send aContent-Lengthbody by default rather than making it configurable. If the outcome should instead be opt-in via aHttpClientTransportOptionsflag, say so and I will rework it.Fixes #932
Note
This pull request description was AI-generated.