@sdogruyol
This is something I found while testing. This isn’t intentional, is it? If so, I’d like to restrict it to GET to match the RFC-6455. (It would be good to report this to Crystal as well, since its stdlib also has the same behavior, so I’m thinking of opening an issue there)
Description
Kemal::WebSocketHandler currently gates the WebSocket upgrade solely on ws_route_found? and the presence of Upgrade / Connection headers. The HTTP method is never validated.
RFC 6455 §4.1 requires the opening handshake to be a GET request. As a result, any method (POST, QUERY, etc.) that includes the correct headers will successfully complete the handshake:
curl -si -X QUERY localhost:3000/chat \
-H 'Upgrade: websocket' \
-H 'Connection: Upgrade' \
-H 'Sec-WebSocket-Key: dGhlIHNhbXBsZSBub25jZQ==' \
-H 'Sec-WebSocket-Version: 13' \
-H 'Origin: http://localhost:3000'
# HTTP/1.1 101 Switching ProtocolsNote: Crystal’s standard library HTTP::WebSocketHandler exhibits the same behavior (websocket_upgrade_request? only inspects headers), so an upstream report may also be worthwhile.
Expected behavior
Non-GET upgrade requests should not be upgraded. They should either fall through to the next handler or be rejected with 405 Method Not Allowed. This matches the behavior of the broader ecosystem:
gorilla/websocket and Node ws → 405python-websockets → 405 + Allow: GETfaye/websocket-driver (Sinatra/Rails) → handshake failure- Cowboy → only upgrades HTTP/1.1 GET (RFC 8441 CONNECT is HTTP/2+ only)
Suggested fix
In WebSocketHandler#call, reject the request unless context.request.method == "GET" before attempting the upgrade. This is a one-line change plus a corresponding spec.
Impact
Direct impact is low because the Origin check still applies. However, it is a clear protocol violation and can confuse intermediaries. The issue was discovered while dogfooding HTTP QUERY support (#769), where a QUERY route and a WebSocket route share the same path.
@sdogruyol
This is something I found while testing. This isn’t intentional, is it? If so, I’d like to restrict it to GET to match the RFC-6455. (It would be good to report this to Crystal as well, since its stdlib also has the same behavior, so I’m thinking of opening an issue there)
Description
Kemal::WebSocketHandlercurrently gates the WebSocket upgrade solely onws_route_found?and the presence ofUpgrade/Connectionheaders. The HTTP method is never validated.RFC 6455 §4.1 requires the opening handshake to be a GET request. As a result, any method (POST, QUERY, etc.) that includes the correct headers will successfully complete the handshake:
curl -si -X QUERY localhost:3000/chat \ -H 'Upgrade: websocket' \ -H 'Connection: Upgrade' \ -H 'Sec-WebSocket-Key: dGhlIHNhbXBsZSBub25jZQ==' \ -H 'Sec-WebSocket-Version: 13' \ -H 'Origin: http://localhost:3000' # HTTP/1.1 101 Switching ProtocolsNote: Crystal’s standard library
HTTP::WebSocketHandlerexhibits the same behavior (websocket_upgrade_request?only inspects headers), so an upstream report may also be worthwhile.Expected behavior
Non-GET upgrade requests should not be upgraded. They should either fall through to the next handler or be rejected with
405 Method Not Allowed. This matches the behavior of the broader ecosystem:gorilla/websocketand Nodews→ 405python-websockets→ 405 +Allow: GETfaye/websocket-driver(Sinatra/Rails) → handshake failureSuggested fix
In
WebSocketHandler#call, reject the request unlesscontext.request.method == "GET"before attempting the upgrade. This is a one-line change plus a corresponding spec.Impact
Direct impact is low because the Origin check still applies. However, it is a clear protocol violation and can confuse intermediaries. The issue was discovered while dogfooding HTTP QUERY support (#769), where a QUERY route and a WebSocket route share the same path.