From b8f7d3668ec79c62abb9562ee3b5dd355908b6a5 Mon Sep 17 00:00:00 2001 From: Alexey Galakhov Date: Tue, 14 May 2019 01:12:53 +0200 Subject: [PATCH 1/4] close: refine close semantics Signed-off-by: Alexey Galakhov --- src/error.rs | 29 +++++++++++------- src/protocol/mod.rs | 71 ++++++++++++++++++++++++++++----------------- 2 files changed, 62 insertions(+), 38 deletions(-) diff --git a/src/error.rs b/src/error.rs index 543774c..0483a82 100644 --- a/src/error.rs +++ b/src/error.rs @@ -11,7 +11,6 @@ use std::string; use httparse; -use protocol::frame::CloseFrame; use protocol::Message; #[cfg(feature="tls")] @@ -26,8 +25,20 @@ pub type Result = result::Result; /// Possible WebSocket errors #[derive(Debug)] pub enum Error { - /// WebSocket connection closed (normally) - ConnectionClosed(Option>), + /// WebSocket connection closed normally + /// + /// Upon receiving this, the server must drop the WebSocket object as soon as possible + /// to close the connection. + /// The client gets this error if the connection is already closed at the server side. + /// + /// Receiving this error means that the WebSocket object is not usable anymore and the only + /// meaningful action with it is dropping it. + ConnectionClosed, + /// Trying to work with already closed connection + /// + /// Trying to write after receiving `Message::Close` or trying to read after receiving + /// `Error::ConnectionClosed` causes this. + AlreadyClosed, /// Input-output error Io(io::Error), #[cfg(feature="tls")] @@ -50,13 +61,8 @@ pub enum Error { impl fmt::Display for Error { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match *self { - Error::ConnectionClosed(ref frame) => { - if let Some(ref cf) = *frame { - write!(f, "Connection closed: {}", cf) - } else { - write!(f, "Connection closed (empty close frame)") - } - } + Error::ConnectionClosed => write!(f, "Connection closed normally"), + Error::AlreadyClosed => write!(f, "Trying to work with closed connection"), Error::Io(ref err) => write!(f, "IO error: {}", err), #[cfg(feature="tls")] Error::Tls(ref err) => write!(f, "TLS error: {}", err), @@ -73,7 +79,8 @@ impl fmt::Display for Error { impl ErrorTrait for Error { fn description(&self) -> &str { match *self { - Error::ConnectionClosed(_) => "A close handshake is performed", + Error::ConnectionClosed => "A close handshake is performed", + Error::AlreadyClosed => "Trying to read or write after getting close notification", Error::Io(ref err) => err.description(), #[cfg(feature="tls")] Error::Tls(ref err) => err.description(), diff --git a/src/protocol/mod.rs b/src/protocol/mod.rs index f794859..1997a75 100644 --- a/src/protocol/mod.rs +++ b/src/protocol/mod.rs @@ -8,7 +8,7 @@ pub use self::message::Message; pub use self::frame::CloseFrame; use std::collections::VecDeque; -use std::io::{Read, Write}; +use std::io::{Read, Write, ErrorKind as IoErrorKind}; use std::mem::replace; use error::{Error, Result}; @@ -206,6 +206,9 @@ impl WebSocketContext { where Stream: Read + Write, { + // Do not read from already closed connections. + self.state.check_active()?; + loop { // Since we may get ping or close, we need to reply to the messages even during read. // Thus we call write_pending() but ignore its blocking. @@ -231,6 +234,9 @@ impl WebSocketContext { where Stream: Read + Write, { + // Do not write to already closed connections. + self.state.check_active()?; + if let Some(max_send_queue) = self.config.max_send_queue { if self.send_queue.len() >= max_send_queue { // Try to make some room for the new message. @@ -288,17 +294,14 @@ impl WebSocketContext { // willing to take more data. // If we're closing and there is nothing to send anymore, we should close the connection. - if let WebSocketState::ClosedByPeer(ref mut frame) = self.state { + if let (Role::Server, WebSocketState::ClosedByPeer) = (&self.role, &self.state) { // The underlying TCP connection, in most normal cases, SHOULD be closed // first by the server, so that it holds the TIME_WAIT state and not the // client (as this would prevent it from re-opening the connection for 2 // maximum segment lifetimes (2MSL), while there is no corresponding // server impact as a TIME_WAIT connection is immediately reopened upon // a new SYN with a higher seq number). (RFC 6455) - match self.role { - Role::Client => Ok(()), - Role::Server => Err(Error::ConnectionClosed(frame.take())), - } + Err(Error::ConnectionClosed) } else { Ok(()) } @@ -449,9 +452,10 @@ impl WebSocketContext { } // match opcode } else { + // Connection closed by peer match replace(&mut self.state, WebSocketState::Terminated) { - WebSocketState::CloseAcknowledged(close) | WebSocketState::ClosedByPeer(close) => { - Ok(Some(Message::Close(close))) + WebSocketState::ClosedByPeer | WebSocketState::CloseAcknowledged => { + Err(Error::ConnectionClosed) } _ => { Err(Error::Protocol("Connection reset without closing handshake".into())) @@ -461,13 +465,12 @@ impl WebSocketContext { } /// Received a close frame. Tells if we need to return a close frame to the user. - fn do_close(&mut self, close: Option) -> Option>> { + fn do_close<'t>(&mut self, close: Option>) -> Option>> { debug!("Received close frame: {:?}", close); match self.state { WebSocketState::Active => { let close_code = close.as_ref().map(|f| f.code); - let close = close.map(CloseFrame::into_owned); - self.state = WebSocketState::ClosedByPeer(close.clone()); + self.state = WebSocketState::ClosedByPeer; let reply = if let Some(code) = close_code { if code.is_allowed() { Frame::close(Some(CloseFrame { @@ -488,24 +491,14 @@ impl WebSocketContext { Some(close) } - WebSocketState::ClosedByPeer(_) | WebSocketState::CloseAcknowledged(_) => { + WebSocketState::ClosedByPeer | WebSocketState::CloseAcknowledged => { // It is already closed, just ignore. None } WebSocketState::ClosedByUs => { // We received a reply. - let close = close.map(CloseFrame::into_owned); - match self.role { - Role::Client => { - // Client waits for the server to close the connection. - self.state = WebSocketState::CloseAcknowledged(close); - None - } - Role::Server => { - // Server closes the connection. - Some(close) - } - } + self.state = WebSocketState::CloseAcknowledged; + Some(close) } WebSocketState::Terminated => unreachable!(), } @@ -525,7 +518,20 @@ impl WebSocketContext { frame.set_random_mask(); } } - self.frame.write_frame(stream, frame) + + let res = self.frame.write_frame(stream, frame); + // An expected "Connection reset by peer" is not fatal + match res { + Err(Error::Io(err)) => Err({ + match self.state { + WebSocketState::ClosedByPeer | WebSocketState::CloseAcknowledged + if err.kind() == IoErrorKind::ConnectionReset => + Error::ConnectionClosed, + _ => Error::Io(err), + } + }), + x => x, + } } } @@ -538,9 +544,9 @@ enum WebSocketState { /// We initiated a close handshake. ClosedByUs, /// The peer initiated a close handshake. - ClosedByPeer(Option>), + ClosedByPeer, /// The peer replied to our close handshake. - CloseAcknowledged(Option>), + CloseAcknowledged, /// The connection does not exist anymore. Terminated, } @@ -553,6 +559,17 @@ impl WebSocketState { _ => false, } } + + /// Check if the state is active, return error if not. + fn check_active(&self) -> Result<()> { + match self { + WebSocketState::ClosedByPeer | WebSocketState::CloseAcknowledged + => Err(Error::ConnectionClosed), + WebSocketState::Terminated + => Err(Error::AlreadyClosed), + _ => Ok(()), + } + } } #[cfg(test)] From 8cc65fdddcf90ac7614a8e30af1f535c258a54d3 Mon Sep 17 00:00:00 2001 From: Alexey Galakhov Date: Tue, 14 May 2019 01:57:12 +0200 Subject: [PATCH 2/4] fuzz: make it working again Signed-off-by: Alexey Galakhov --- fuzz/Cargo.toml | 6 +++--- .../{parse_frame.rs => parse_frame_header.rs} | 2 +- fuzz/fuzz_targets/read_message_client.rs | 2 +- fuzz/fuzz_targets/read_message_server.rs | 2 +- .../0a476920bf2b922f5d1955e5bedfedec8c13765c | Bin .../0b3a842c0b93a42d724bbab2504ae92f0593232a | 0 .../0e1990ce3bb9b7c62d5e11b83514467eca3be187 | 0 .../0fe89d4911d87df66f98147a0fd1f3624f34cdaa | 0 .../370ee4d5682fb8c2770cac1ec89e0f6674990afb | Bin .../47167be8eb1f00d36f0a93a8f61c77b4d4d2f515 | Bin .../51112bf88688ede230dd473db8fa0be2b945e636 | 0 .../7a0edc789a1d22737bfbb4cb29f26d994614cb80 | 0 .../9cf76db0663ea4025bbf89de1dfbb2ff818a26ae | 0 .../bc4057a2970c60f0c2b07200842b8d0c15e2af27 | Bin .../bd811f21956aa72bb5a9114bdf4dc61e1710ad44 | 0 .../dc048e38bdb36cf80cc4fe5b3b21ea094510af4a | Bin .../ec7d070174e40ace678006d0c631026f1d9a0779 | 0 .../ef420abfddbda7b9ee665d85ef62e4a437554003 | Bin .../fab3e168d56fecfc59f98a69f75673a53e94c215 | Bin 19 files changed, 6 insertions(+), 6 deletions(-) rename fuzz/fuzz_targets/{parse_frame.rs => parse_frame_header.rs} (75%) rename fuzz/seeds/{parse_frame => parse_frame_header}/0a476920bf2b922f5d1955e5bedfedec8c13765c (100%) rename fuzz/seeds/{parse_frame => parse_frame_header}/0b3a842c0b93a42d724bbab2504ae92f0593232a (100%) rename fuzz/seeds/{parse_frame => parse_frame_header}/0e1990ce3bb9b7c62d5e11b83514467eca3be187 (100%) rename fuzz/seeds/{parse_frame => parse_frame_header}/0fe89d4911d87df66f98147a0fd1f3624f34cdaa (100%) rename fuzz/seeds/{parse_frame => parse_frame_header}/370ee4d5682fb8c2770cac1ec89e0f6674990afb (100%) rename fuzz/seeds/{parse_frame => parse_frame_header}/47167be8eb1f00d36f0a93a8f61c77b4d4d2f515 (100%) rename fuzz/seeds/{parse_frame => parse_frame_header}/51112bf88688ede230dd473db8fa0be2b945e636 (100%) rename fuzz/seeds/{parse_frame => parse_frame_header}/7a0edc789a1d22737bfbb4cb29f26d994614cb80 (100%) rename fuzz/seeds/{parse_frame => parse_frame_header}/9cf76db0663ea4025bbf89de1dfbb2ff818a26ae (100%) rename fuzz/seeds/{parse_frame => parse_frame_header}/bc4057a2970c60f0c2b07200842b8d0c15e2af27 (100%) rename fuzz/seeds/{parse_frame => parse_frame_header}/bd811f21956aa72bb5a9114bdf4dc61e1710ad44 (100%) rename fuzz/seeds/{parse_frame => parse_frame_header}/dc048e38bdb36cf80cc4fe5b3b21ea094510af4a (100%) rename fuzz/seeds/{parse_frame => parse_frame_header}/ec7d070174e40ace678006d0c631026f1d9a0779 (100%) rename fuzz/seeds/{parse_frame => parse_frame_header}/ef420abfddbda7b9ee665d85ef62e4a437554003 (100%) rename fuzz/seeds/{parse_frame => parse_frame_header}/fab3e168d56fecfc59f98a69f75673a53e94c215 (100%) diff --git a/fuzz/Cargo.toml b/fuzz/Cargo.toml index efe5f81..983fa62 100644 --- a/fuzz/Cargo.toml +++ b/fuzz/Cargo.toml @@ -18,8 +18,8 @@ git = "https://github.com/rust-fuzz/libfuzzer-sys.git" members = ["."] [[bin]] -name = "parse_frame" -path = "fuzz_targets/parse_frame.rs" +name = "parse_frame_header" +path = "fuzz_targets/parse_frame_header.rs" [[bin]] name = "read_message_server" @@ -27,4 +27,4 @@ path = "fuzz_targets/read_message_server.rs" [[bin]] name = "read_message_client" -path = "fuzz_targets/read_message_client.rs" \ No newline at end of file +path = "fuzz_targets/read_message_client.rs" diff --git a/fuzz/fuzz_targets/parse_frame.rs b/fuzz/fuzz_targets/parse_frame_header.rs similarity index 75% rename from fuzz/fuzz_targets/parse_frame.rs rename to fuzz/fuzz_targets/parse_frame_header.rs index 1ed92ec..e4aedf7 100644 --- a/fuzz/fuzz_targets/parse_frame.rs +++ b/fuzz/fuzz_targets/parse_frame_header.rs @@ -8,5 +8,5 @@ fuzz_target!(|data: &[u8]| { let vector: Vec = data.into(); let mut cursor = Cursor::new(vector); - tungstenite::protocol::frame::Frame::parse(&mut cursor); + tungstenite::protocol::frame::FrameHeader::parse(&mut cursor).ok(); }); diff --git a/fuzz/fuzz_targets/read_message_client.rs b/fuzz/fuzz_targets/read_message_client.rs index 15a423f..1c0708b 100644 --- a/fuzz/fuzz_targets/read_message_client.rs +++ b/fuzz/fuzz_targets/read_message_client.rs @@ -33,5 +33,5 @@ fuzz_target!(|data: &[u8]| { //let vector: Vec = data.into(); let cursor = Cursor::new(data); let mut socket = WebSocket::from_raw_socket(WriteMoc(cursor), Role::Client, None); - socket.read_message(); + socket.read_message().ok(); }); diff --git a/fuzz/fuzz_targets/read_message_server.rs b/fuzz/fuzz_targets/read_message_server.rs index d96d649..d96db96 100644 --- a/fuzz/fuzz_targets/read_message_server.rs +++ b/fuzz/fuzz_targets/read_message_server.rs @@ -33,5 +33,5 @@ fuzz_target!(|data: &[u8]| { //let vector: Vec = data.into(); let cursor = Cursor::new(data); let mut socket = WebSocket::from_raw_socket(WriteMoc(cursor), Role::Server, None); - socket.read_message(); + socket.read_message().ok(); }); diff --git a/fuzz/seeds/parse_frame/0a476920bf2b922f5d1955e5bedfedec8c13765c b/fuzz/seeds/parse_frame_header/0a476920bf2b922f5d1955e5bedfedec8c13765c similarity index 100% rename from fuzz/seeds/parse_frame/0a476920bf2b922f5d1955e5bedfedec8c13765c rename to fuzz/seeds/parse_frame_header/0a476920bf2b922f5d1955e5bedfedec8c13765c diff --git a/fuzz/seeds/parse_frame/0b3a842c0b93a42d724bbab2504ae92f0593232a b/fuzz/seeds/parse_frame_header/0b3a842c0b93a42d724bbab2504ae92f0593232a similarity index 100% rename from fuzz/seeds/parse_frame/0b3a842c0b93a42d724bbab2504ae92f0593232a rename to fuzz/seeds/parse_frame_header/0b3a842c0b93a42d724bbab2504ae92f0593232a diff --git a/fuzz/seeds/parse_frame/0e1990ce3bb9b7c62d5e11b83514467eca3be187 b/fuzz/seeds/parse_frame_header/0e1990ce3bb9b7c62d5e11b83514467eca3be187 similarity index 100% rename from fuzz/seeds/parse_frame/0e1990ce3bb9b7c62d5e11b83514467eca3be187 rename to fuzz/seeds/parse_frame_header/0e1990ce3bb9b7c62d5e11b83514467eca3be187 diff --git a/fuzz/seeds/parse_frame/0fe89d4911d87df66f98147a0fd1f3624f34cdaa b/fuzz/seeds/parse_frame_header/0fe89d4911d87df66f98147a0fd1f3624f34cdaa similarity index 100% rename from fuzz/seeds/parse_frame/0fe89d4911d87df66f98147a0fd1f3624f34cdaa rename to fuzz/seeds/parse_frame_header/0fe89d4911d87df66f98147a0fd1f3624f34cdaa diff --git a/fuzz/seeds/parse_frame/370ee4d5682fb8c2770cac1ec89e0f6674990afb b/fuzz/seeds/parse_frame_header/370ee4d5682fb8c2770cac1ec89e0f6674990afb similarity index 100% rename from fuzz/seeds/parse_frame/370ee4d5682fb8c2770cac1ec89e0f6674990afb rename to fuzz/seeds/parse_frame_header/370ee4d5682fb8c2770cac1ec89e0f6674990afb diff --git a/fuzz/seeds/parse_frame/47167be8eb1f00d36f0a93a8f61c77b4d4d2f515 b/fuzz/seeds/parse_frame_header/47167be8eb1f00d36f0a93a8f61c77b4d4d2f515 similarity index 100% rename from fuzz/seeds/parse_frame/47167be8eb1f00d36f0a93a8f61c77b4d4d2f515 rename to fuzz/seeds/parse_frame_header/47167be8eb1f00d36f0a93a8f61c77b4d4d2f515 diff --git a/fuzz/seeds/parse_frame/51112bf88688ede230dd473db8fa0be2b945e636 b/fuzz/seeds/parse_frame_header/51112bf88688ede230dd473db8fa0be2b945e636 similarity index 100% rename from fuzz/seeds/parse_frame/51112bf88688ede230dd473db8fa0be2b945e636 rename to fuzz/seeds/parse_frame_header/51112bf88688ede230dd473db8fa0be2b945e636 diff --git a/fuzz/seeds/parse_frame/7a0edc789a1d22737bfbb4cb29f26d994614cb80 b/fuzz/seeds/parse_frame_header/7a0edc789a1d22737bfbb4cb29f26d994614cb80 similarity index 100% rename from fuzz/seeds/parse_frame/7a0edc789a1d22737bfbb4cb29f26d994614cb80 rename to fuzz/seeds/parse_frame_header/7a0edc789a1d22737bfbb4cb29f26d994614cb80 diff --git a/fuzz/seeds/parse_frame/9cf76db0663ea4025bbf89de1dfbb2ff818a26ae b/fuzz/seeds/parse_frame_header/9cf76db0663ea4025bbf89de1dfbb2ff818a26ae similarity index 100% rename from fuzz/seeds/parse_frame/9cf76db0663ea4025bbf89de1dfbb2ff818a26ae rename to fuzz/seeds/parse_frame_header/9cf76db0663ea4025bbf89de1dfbb2ff818a26ae diff --git a/fuzz/seeds/parse_frame/bc4057a2970c60f0c2b07200842b8d0c15e2af27 b/fuzz/seeds/parse_frame_header/bc4057a2970c60f0c2b07200842b8d0c15e2af27 similarity index 100% rename from fuzz/seeds/parse_frame/bc4057a2970c60f0c2b07200842b8d0c15e2af27 rename to fuzz/seeds/parse_frame_header/bc4057a2970c60f0c2b07200842b8d0c15e2af27 diff --git a/fuzz/seeds/parse_frame/bd811f21956aa72bb5a9114bdf4dc61e1710ad44 b/fuzz/seeds/parse_frame_header/bd811f21956aa72bb5a9114bdf4dc61e1710ad44 similarity index 100% rename from fuzz/seeds/parse_frame/bd811f21956aa72bb5a9114bdf4dc61e1710ad44 rename to fuzz/seeds/parse_frame_header/bd811f21956aa72bb5a9114bdf4dc61e1710ad44 diff --git a/fuzz/seeds/parse_frame/dc048e38bdb36cf80cc4fe5b3b21ea094510af4a b/fuzz/seeds/parse_frame_header/dc048e38bdb36cf80cc4fe5b3b21ea094510af4a similarity index 100% rename from fuzz/seeds/parse_frame/dc048e38bdb36cf80cc4fe5b3b21ea094510af4a rename to fuzz/seeds/parse_frame_header/dc048e38bdb36cf80cc4fe5b3b21ea094510af4a diff --git a/fuzz/seeds/parse_frame/ec7d070174e40ace678006d0c631026f1d9a0779 b/fuzz/seeds/parse_frame_header/ec7d070174e40ace678006d0c631026f1d9a0779 similarity index 100% rename from fuzz/seeds/parse_frame/ec7d070174e40ace678006d0c631026f1d9a0779 rename to fuzz/seeds/parse_frame_header/ec7d070174e40ace678006d0c631026f1d9a0779 diff --git a/fuzz/seeds/parse_frame/ef420abfddbda7b9ee665d85ef62e4a437554003 b/fuzz/seeds/parse_frame_header/ef420abfddbda7b9ee665d85ef62e4a437554003 similarity index 100% rename from fuzz/seeds/parse_frame/ef420abfddbda7b9ee665d85ef62e4a437554003 rename to fuzz/seeds/parse_frame_header/ef420abfddbda7b9ee665d85ef62e4a437554003 diff --git a/fuzz/seeds/parse_frame/fab3e168d56fecfc59f98a69f75673a53e94c215 b/fuzz/seeds/parse_frame_header/fab3e168d56fecfc59f98a69f75673a53e94c215 similarity index 100% rename from fuzz/seeds/parse_frame/fab3e168d56fecfc59f98a69f75673a53e94c215 rename to fuzz/seeds/parse_frame_header/fab3e168d56fecfc59f98a69f75673a53e94c215 From f4769a950cfbaac231e04a60d68683e28c4f6b0c Mon Sep 17 00:00:00 2001 From: Alexey Galakhov Date: Tue, 14 May 2019 02:10:51 +0200 Subject: [PATCH 3/4] example: trivial: init logger in server example Signed-off-by: Alexey Galakhov --- examples/server.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/examples/server.rs b/examples/server.rs index 86df2fe..d1b5d95 100644 --- a/examples/server.rs +++ b/examples/server.rs @@ -1,4 +1,5 @@ extern crate tungstenite; +extern crate env_logger; use std::thread::spawn; use std::net::TcpListener; @@ -7,6 +8,7 @@ use tungstenite::accept_hdr; use tungstenite::handshake::server::Request; fn main() { + env_logger::init(); let server = TcpListener::bind("127.0.0.1:3012").unwrap(); for stream in server.incoming() { spawn(move || { From 3fb359224ed5229dc1cf40d967dde7c1a1ed0778 Mon Sep 17 00:00:00 2001 From: Alexey Galakhov Date: Tue, 14 May 2019 10:16:27 +0200 Subject: [PATCH 4/4] Bump version Signed-off-by: Alexey Galakhov --- Cargo.toml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 1f432c4..0e240e8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -7,9 +7,9 @@ authors = ["Alexey Galakhov"] license = "MIT/Apache-2.0" readme = "README.md" homepage = "https://github.com/snapview/tungstenite-rs" -documentation = "https://docs.rs/tungstenite/0.7.0" +documentation = "https://docs.rs/tungstenite/0.8.0" repository = "https://github.com/snapview/tungstenite-rs" -version = "0.7.0" +version = "0.8.0" [features] default = ["tls"]