Fixes for Issues 26 and 27.

Pithikos / python-websocket-server

A simple fully working websocket-server in Python with no external dependencies

MIT License

1.14k stars 381 forks source link

Fixes for Issues 26 and 27. #28

Closed CMTaylor closed 6 years ago

CMTaylor commented 7 years ago

This PR contains fixes as described in my comments to the recently added Issues 26 and 27.

Pithikos commented 7 years ago

Thanks for work on this CMTaylor.

Atm I am a bit tight on time but I assure you that I will review the pull request when I get some free time. Atm the testing is quite manual so I have also started to try getting the testing in an automated way.

From a fast glance on the code, the code itself seems fine. I am a bit hesitant on the comments in the code though. I would consider moving that information to the pull request itself in order to not get any confusion in the future.

CMTaylor commented 6 years ago

This PR has been languishing for >6 months. If its blocked on your hesitancy about the included comments please be very specific in what changes you want me to make. Personally I LOVE comments like this in the code 'cause they explain WHY a particular implementation or design choice was made, thus making future maintenance decisions much easier.

Please do what you can to get this PR reviewed and merged ASAP. My team is lagging behind on other PRs 'cause we MUST have this one in order to make python-websocket-server work for us.

Pithikos commented 6 years ago

Hi Taylor. I spent some time today on this. Although your solution probably works, I decided to get a bit deeper and try to resolve the underlying issue. With c3f0b00fbd29e478b814b4ea32b0b7d479e89841 the HTTP headers should be read irregardless of size now.

If you notice that this doesn't work please let me know.

Cheers