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

Possible error in FrameCodec::write_pending #83

Closed
najamelan opened this issue Oct 6, 2019 · 2 comments
Closed

Possible error in FrameCodec::write_pending #83

najamelan opened this issue Oct 6, 2019 · 2 comments
Assignees
Labels

Comments

@najamelan
Copy link
Contributor

This method has a loop as:

while !self.out_buffer.is_empty() {
    let len = stream.write(&self.out_buffer)?;
    self.out_buffer.drain(0..len);
}

However the documentation of std::io::Write::write states:

If the return value is Ok(n) then it must be guaranteed that n <= buf.len(). A return value of 0 typically means that the underlying object is no longer able to accept bytes and will likely not be able to in the future as well, or that the buffer provided is empty.

If anything ever returns Ok(0) here, this hangs in an infinite loop. I think 0 len is sometimes returned to signal an unexpected EOF. I think this loop should check for that and probably bubble up an error instead of looping infinitely.

najamelan added a commit to najamelan/ws_stream_tungstenite that referenced this issue Oct 6, 2019
@agalakhov agalakhov added bug and removed question labels Oct 8, 2019
@agalakhov
Copy link
Member

agalakhov commented Oct 8, 2019

This is a bug. It seems to be harmless since things like TcpStream never actually return Ok(0) on write. Their indicator of "connection closed" is io::Error with ConnectionReset kind. Corrected this anyway. Let me know if it works for you as expected.

@najamelan
Copy link
Contributor Author

Thanks or the quick resolution. The fix looks good. The thing is that tungstenite works on any io::Read/Write, not just TcpStream. I don't intend to use it other than to communicate to browsers, but sometimes people do. Also testing code might not use TcpStream but a mock network stream.

a-miyashita pushed a commit to givery-technology/tungstenite-rs that referenced this issue Jul 20, 2023
Closes: snapview#83

Signed-off-by: Alexey Galakhov <agalakhov@snapview.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants