-
Notifications
You must be signed in to change notification settings - Fork 338
mcp: add Allow header to 405 responses per RFC 9110 §15.5.6 #757
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
mcp: add Allow header to 405 responses per RFC 9110 §15.5.6 #757
Conversation
1e80e30 to
566bb29
Compare
405 Method Not Allowed responses MUST include an Allow header listing supported methods, per RFC 9110 Section 15.5.6. This fixes issues with strict HTTP gateways (like Apigee) that treat 405 responses without an Allow header as malformed, returning 502 Bad Gateway errors. Changes: - SSEHandler: Add Allow header (GET, POST) for unsupported methods - StreamableHTTPHandler: Add Allow header for GET-without-session case, differentiating between stateless mode (POST, DELETE) and stateful mode (GET, POST, DELETE) - Add tests to verify Allow header presence in all 405 responses Co-authored-by: omgitsads <4619+omgitsads@users.noreply.github.com>
566bb29 to
ac627b0
Compare
|
Pushed amended commit with gofmt applied properly. |
- Remove unused withSession and wantStatus fields from streamable test - Use got, want pattern for status code checks to avoid repetition
|
Thanks for the review @maciej-kisiel, I agree with your comments. |
maciej-kisiel
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for quick action. I thought about this behavior a bit deeper and have two more questions. Sorry for not including them in my initial review.
|
@maciej-kisiel they are great questions. I was thinking about some of them too when writing. I also suggested in MCP discord the idea of adding some HTTP conformance tests to the SDK conformance testing, and so I think as well as trying to get this correct first time, I think we will end up with http conformance being checked and consistent across SDKs with community decision on some of these things, I hope this PR will be the guide for some that work, but I also think that there will likely be follow-ups. |
Per MCP spec and reviewer feedback: - In stateful mode, GET without session now returns 400 Bad Request (like DELETE), not 405 - because GET IS a supported method, it just requires a session ID as a precondition. - In stateless mode, GET correctly returns 405 Method Not Allowed since the server doesn't offer SSE streaming. - Unsupported methods (PUT, PATCH, etc.) now return the correct Allow header based on mode: - Stateless: Allow: POST - Stateful: Allow: GET, POST, DELETE This aligns with MCP spec section 'Listening for Messages from the Server': 'The server MUST either return Content-Type: text/event-stream in response to this HTTP GET, or else return HTTP 405 Method Not Allowed, indicating that the server does not offer an SSE stream at this endpoint.'
|
Thanks for the review @maciej-kisiel, I agree with your comments. After researching the MCP spec, I've made the following changes: GET without session in stateful mode → 400 Bad Request
Allow header in stateless mode →
Summary of behavior now:
|
maciej-kisiel
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
LGTM! Thanks @SamMorrowDrums for the contribution, and @maciej-kisiel for the review! |
Hey @findleyr we have an issue where a customer is using Gateways and technically the SDK throws non-compliant method not allowed headers, when they try to inspect server with a HEAD request. It's fine that it fails, but it highlights that it doesn't specify the required allowed methods, and so I think this is a change that should be made. Let me know 🙏
Summary
405 Method Not Allowed responses MUST include an Allow header listing supported methods, per RFC 9110 Section 15.5.6. This fixes issues with strict HTTP gateways (like Apigee) that treat 405 responses without an Allow header as malformed, returning 502 Bad Gateway errors.
Changes
Allow: GET, POSTheader for unsupported methodsAllow: POST, DELETE(GET is never valid)Allow: GET, POST, DELETE(GET is valid once you have a session)RFC 9110 Reference
Section 15.5.6 (405 Method Not Allowed):
Testing
All existing tests pass, plus new tests added:
TestSSE405AllowHeader- verifies SSE handler complianceTestStreamable405AllowHeader- verifies Streamable handler compliance in both stateful and stateless modes