enforce rfc6455 section 4.2.1 requirement to refuse malformed Websocket handshake

[pajod]

What do these changes do?

• refuse HTTP/1.1 WebSocket handshake when method != "GET"
• there is precedent for being careful about emitting 1xx informational responses that could confuse a proxy

I only noticed this was previously in place and had been removed in #3980 after writing this. I do not believe it should have been, given that proxies may depend on the clear language in the spec.

Are there changes in behavior for the user?

• Not for users that follow the instructions in documentation:

The handler should be registered as HTTP GET processor

• Possibly for users that extend examples/web_ws.py and end up registering such dual-use handler to non-GET routes.
• Possibly for other users having legitimate use cases that I am simply not aware of.

Is it a substantial burden for the maintainers to support this?

• adds single branch which can probably stay untouched forever
  • but will break BADLY in backwards-incompatible manner, in case I missed something
• by submitting a draft I am potentially dumping related work (HTTP version check, allow_head=True docs, ..) on others

References

• https://datatracker.ietf.org/doc/html/rfc6455#section-4.2.1

If the server, while reading the handshake, finds that the client did
not send a handshake that matches the description below [..] the server MUST stop
processing the client's handshake and return an HTTP response with an
appropriate error code (such as 400 Bad Request).

1. An HTTP/1.1 or higher GET request [..]

• https://websockets.spec.whatwg.org/

• https://datatracker.ietf.org/doc/html/rfc9110#field.upgrade

• https://docs.aiohttp.org/en/stable/web_quickstart.html#websockets

• I have failed to identify which client issue motivated supporting non-rfc6455 websocket handshake, but recent docker versions are clearly expecting GET for the relevant API endpoint.

Related issue number

• Reverts: Accept non-GET request for starting websocket handshake on server side #3980

Checklist

• I think the code is well written
• Unit tests for the changes exist
• Documentation reflects the changes
  • already did.. with this patch no further warning about the footgun is needed
• Add a new news fragment into the CHANGES/ folder

[codspeed-hq]

CodSpeed Performance Report

Merging #10729 will not alter performance

_{Comparing pajod:patch-1 (14ed61f) with master (d912123)}

Summary

✅ 55 untouched benchmarks

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.72%. Comparing base (d912123) to head (14ed61f).

Additional details and impacted files

@@           Coverage Diff           @@
##           master   #10729   +/-   ##
=======================================
  Coverage   98.72%   98.72%           
=======================================
  Files         125      125           
  Lines       37722    37755   +33     
  Branches     2082     2083    +1     
=======================================
+ Hits        37240    37273   +33     
  Misses        334      334           
  Partials      148      148           

Flag	Coverage Δ
CI-GHA	98.60% <100.00%> (+<0.01%)	⬆️
OS-Linux	98.27% <100.00%> (+<0.01%)	⬆️
OS-Windows	96.22% <100.00%> (-0.01%)	⬇️
OS-macOS	97.38% <100.00%> (+<0.01%)	⬆️
Py-3.10.11	97.29% <100.00%> (-0.01%)	⬇️
Py-3.10.16	?
Py-3.10.17	97.83% <100.00%> (-0.01%)	⬇️
Py-3.11.11	?
Py-3.11.12	97.92% <100.00%> (-0.01%)	⬇️
Py-3.11.9	97.38% <100.00%> (-0.01%)	⬇️
Py-3.12.10	98.03% <100.00%> (+<0.01%)	⬆️
Py-3.12.9	97.49% <100.00%> (-0.88%)	⬇️
Py-3.13.2	97.48% <100.00%> (-0.89%)	⬇️
Py-3.13.3	98.03% <100.00%> (+<0.01%)	⬆️
Py-3.9.13	97.16% <100.00%> (+<0.01%)	⬆️
Py-3.9.21	?
Py-3.9.22	97.69% <100.00%> (-0.01%)	⬇️
Py-pypy7.3.16	76.41% <11.36%> (-19.06%)	⬇️
VM-macos	97.38% <100.00%> (+<0.01%)	⬆️
VM-ubuntu	98.27% <100.00%> (+<0.01%)	⬆️
VM-windows	96.22% <100.00%> (-0.01%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[Dreamsorcerer]

The release notes that added the client-side functionality says:

Sometimes server API requires a different HTTP method for WebSocket connection establishment.
For example, Docker exec needs POST.
#3368

I don't know if that's still true, but if so, it may be desirable to be able to imitate the same behaviour. (We've seen this also in aiohttp-sse, where several users need to be compatible with some client that does POST requests for SSE connection).

[Dreamsorcerer]

For aiohttp-sse, we ended up removing the check with a flag, so people couldn't use a non-GET endpoint without explicitly opting into it (this helped avoid the situation of a backend developer creating POST endpoints, and then the frontend developers telling them after it's deployed that they can't connect to it, because browsers only allow GET requests).

[bdraco]

This probably needs to be controlled by a flag so that users can opt out if needed. I don’t think #3980 would’ve been merged unless it was addressing a real issue, so we should be cautious about changing the behavior outright.

If the flag is unset, we should default to the current 3.x behavior for now. In 4.x, we can flip the default so that only GET is allowed when the flag is unset.

To help with the transition, 3.x should log a deprecation warning whenever a non-GET request is accepted, so users get a clear signal that this will break in 4.x unless they explicitly set the flag.