search

python-hyper / wsproto

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

Should fix issue #160 #161

Closed gndu91 closed 2 years ago

gndu91 commented 3 years ago

I changed both the encoding and the decoding of the Host, as I assume it is internally represented as Unicode.

gndu91 commented 3 years ago

160

Kriechi commented 3 years ago

Would you mind adding a test in test/test_handshake.py that checks that we really are using idna from now on (and if anyone would refactor the code it would be obvious)?

gndu91 commented 3 years ago

I changed the host to include non-ascii characters. For the moment it's the only way I found to make sure it works with non-ascii character, as in order to make sure idna is used, I will have to use private functions, namely H11Handshake._initiate_connection and H11Handshake._process_connection_request, as Unicode is used everywhere, for instance in the result of next(server.events()) (line 14 of test/test_handshake.py)

Kriechi commented 3 years ago

Thanks @gndu91 - while this now tests that it sends "something", ideally we should also assert that we actually receive a correctly idna-encoded Host in the test. And the same for the other direction: we should test that receiving an idna-encoded Host is correctly decoded.

Unfortunately we don't have any easy test that you could copy, so you might need to get a bit creative here. You can probably hook into the output of client.send and into the server.events() to assert the right values (and also the other direction by swapping client and server).

pgjones commented 2 years ago

Thanks I've partially merged this with an alternative test.