search

vbmithr / ocaml-websocket

Websocket library for OCaml
ISC License
162 stars 44 forks source link

Handle truncated cohttp stream reads #60

Closed kayceesrk closed 8 years ago

kayceesrk commented 8 years ago

Cohttp input stream reads may return less than the requested number of bytes [0]. The PR fixes the use of read in websocket core by replacing them with read_exactly which attempts to read exactly the requested number of bytes (and returns None if such a read is not possible).

This PR fixes a bug with the slackbot that I'm writing (will file an issue once I clean up the test case). The summary is that the header reads in make_read_frame causes Invalid_argument "index out of bounds" exception since only 1 byte was read, whereas the continuation expects 2 bytes [1].

Happy to take comments on improving the PR.

[0] https://github.com/mirage/ocaml-cohttp/blob/5c68585259ef4d5a2bf9c2250cbadfae94704241/lib/s.mli#L51-L54 [1] https://github.com/vbmithr/ocaml-websocket/blob/master/lib/websocket.ml#L163-L167

vbmithr commented 8 years ago

Thanks.

vbmithr commented 8 years ago

Tell me if you have other problems with it, I'll make a new release once you're happy with it.

kayceesrk commented 8 years ago

Excellent. Thanks for merging.

kayceesrk commented 8 years ago

I haven't encountered any further problems. I think it is good to go for a new release.

vbmithr commented 8 years ago

On 26/07/2016 10:56, KC Sivaramakrishnan wrote:

I haven't encountered any further problems. I think it is good to go for a new release.

Done. Thanks :)