Repository navigation
fix: don't fail a notification POST on response shape - #1063
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.
| // 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 => { |
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 pull request has had no activity for 60 days and is now marked as stale. It will be closed in 14 days if there is no further activity. Add the |
|
Closing this pull request after 14 days of inactivity since it was marked stale. Feel free to reopen if you plan to continue the work. |
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.)