Skip to content

Allow more accept headers #429

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 20, 2025
Merged

Conversation

halter73
Copy link
Contributor

@halter73 halter73 commented May 17, 2025

The StreamableHttpHandler was a bit pickier about accept headers than I think it should be. While the spec does say the following:

The client MUST include an Accept header, listing both application/json and text/event-stream as supported content types.

https://modelcontextprotocol.io/specification/2025-03-26/basic/transports#sending-messages-to-the-server

I don't think it's necessary to the server to be so strict enforcing this. If a client wants to say it accepts "*/*", it can deal with the fallout if it really cannot handle a JSON or SSE response. The most visible impact of this will probably be the response you see if you enter a Streamable HTTP endpoint URL directly into your browser's address bar since it does typically accept "*/*". Now, instead of seeing a 406 response with the following body:

{
  "error": {
    "code": -32000,
    "message": "Not Acceptable: Client must accept text/event-stream"
  },
  "id": "",
  "jsonrpc": "2.0"
}

You'll see a 404 response with this body instead:

{
  "error": {
    "code": -32001,
    "message": "Session not found"
  },
  "id": "",
  "jsonrpc": "2.0"
}

I think both of these are about just as useful to someone hitting a Streamable HTTP endpoint URL directly with the browser in conveying that something is responding to the endpoint, but a plain GET request from the browser without the right headers is unsupported.

We could decide to still reject "*/*" considering the spec says clients MUST list both application/json and text/event-stream, but it would take a little more code. By leveraging the MatchesMediaType() method, we support accept headers like "*/*" and headers with extra parameters like the "text/event-stream, application/json;q=0.9" from the test. Whatever we decide for "*/*" we should ignore these extra parameters.

@halter73 halter73 merged commit d23f442 into modelcontextprotocol:main May 20, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants