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 length when reading websocket frame #2

Merged
merged 2 commits into from Jul 13, 2019
Merged

Fix length when reading websocket frame #2

merged 2 commits into from Jul 13, 2019

Conversation

soumya92
Copy link
Contributor

Per https://tools.ietf.org/html/rfc6455 page 29:

If 127, the following 8 bytes interpreted as a 64-bit unsigned integer

I've verified that changing the 4 to an 8 makes this gem work locally. Otherwise it raises protocol-websocket-0.7.2/lib/protocol/websocket/frame.rb:154:in `read': undefined method `>' for nil:NilClass (NoMethodError)

@ioquatix
Copy link
Member

This looks great, can you add a spec for it too? Thanks!

@soumya92
Copy link
Contributor Author

Sure, I'll have something later today.

soumya92 and others added 2 commits July 12, 2019 22:19
Per https://tools.ietf.org/html/rfc6455 page 29:
> If 127, the following 8 bytes interpreted as a 64-bit unsigned integer

I've verified that changing the 4 to an 8 makes this gem work locally. Otherwise it raises `protocol-websocket-0.7.2/lib/protocol/websocket/frame.rb:154:in `read': undefined method `>' for nil:NilClass (NoMethodError)`
Without the fix in the previous commit, this spec will fail:

Protocol::WebSocket::Connection
  message length
    can handle large message (>65k) (FAILED - 1)

Failures:

  1) Protocol::WebSocket::Connection message length can handle large message (>65k)
     Failure/Error: if length > maximum_frame_size

     NoMethodError:
       undefined method `>' for nil:NilClass
@soumya92
Copy link
Contributor Author

Added a spec. Let me know if that works for you

@ioquatix ioquatix merged commit 5736f5d into socketry:master Jul 13, 2019
@ioquatix
Copy link
Member

Excellent work! Thanks so much for this.

@ioquatix ioquatix self-assigned this Jul 13, 2019
@ioquatix ioquatix added the bug Something isn't working label Jul 13, 2019
@ioquatix ioquatix added this to the 0.7.3 milestone Jul 13, 2019
@ioquatix
Copy link
Member

Released in v0.7.3. What are you planning to use this library for?

@soumya92
Copy link
Contributor Author

Thanks for the quick release! I had it locally patched in the meanwhile.
I use this library to interface with home-assistant, for quicker updates and access to portions of its websockets-only API.

@ioquatix
Copy link
Member

What is home-assistant?

@soumya92
Copy link
Contributor Author

https://www.home-assistant.io/
Platform for using smart home devices together

@ioquatix
Copy link
Member

Looks great! Well if you find any other issues your contributions are welcome!

@soumya92
Copy link
Contributor Author

Thanks, I'll definitely send any other bugfixes/improvements your way, especially since the review was very quick :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants