Skip to content

Reject non-GET WebSocket upgrade requests with 405 (RFC 6455) - #773

Merged
sdogruyol merged 2 commits into
masterfrom
hahwul/fix-websocket-rfc-6455
Aug 15, 2026
Merged

Reject non-GET WebSocket upgrade requests with 405 (RFC 6455)#773
sdogruyol merged 2 commits into
masterfrom
hahwul/fix-websocket-rfc-6455

Conversation

@hahwul

@hahwul hahwul commented Aug 15, 2026

Copy link
Copy Markdown
Member

Description of the Change

Kemal::WebSocketHandler previously gated the WebSocket upgrade solely on a matching ws route and the presence of Upgrade / Connection headers. The HTTP method itself was never validated, so any method (POST, QUERY, …) carrying valid upgrade headers could complete the handshake. RFC 6455 §4.1 requires the opening handshake to be a GET request.

This PR adds a method guard in WebSocketHandler#call, placed after the existing “is this an upgrade request for a ws route?” check and before the Origin check:

  • Non-GET upgrade requests are rejected with 405 Method Not Allowed, an Allow: GET header (required for 405 by RFC 9110 §10.2.1), and Connection: close — the same connection-smuggling protection already used by the 403 Origin rejection ([Security] Close connection after rejected WebSocket upgrade #767).
  • Requests without upgrade headers are left untouched: they fall through to the next handler exactly as before, so an HTTP route that shares a path with a ws route continues to work.
  • Both rejection paths (403 / 405) now share a single reject_websocket! helper driven by HTTP::Status. The helper also skips writing if an upstream handler (e.g. a before filter) has already closed the response.

Before / after, using the reproduction from #770:

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'
# before: HTTP/1.1 101 Switching Protocols
# after:  HTTP/1.1 405 Method Not Allowed
#         Allow: GET
#         Connection: close

Specs cover the full 405 response shape (status, Allow, Connection: close, Content-Type, body) for POST / QUERY / HEAD, confirm that the method check runs before the Origin check, and verify the unchanged fall-through for non-upgrade requests. A CHANGELOG entry is included.

Alternate Designs

  • Fall through to the next handler instead of rejecting.
    RFC 9110 §7.8 allows a server to ignore Upgrade and process the request normally. Rejected: before this change a non-GET upgrade request never reached RouteHandler anyway (it was upgraded or returned 403), so falling through would be a larger behavior change. An explicit 405 also matches the wider ecosystem — gorilla/websocket and Node ws return 405, python-websockets returns 405 + Allow: GET, and Cowboy only upgrades GET.

  • Route the rejection through custom error 405 handlers.
    Rejected for consistency: the 403 Origin rejection deliberately writes the response directly so the security-relevant shape (Connection: close) cannot be altered; the 405 path follows the same rule.

  • Compute Allow dynamically from coexisting routes on the same path.
    Rejected as overkill: it would couple WebSocketHandler to RouteHandler internals. A static Allow: GET correctly describes the upgrade resource and matches python-websockets’ behavior.

Benefits

  • RFC 6455 §4.1 conformance and parity with the broader WebSocket ecosystem.
  • A clear, diagnosable 405 + Allow: GET instead of a protocol-violating 101 that can confuse intermediaries (or a misleading 403 that suggests an Origin problem).
  • A slightly more robust rejection path: shared helper, reason phrases taken from HTTP::Status, and no write attempt on an already-closed response.

Possible Drawbacks

  • Behavior change. Clients that (incorrectly) performed the handshake with a non-GET method previously received 101 (allowed origin) or 403, and now receive 405. This is documented in the CHANGELOG; such clients were already violating RFC 6455.
  • A non-GET request that carries upgrade headers to a path shared by a ws route and an HTTP route is answered with 405 rather than reaching the HTTP route. This is not a regression — previously the request never reached the route either (it was upgraded or returned 403) — and requests without upgrade headers are unaffected.
  • Allow: GET is static; it does not enumerate methods of HTTP routes that coexist on the same path (see Alternate Designs).

WebSocketHandler gated the upgrade solely on a ws route match and the
Upgrade/Connection headers, so any HTTP method (POST, QUERY, ...) with
the right headers completed the handshake. RFC 6455 §4.1 requires the
opening handshake to be a GET request; respond with 405 Method Not
Allowed plus Allow: GET instead of upgrading, keeping Connection: close
like the 403 path so a pipelined request cannot reuse the connection.

The method check runs before the Origin check, both rejections share a
single HTTP::Status-driven helper, and rejections are skipped when an
upstream handler (e.g. a before filter) already closed the response.

Closes #770
@hahwul hahwul self-assigned this Aug 15, 2026
Mirror the connection-smuggling regression spec (#767) for the new
non-GET rejection path: a POST upgrade with a compound
`Connection: keep-alive, Upgrade` must produce a single 405 with
Connection: close, and the pipelined request must not be served.
@hahwul
hahwul marked this pull request as ready for review August 15, 2026 05:14
@hahwul
hahwul requested a review from sdogruyol August 15, 2026 05:14

@sdogruyol sdogruyol left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot @hahwul 🙏

@hahwul

hahwul commented Aug 15, 2026

Copy link
Copy Markdown
Member Author

@sdogruyol
Thanks for your kind words!

@sdogruyol
sdogruyol merged commit 204a358 into master Aug 15, 2026
38 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.

WebSocketHandler accepts upgrade requests with any HTTP method (RFC 6455 requires GET)

2 participants