From 596dd20bcf3ae781ce07ce7c2471c5b9c950b5f0 Mon Sep 17 00:00:00 2001 From: Nils Andreas Svee Date: Sun, 4 Oct 2026 13:35:37 +0700 Subject: [PATCH] fix(client): validate blocking receive lifecycle before socket I/O Blocking receives previously checked lifecycle only after a datagram arrived, allowing invalid operations to block or return empty success after timeout. Reuse session validation before socket access and cover connected, closed, and no-test-completed states through public APIs. --- crates/irtt-client/src/client.rs | 5 ++++ crates/irtt-client/src/session/machine.rs | 1 - crates/irtt-client/tests/client/close.rs | 35 ++++++++++++++++++++-- crates/irtt-client/tests/client/no_test.rs | 17 ++++++++++- crates/irtt-client/tests/client/open.rs | 26 ++++++++++++++++ 5 files changed, 80 insertions(+), 4 deletions(-) diff --git a/crates/irtt-client/src/client.rs b/crates/irtt-client/src/client.rs index 5ef9b03..063e08b 100644 --- a/crates/irtt-client/src/client.rs +++ b/crates/irtt-client/src/client.rs @@ -222,7 +222,9 @@ impl Client { /// /// Returns an empty event list when the socket read would block or times /// out. Malformed or unrelated datagrams are reported as warning events. + /// Requires an open session; lifecycle errors are returned before socket I/O. pub fn recv_once(&mut self) -> Result, ClientError> { + self.runtime.ensure_open()?; self.socket .set_read_timeout(self.runtime.config().socket_config.recv_timeout)?; self.recv_once_inner() @@ -316,7 +318,10 @@ impl Client { /// Receive and classify datagrams until a receive produces no events or the /// receive budget is exhausted. + /// + /// Requires an open session; lifecycle errors are returned before socket I/O. pub fn recv_available(&mut self, budget: RecvBudget) -> Result, ClientError> { + self.runtime.ensure_open()?; self.socket .set_read_timeout(self.runtime.config().socket_config.recv_timeout)?; let mut all_events = Vec::new(); diff --git a/crates/irtt-client/src/session/machine.rs b/crates/irtt-client/src/session/machine.rs index 3c74dd0..0441ad3 100644 --- a/crates/irtt-client/src/session/machine.rs +++ b/crates/irtt-client/src/session/machine.rs @@ -591,7 +591,6 @@ impl SessionMachine { } } - #[cfg(feature = "tokio")] pub(crate) fn ensure_open(&self) -> Result<(), ClientError> { self.open_session().map(|_| ()) } diff --git a/crates/irtt-client/tests/client/close.rs b/crates/irtt-client/tests/client/close.rs index 128172c..9b70f5c 100644 --- a/crates/irtt-client/tests/client/close.rs +++ b/crates/irtt-client/tests/client/close.rs @@ -1,7 +1,7 @@ use super::*; #[test] -fn open_fails_after_close() { +fn operations_fail_after_local_close_without_datagrams() { let params = default_params(); let server = start_fake_server(move |socket, tx| { let (_, peer) = recv_request(&socket, &tx); @@ -9,10 +9,26 @@ fn open_fails_after_close() { socket.send_to(&reply, peer).unwrap(); let _ = recv_request(&socket, &tx); }); - let mut client = Client::connect(default_test_config(server.addr)).unwrap(); + let mut config = default_test_config(server.addr); + config.socket_config.recv_timeout = Some(Duration::from_millis(50)); + let mut client = Client::connect(config).unwrap(); assert_open_started(client.open().unwrap()); client.close().unwrap(); assert!(matches!(client.open(), Err(ClientError::AlreadyClosed))); + let results = [ + client.recv_once(), + client.recv_available(RecvBudget { max_packets: 1 }), + ]; + assert!( + matches!( + results, + [ + Err(ClientError::AlreadyClosed), + Err(ClientError::AlreadyClosed) + ] + ), + "unexpected receive results: {results:?}" + ); server.join(); } @@ -473,6 +489,21 @@ fn close_flagged_echo_reply_emits_reply_then_closes_without_sending_close() { )); assert_eq!(events.len(), 2); + let results = [ + client.recv_once(), + client.recv_available(RecvBudget { max_packets: 1 }), + ]; + assert!( + matches!( + results, + [ + Err(ClientError::AlreadyClosed), + Err(ClientError::AlreadyClosed) + ] + ), + "unexpected receive results: {results:?}" + ); + assert!(matches!( client.send_probe(), Err(ClientError::AlreadyClosed) diff --git a/crates/irtt-client/tests/client/no_test.rs b/crates/irtt-client/tests/client/no_test.rs index b90f0b7..2ba24cf 100644 --- a/crates/irtt-client/tests/client/no_test.rs +++ b/crates/irtt-client/tests/client/no_test.rs @@ -101,14 +101,29 @@ fn send_probe_fails_after_no_test_completed() { } #[test] -fn open_fails_after_no_test_completed() { +fn operations_fail_after_no_test_completed_without_datagrams() { let mut config = default_test_config(SocketAddr::from(([127, 0, 0, 1], 1))); config.run_mode = RunMode::NoTest; + config.socket_config.recv_timeout = Some(Duration::from_millis(50)); let params = default_params(); let server = no_test_server(params, 0); config.server_addr = server.addr.to_string(); let mut client = Client::connect(config).unwrap(); assert_no_test_completed(client.open().unwrap()); assert!(matches!(client.open(), Err(ClientError::AlreadyCompleted))); + let results = [ + client.recv_once(), + client.recv_available(RecvBudget { max_packets: 1 }), + ]; + assert!( + matches!( + results, + [ + Err(ClientError::AlreadyCompleted), + Err(ClientError::AlreadyCompleted) + ] + ), + "unexpected receive results: {results:?}" + ); server.join(); } diff --git a/crates/irtt-client/tests/client/open.rs b/crates/irtt-client/tests/client/open.rs index d6f47cd..456fd82 100644 --- a/crates/irtt-client/tests/client/open.rs +++ b/crates/irtt-client/tests/client/open.rs @@ -1,5 +1,31 @@ use super::*; +#[test] +fn receive_before_open_returns_not_open_without_datagrams() { + let peer = std::net::UdpSocket::bind("127.0.0.1:0").unwrap(); + let mut config = default_test_config(peer.local_addr().unwrap()); + // Bound the old implementation's blocking receive without sending a packet. + config.socket_config.recv_timeout = Some(Duration::from_millis(50)); + let mut client = Client::connect(config).unwrap(); + + let results = [ + client.recv_once(), + client.recv_available(RecvBudget { max_packets: 1 }), + client.recv_available(RecvBudget { max_packets: 0 }), + ]; + assert!( + matches!( + results, + [ + Err(ClientError::NotOpen), + Err(ClientError::NotOpen), + Err(ClientError::NotOpen) + ] + ), + "unexpected receive results: {results:?}" + ); +} + // Untrusted returned intervals must be rejected before managed scheduling, // even when loose negotiation permits legitimate server restrictions. #[test]