fix: don't fail a notification POST on response shape - #1063
Open
otherwhere666 wants to merge 1 commit into
Open
fix: don't fail a notification POST on response shape#1063otherwhere666 wants to merge 1 commit into
otherwhere666 wants to merge 1 commit into
Conversation
A POST that carries only notifications/responses/errors leaves no request outstanding, so nothing in the response is load-bearing. The spec says the server SHOULD reply 202 Accepted with no body, but servers in the wild routinely reply 200 instead, with or without a body, and with or without a Content-Type. `post_message` previously accepted such a POST only on 202/204, on a 2xx carrying a literal `Content-Length: 0`, or on a 2xx typed `application/json` or `text/event-stream`. Anything else returned `UnexpectedContentType`, which fails the send. When the message is `notifications/initialized` that takes the entire handshake down, against servers whose only fault is answering 200 instead of 202. Two shapes hit this in practice: - 200 with an unexpected or absent Content-Type. The TypeScript SDK gates its content-type validation behind `hasRequests`, so a notification-only POST skips the check entirely and any 2xx passes; we were stricter than the other client SDKs a server is likely to have been tested against. - An empty body sent with chunked transfer encoding. Content-Length is absent, so the existing empty-body tolerance cannot fire even though the body is in fact empty. Never fail a POST that awaits no response on response shape alone — only on a non-success status. `application/json` and `text/event-stream` are still parsed and streamed exactly as before, so no behavior is lost; only the previously fatal fallback arm changes. A POST that is still waiting on a reply keeps every existing strict check.
DaleSeo
reviewed
Jul 28, 2026
| // servers that answer 200 with an unexpected or absent Content-Type. | ||
| // This also covers an empty body sent with chunked transfer encoding, | ||
| // where Content-Length is absent and the check above cannot fire. | ||
| _ if awaits_no_response => { |
Member
There was a problem hiding this comment.
UnixSocketHttpClient::post_message_with_max_sse_event_size uses the same Content-Length and Content-Type decision tree, but its fallback still returns UnexpectedContentType for notification, response, and error POSTs. Could we apply the same awaits_no_response guard and fallback there, along with equivalent regression tests, so notifications/initialized stops failing for users of the Unix-socket transport?
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.
Problem
A POST that carries only notifications/responses/errors leaves no request outstanding, so nothing in the response is load-bearing. The spec says the server SHOULD reply
202 Acceptedwith no body — but servers in the wild routinely reply200instead, with or without a body, and with or without aContent-Type.post_messagecurrently accepts such a POST only on202/204, on a 2xx carrying a literalContent-Length: 0, or on a 2xx typedapplication/json/text/event-stream. Anything else returnsUnexpectedContentType, which fails the send. When the message isnotifications/initialized, that takes the whole handshake down against a server whose only fault is answering200instead of202.Two shapes hit this in practice:
Content-Type. The TypeScript SDK gates its content-type validation behindhasRequests, so a notification-only POST skips the check entirely and any 2xx passes. We are stricter than the other client SDKs a server is likely to have been tested against — a server can pass every other client and fail only here.Content-Lengthis absent, socontent_length == Some(0)cannot fire even though the body is in fact empty, and the request falls through to the fatal arm.We hit (1) against a stateless server that returned
200fornotifications/initialized. The same endpoint completed the handshake against five other MCP clients; ours was the only one that rejected it.Change
Never fail a POST that awaits no response on response shape alone — only on a non-success status.
application/jsonandtext/event-streamare still parsed and streamed exactly as before, so no behavior is lost; only the previously fatal fallback arm changes. A POST that is still waiting on a reply keeps every existing strict check.Tests
Added to the module's inline
mod tests:post_notification_accepts_unusable_success_body— tworstestcases,200+text/plainand200with theContent-Typestripped. Both fail onmainwithUnexpectedContentType(Some("text/plain"))andUnexpectedContentType(None)respectively; both pass with the change. The second also covers the chunked-empty-body shape, which takes the identical path (noContent-Length, noContent-Type).post_request_still_rejects_unexpected_content_type— guard against over-loosening: a POST still awaiting a reply must keep failing. Passes before and after.cargo clippy -p rmcp --all-features --all-targetsis clean andcargo fmt --checkpasses. Full suite per thejust testrecipe: 949 passed, 0 failed. (test_with_pythonfails identically on pristinemainin my environment becauseuvisn't installed — unrelated.)