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

Error decoding fragmented frames #71

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jeanparpaillon
Copy link

See #70

Jean Parpaillon added 2 commits September 3, 2021 12:21
When receiving fragmented frames, decoding can fail if websocket / TCP
fragmentation is aligned for the last frame.

Fixes sanmiguel#70
@fhunleth
Copy link

fhunleth commented Sep 3, 2021

Did you mean to include the decode_frame -> decode_header change?

Also, wow, nice find!

@jeanparpaillon
Copy link
Author

@fhunleth if you don't mind, we can include the function name change, but you can pick the second commit only, if you prefer

btw, I would have liked adding a non-regression test (I have some sample data for this), but I was unable to run the tests. Got this error:

$ rebar3 ct 
===> Verifying dependencies...
===> Compiling websocket_client
src/websocket_client.erl:134:10: Warning: http_uri:parse/2 is deprecated and will be removed in OTP 25; use uri_string functions instead

src/wsc_lib.erl:7:2: Warning: export_all flag enabled - all functions will be exported

build/test/lib/websocketclient/test/wsreq_SUITE.erl:3:2: Warning: export_all flag enabled - all functions will be exported

build/test/lib/websocketclient/test/wsc_lib_SUITE.erl:3:2: Warning: export_all flag enabled - all functions will be exported

===> Running Common Test suites...
===> Error running tests:
  "Failed to start CTH, see the CT Log for details"

@fhunleth
Copy link

fhunleth commented Sep 3, 2021

Sorry - I don't maintain this project. I just saw the fix and was trying to be helpful to get it merged as quickly as possible.

@sanmiguel
Copy link
Owner

I am finally making my way back to get this library up-to-date. Will pick up this PR soon.... thanks Jean!

@jeanparpaillon
Copy link
Author

@sanmiguel you're welcome 🙏

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.

3 participants