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

Author: schiemon

quiche/src/lib.rs

@@ -6025,6 +6025,10 @@ impl<F: BufFactory> Connection<F> {
                 return Err(Error::StreamLimit);
             },
 
+            // Stream already collected; mirror stream_recv()'s
+            // InvalidStreamState.
+            Err(Error::Done) => return Err(Error::InvalidStreamState(stream_id)),
+
             Err(e) => return Err(e),
         };
 

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();