Return 405 Method Not Allowed for non-GET HTTP requests #1684
Conversation
Instead of raising an exception when receiving HEAD, POST, or other non-GET HTTP methods, the server now properly returns a 405 Method Not Allowed response with an Allow: GET header. This change: - Adds InvalidMethod exception for unsupported HTTP methods - Modifies Request.parse() to raise InvalidMethod instead of ValueError - Handles InvalidMethod in ServerProtocol.parse() to return 405 response - Updates tests accordingly Fixes python-websockets#1677
aaugustin
left a comment
aaugustin
left a comment
There was a problem hiding this comment.
See last comment for why I stopped reviewing.
src/websockets/exceptions.py
Outdated
| * :exc:`ProxyError` | ||
| * :exc:`InvalidProxyMessage` | ||
| * :exc:`InvalidProxyStatus` | ||
| * :exc:`InvalidMethod` |
There was a problem hiding this comment.
Should be after InvalidMessage -- you can only tell which method is used after you're successfully parsed the message.
src/websockets/exceptions.py
Outdated
| "ProxyError", | ||
| "InvalidProxyMessage", | ||
| "InvalidProxyStatus", | ||
| "InvalidMethod", |
| def __str__(self) -> str: | ||
| return f"invalid HTTP method; expected GET; got {self.method}" | ||
|
|
||
|
|
| self.parser = self.discard() | ||
| next(self.parser) # start coroutine | ||
| yield | ||
| return |
There was a problem hiding this comment.
Here you're calling reject(), a high level function, from the parser, a low level component.
This doesn't make sense from a layering perspective. Can you explain why you're done that?
I suspect the honest answer is "you just threw AI at the issue and didn't bothing reviewing" :-(
I can throw AI at an issue myself, thank you. Stopping the review there until I hear back.
- Move InvalidMethod exception after InvalidMessage in hierarchy, docstrings, and __all__ (since method is known only after parsing) - Build 405 response directly in parse() without calling reject() to maintain proper layering between low-level parser and high-level API - Remove redundant discard() setup (send_response already handles it) - Add InvalidMethod to Request.parse() docstring Signed-off-by: majiayu000 <1835304752@qq.com>
|
Thanks for the feedback. You're right about the layering issue - I've fixed it:
I apologize for the initial oversight. The code has been updated. |
2 participants
Summary
Changes
InvalidMethodexception towebsockets.exceptionsRequest.parse()inhttp11.pyto raiseInvalidMethodinstead ofValueErrorfor unsupported methodsServerProtocol.parse()inserver.pyto catchInvalidMethodand return a proper 405 response withAllow: GETheaderTest plan
InvalidMethodinstead ofValueErrorFixes #1677