Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix byte/string conversion for Python 3. #9

Closed
wants to merge 3 commits into from
Closed

Fix byte/string conversion for Python 3. #9

wants to merge 3 commits into from

Conversation

Lokaltog
Copy link

Python 3's handling of bytes and strings caused the Sec-WebSocket-Accept header to be wrapped in b'...', e.g. like Sec-WebSocket-Accept: b'qqPS3vItJSiRDxlSVrmaSmAi+MQ=' as well as throwing a TypeError at line 315. These issues seem to be resolved by using the s2b/b2s functions to convert bytes/strings correctly. I'm able to run the tests successfully with both Python 2.7.2 and Python 3.2.2 after applying this patch.

These functions throw TypeError exceptions if the parameters are already
encoded/decoded. This can be avoided by type checking before attempting
to encode/decode the parameters.
@kanaka
Copy link
Member

kanaka commented Sep 10, 2011

Figured out a simple solution: 1c39c7f

The problem was really the initializing of b and c to normal strings. Just wrapping those in s2b() (and doing b2s on the accept value) fixes the issue.

Thanks for reporting it. Even though I didn't go with your solution, seeing your solution helped me understand what was going on and create a fix faster so thanks for your effort.

@kanaka kanaka closed this Sep 10, 2011
@Lokaltog
Copy link
Author

That looks much better. Thanks for fixing this issue!

ncarrier pushed a commit to ncarrier/carino-packages-websockify that referenced this pull request Mar 21, 2015
Simpler fix for this issue:
novnc/websockify#9

Use consistent string/byte type for HyBi decode with numpy.
@groupboard groupboard mentioned this pull request Jan 27, 2018
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.

2 participants