search

python-hyper / wsproto

Sans-IO WebSocket protocol implementation
https://wsproto.readthedocs.io/
MIT License
261 stars 38 forks source link

Make the value of the upgrade header the same as the RFC. #187

Closed palkeo closed 8 months ago

palkeo commented 9 months ago

As per https://datatracker.ietf.org/doc/html/rfc6455#section-4.2.1 the "Upgrade" header should have the value "websocket", and the server should treat it as case-insensitive.

However, sadly some servers don't respect that case-insentitivity, so better to use exactly the string as specified in the RFC.

That's what the other websocket implementations do. See for example: https://github.com/python-websockets/websockets/blob/94dd203f63bb52b1a30faa228e63ada2f0f2e874/src/websockets/client.py#L119

Kriechi commented 9 months ago

Since this value seems to be important to follow the spec, how about pulling it out into a CONSTANT and then re-using it in all the places you now touched, instead of hard-coding it?

We already have this similar one: https://github.com/python-hyper/wsproto/blob/c0a107939d6c0fccdb55028b39b3db026319b65e/src/wsproto/handshake.py#L36

Otherwise, I think I'm 👍 for this change! thanks for catching it!

palkeo commented 9 months ago

Good point, I moved it into a constant.

I kept it hardcoded for tests, I think it makes sense to check the end result there. WEBSOCKET_VERSION is also hardcoded in a test.

Kriechi commented 9 months ago

meta comment: CI lint failed for unrelated reasons.