Conversation
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:
Browser-based MCP clients (origin different from the server's origin) cannot use StreamableHttpServerWrapper reliably today. Three distinct CORS issues cause preflights and actual requests to be blocked by the browser, and cause the JavaScript client to be unable to read the Mcp-Session-Id response header even when it is physically present on the wire.
This PR fixes all three in src/server/streamable_http_server.cpp.
Problems:
GET /mcp returns a 405 without CORS headers:
The Get handler constructs a 405 "Method Not Allowed" response but never calls apply_additional_response_headers(res). Any cross-origin browser request to GET /mcp (which most MCP JS SDKs attempt as part of the Streamable HTTP transport handshake, since the spec allows GET for server-initiated SSE streams) therefore fails with:
Access to fetch at http://localhost:.../mcp from origin http://localhost:... has been blocked by CORS policy: No Access-Control-Allow-Origin header is present on the requested resource.
DELETE /mcp is not handled at all:
The MCP Streamable HTTP spec (2025-03-26) allows clients to send DELETE /mcp to terminate a session. fastmcpp does not register a Delete handler, so httplib falls back to its default 404 response, which has no configured CORS headers and triggers the same browser error.
Additionally, the Options preflight handler advertises only POST, OPTIONS in Access-Control-Allow-Methods. Browsers therefore reject the DELETE preflight preemptively, before ever hitting the (currently non-existent) DELETE handler.
Mcp-Session-Id response header is not exposed to JS:
The Post handler attaches Mcp-Session-Id on the initialize response, but does not set Access-Control-Expose-Headers. Per CORS, browsers only expose a small whitelist of "safe" response headers to cross-origin JS (Cache-Control, Content-Language, Content-Length, Content-Type, Expires, Last-Modified, Pragma). Mcp-Session-Id is not in that whitelist, so response.headers.get("Mcp-Session-Id") in browser JS returns null even though curl -v clearly shows the header on the wire. The client then cannot store or echo the session id, and every subsequent non-initialize request fails with HTTP 400 Mcp-Session-Id header required.
Several early-return paths in the POST handler ran before apply_additional_response_headers(res):
The 401 Unauthorized response was emitted before apply_additional_response_headers(res) was called, so auth failures lacked CORS headers. In addition, none of the catch handlers re-applied the headers — this happens to work because httplib preserves headers already set on res across the try/catch boundary, but relying on that is brittle and obscures intent.
Changes:
POST handler: moved apply_additional_response_headers(res) to the very top of the lambda, before the auth check and outside the try block, so every response path (success, 401, 503, 400, 404, 5xx via catch) carries the configured CORS / custom headers unconditionally.
POST handler: added res.set_header("Access-Control-Expose-Headers", "Mcp-Session-Id") so browser JS can actually read the session id via response.headers.get("Mcp-Session-Id").
GET handler: added apply_additional_response_headers(res) at the top so the 405 response carries CORS. Updated the lambda capture from [] to [this] accordingly.
OPTIONS preflight: broadened Access-Control-Allow-Methods from POST, OPTIONS to GET, POST, DELETE, OPTIONS so browsers accept preflights for all methods the server now supports.
DELETE handler: registered. Best-effort erases the session from sessions_ when a valid Mcp-Session-Id header is present, and responds 204 No Content with the configured CORS headers attached.
No public API changes. No new dependencies. SseServerWrapper is not modified (the same "apply headers mid-handler" anti-pattern exists there, but fixing it is out of scope for this PR).
Reproduction:
Before the patch, starting a StreamableHttpServerWrapper with cors_origin = "*":
After the patch all four responses carry the appropriate CORS headers and the POST response carries Access-Control-Expose-Headers: Mcp-Session-Id.