python-websockets / websockets

Library for building WebSocket servers and clients in Python
https://websockets.readthedocs.io/
BSD 3-Clause "New" or "Revised" License
5.16k stars 513 forks source link

HTTP server returns 404 from Websocket request on valid websocket endpoint #250

Closed Redrield closed 7 years ago

Redrield commented 7 years ago

The problem: My HTTP server, written using the Ktor framework in Kotlin has a Websocket endpoint at /ws. When I try to connect to it using this library, it returns a 404. I'm running on Manjaro Linux

The solution: I found that a solution to the issue was as simple as changing Upgrade: WebSocket to Upgrade: websocket in the headers of the request. After that small change, it connects as usual and just works.

cjerdonek commented 7 years ago

Could this be a bug in that framework (perhaps here)?

RFC 6455 says in section 4.2.1. "Reading the Client's Opening Handshake":

An |Upgrade| header field containing the value "websocket", treated as an ASCII case-insensitive value.

cjerdonek commented 7 years ago

It couldn't hurt to change it here, though. The same RFC also says in section 4.1. "Client Requirements":

The request MUST contain an |Upgrade| header field whose value MUST include the "websocket" keyword.

This language doesn't include the case-insensitive qualifier though, so maybe changing to lowercase will be more likely to work with other servers.

aaugustin commented 7 years ago

Yes, let's change it to lowercase. Cool edge case :-)

Redrield commented 7 years ago

In the end it was actually a problem with the server framework, turns out. They've patched it, so this isn't a necessity for me anymore. Just so you know it's not super urgent

aaugustin commented 7 years ago

FTR the Ktor issue is https://github.com/Kotlin/ktor/issues/186 and the corresponding commit is https://github.com/Kotlin/ktor/commit/6330d036de9d1a58e4569cfc0d3451971c9773fe.

cjerdonek commented 7 years ago

This may be worth a code comment since the reason for lower-casing is somewhat obscure and there is no test.

On Sun, Aug 20, 2017 at 6:12 AM Aymeric Augustin notifications@github.com wrote:

Closed #250 https://github.com/aaugustin/websockets/issues/250 via a786635 https://github.com/aaugustin/websockets/commit/a786635863f591106b258716be2d2998692e2505 .

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/aaugustin/websockets/issues/250#event-1213185556, or mute the thread https://github.com/notifications/unsubscribe-auth/AAVt7vJvOjd7iLEHclhS7PmkoFD4Bzupks5saDChgaJpZM4O36Ar .

aaugustin commented 7 years ago

Damn, caught by the patrol again :-)

To be honest I didn't care enough to write a test... mostly because I don't think anyone will ever find a reason to change it back.

cjerdonek commented 7 years ago

:) Yeah, I wasn't demanding a test! I was just suggesting a code comment as an easy alternative (and in the absence of a test).