1tgr / rust-websocket-lite

A fast, low-overhead WebSocket client
MIT License
115 stars 17 forks source link

Enable buffering multiple messages before flush #291

Closed julihoh closed 2 years ago

julihoh commented 2 years ago

The current implementation of MessageCodec does not correctly handle the buffer when writing.

Specifically, writing multiple websocket messages without flushing between each write will write incorrect frames.

The following does not work on the current implementation:

let buf = Vec::new();
let mut framed = FramedWrite::new(buf, websocket_lite::MessageCodec::server());
framed.feed(Message::text("A")).await.unwrap();
framed.feed(Message::text("B")).await.unwrap();
SinkExt::<Message>::flush(&mut framed).await.unwrap();
let test = framed.into_inner();
let framed = FramedRead::new(test.as_slice(), MessageCodec::client());
let read = framed.try_collect::<Vec<_>>().await.unwrap();
assert_eq!(&read, &[Message::text("A"), Message::text("B")]);

The reason for this misbehaviour is that the codec implementation assumes that it receives a new buffer for each write. However, according to the docs:

The buffer may already contain data, and in this case, the encoder should append the new frame the to buffer rather than overwrite the existing data.

Ignoring this detail, the current implementation overwrites the existing buffer, leading to garbage frames being written.

This PR fixes this bug by correctly handling non-empty buffers in the Encoder implementation.

1tgr commented 2 years ago

Thanks! On CI, cargo fmt wants to make a change, but it's not clear to me why: https://github.com/1tgr/rust-websocket-lite/runs/7482420549?check_suite_focus=true#step:5:15

Can you run cargo fmt on your side?

julihoh commented 2 years ago

Awesome! Thanks for fixing the formatting.

Are you planning to do a release soon?

1tgr commented 2 years ago

It’s all done now, I published 0.5.2: https://crates.io/crates/websocket-codec/0.5.2

Thanks for the fix!