Skip to content

fix: don't fail a notification POST on response shape - #1063

Closed
otherwhere666 wants to merge 1 commit into
modelcontextprotocol:mainfrom
AugureAI:fix/notification-post-response-shape
Closed

otherwhere666 wants to merge 1 commit into
modelcontextprotocol:mainfrom
AugureAI:fix/notification-post-response-shape

Conversation

@otherwhere666

Copy link
Copy Markdown

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 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 currently accepts such a POST only on 202/204, on a 2xx carrying a literal Content-Length: 0, or on a 2xx typed application/json / text/event-stream. Anything else returns UnexpectedContentType, which fails the send. When the message is notifications/initialized, that takes the whole handshake down against a server whose only fault is answering 200 instead of 202.

Two shapes hit this in practice:

  1. 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 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.
  2. An empty body sent with chunked transfer encoding. Content-Length is absent, so content_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 200 for notifications/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/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.

Tests

Added to the module's inline mod tests:

  • post_notification_accepts_unusable_success_body — two rstest cases, 200 + text/plain and 200 with the Content-Type stripped. Both fail on main with UnexpectedContentType(Some("text/plain")) and UnexpectedContentType(None) respectively; both pass with the change. The second also covers the chunked-empty-body shape, which takes the identical path (no Content-Length, no Content-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-targets is clean and cargo fmt --check passes. Full suite per the just test recipe: 949 passed, 0 failed. (test_with_python fails identically on pristine main in my environment because uv isn't installed — unrelated.)

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.
@otherwhere666
otherwhere666 requested a review from a team as a code owner July 27, 2026 22:03
@github-actions github-actions Bot added T-core Core library changes T-transport Transport layer changes labels Jul 27, 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 => {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

@github-actions

Copy link
Copy Markdown
Contributor

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 keep label to keep it open.

@github-actions github-actions Bot added the Stale label Sep 27, 2026
@github-actions

Copy link
Copy Markdown
Contributor

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.

@github-actions github-actions Bot closed this Oct 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Stale T-core Core library changes T-transport Transport layer changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants