From d7757b21fa3fd8ac004392f86755ff3a1e6c2b46 Mon Sep 17 00:00:00 2001 From: "szymon.habrainski" Date: Sun, 7 Jun 2026 21:26:27 +0200 Subject: [PATCH] Return InvalidStreamState from stream_send() for collected streams When a stream has been completed and garbage collected (for example after the peer sends STOP_SENDING, the fin bit is received, and the data is drained via stream_recv()), a subsequent stream_send() call hits get_or_create() for an id that is in the collected set, which returns Error::Done. That error was forwarded unchanged, so callers saw Done (normally "no capacity") for a stream that no longer exists. Map that case to Error::InvalidStreamState instead, matching the behaviour of stream_recv() which already returns InvalidStreamState for a non-existent stream. This lets applications distinguish a lack of send capacity from an invalid stream id without an extra stream_writable() check. Fixes #1695 --- quiche/src/lib.rs | 4 ++++ quiche/src/tests.rs | 42 ++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/quiche/src/lib.rs b/quiche/src/lib.rs index 8c38eda4cf..6f2d05262e 100644 --- a/quiche/src/lib.rs +++ b/quiche/src/lib.rs @@ -6025,6 +6025,10 @@ impl Connection { return Err(Error::StreamLimit); }, + // Stream already collected; mirror stream_recv() and return + // InvalidStreamState. + Err(Error::Done) => return Err(Error::InvalidStreamState(stream_id)), + Err(e) => return Err(e), }; diff --git a/quiche/src/tests.rs b/quiche/src/tests.rs index f2546c95b9..345c3dc903 100644 --- a/quiche/src/tests.rs +++ b/quiche/src/tests.rs @@ -3472,6 +3472,38 @@ fn stop_sending_fin( assert_eq!(iter.next(), None); } +#[rstest] +/// Tests that stream_send() on a collected stream returns InvalidStreamState +/// rather than Done (issue #1695). +fn stream_send_after_stop_sending_and_fin( + #[values("cubic", "bbr2_gcongestion")] cc_algorithm_name: &str, +) { + let mut pipe = test_utils::Pipe::new(cc_algorithm_name).unwrap(); + assert_eq!(pipe.handshake(), Ok(())); + + // Server opens a bidi stream and sends data with fin. + assert_eq!(pipe.server.stream_send(5, b"hello", true), Ok(5)); + + // Server also sends STOP_SENDING, indicating it will not read from the + // client on this stream. + assert_eq!(pipe.server.stream_shutdown(5, Shutdown::Read, 42), Ok(())); + + // Exchange all packets: client receives the STOP_SENDING and STREAM(fin). + assert_eq!(pipe.advance(), Ok(())); + + // Client reads the data. The stream is now complete on both sides (recv + // finished, send stopped), so quiche collects it. + let mut buf = [0; 64]; + assert_eq!(pipe.client.stream_recv(5, &mut buf), Ok((5, true))); + + // The stream has been collected. stream_send() must return + // InvalidStreamState, not Done. + assert_eq!( + pipe.client.stream_send(5, b"world", false), + Err(Error::InvalidStreamState(5)), + ); +} + #[rstest] /// Tests that resetting a stream restores flow control for unsent data. fn stop_sending_unsent_tx_cap( @@ -5541,7 +5573,10 @@ fn collect_streams( assert!(pipe.client.stream_finished(0)); assert!(pipe.server.stream_finished(0)); - assert_eq!(pipe.client.stream_send(0, b"", true), Err(Error::Done)); + assert_eq!( + pipe.client.stream_send(0, b"", true), + Err(Error::InvalidStreamState(0)) + ); let frames = [frame::Frame::Stream { stream_id: 0, @@ -11812,7 +11847,10 @@ fn stop_sending_stream_send_after_reset_stream_ack( // If we called send before the client ACK of reset stream, it would // have failed with StreamStopped. - assert_eq!(pipe.server.stream_send(0, b"world", true), Err(Error::Done),); + assert_eq!( + pipe.server.stream_send(0, b"world", true), + Err(Error::InvalidStreamState(0)), + ); // Stream 0 is still not writable. let mut w = pipe.server.writable();