Skip to content

Commit 044aae0

Browse files
committed
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
1 parent f0c7193 commit 044aae0

3 files changed

Lines changed: 70 additions & 4 deletions

File tree

quiche/src/lib.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6025,6 +6025,10 @@ impl<F: BufFactory> Connection<F> {
60256025
return Err(Error::StreamLimit);
60266026
},
60276027

6028+
// Stream already collected; mirror stream_recv()'s
6029+
// InvalidStreamState.
6030+
Err(Error::Done) => return Err(Error::InvalidStreamState(stream_id)),
6031+
60286032
Err(e) => return Err(e),
60296033
};
60306034

quiche/src/tests.rs

Lines changed: 41 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3472,6 +3472,38 @@ fn stop_sending_fin(
34723472
assert_eq!(iter.next(), None);
34733473
}
34743474

3475+
#[rstest]
3476+
/// Tests that stream_send() on a collected stream returns InvalidStreamState
3477+
/// rather than Done (issue #1695).
3478+
fn stream_send_after_stop_sending_and_fin(
3479+
#[values("cubic", "bbr2_gcongestion")] cc_algorithm_name: &str,
3480+
) {
3481+
let mut pipe = test_utils::Pipe::new(cc_algorithm_name).unwrap();
3482+
assert_eq!(pipe.handshake(), Ok(()));
3483+
3484+
// Server opens a bidi stream and sends data with fin.
3485+
assert_eq!(pipe.server.stream_send(5, b"hello", true), Ok(5));
3486+
3487+
// Server also sends STOP_SENDING, indicating it will not read from the
3488+
// client on this stream.
3489+
assert_eq!(pipe.server.stream_shutdown(5, Shutdown::Read, 42), Ok(()));
3490+
3491+
// Exchange all packets: client receives the STOP_SENDING and STREAM(fin).
3492+
assert_eq!(pipe.advance(), Ok(()));
3493+
3494+
// Client reads the data. The stream is now complete on both sides (recv
3495+
// finished, send stopped), so quiche collects it.
3496+
let mut buf = [0; 64];
3497+
assert_eq!(pipe.client.stream_recv(5, &mut buf), Ok((5, true)));
3498+
3499+
// The stream has been collected. stream_send() must return
3500+
// InvalidStreamState, not Done.
3501+
assert_eq!(
3502+
pipe.client.stream_send(5, b"world", false),
3503+
Err(Error::InvalidStreamState(5)),
3504+
);
3505+
}
3506+
34753507
#[rstest]
34763508
/// Tests that resetting a stream restores flow control for unsent data.
34773509
fn stop_sending_unsent_tx_cap(
@@ -5541,7 +5573,10 @@ fn collect_streams(
55415573
assert!(pipe.client.stream_finished(0));
55425574
assert!(pipe.server.stream_finished(0));
55435575

5544-
assert_eq!(pipe.client.stream_send(0, b"", true), Err(Error::Done));
5576+
assert_eq!(
5577+
pipe.client.stream_send(0, b"", true),
5578+
Err(Error::InvalidStreamState(0))
5579+
);
55455580

55465581
let frames = [frame::Frame::Stream {
55475582
stream_id: 0,
@@ -11810,9 +11845,11 @@ fn stop_sending_stream_send_after_reset_stream_ack(
1181011845
assert_eq!(w.len(), 9);
1181111846
assert!(!w.any(|s| s == 0));
1181211847

11813-
// If we called send before the client ACK of reset stream, it would
11814-
// have failed with StreamStopped.
11815-
assert_eq!(pipe.server.stream_send(0, b"world", true), Err(Error::Done),);
11848+
// Stream 0 has been collected; stream_send must return InvalidStreamState.
11849+
assert_eq!(
11850+
pipe.server.stream_send(0, b"world", true),
11851+
Err(Error::InvalidStreamState(0)),
11852+
);
1181611853

1181711854
// Stream 0 is still not writable.
1181811855
let mut w = pipe.server.writable();
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
thread 'main' panicked at library/std/src/io/stdio.rs:1019:9:
2+
failed printing to stderr: Broken pipe (os error 32)
3+
stack backtrace:
4+
0: 0x101a9a57c - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::hd9028bcf6e51ebaa
5+
1: 0x101aee060 - core::fmt::write::h55bdf6c61e182108
6+
2: 0x101a8fcd8 - std::io::Write::write_fmt::hda38b67d64b411f4
7+
3: 0x101a9a3bc - std::sys_common::backtrace::print::h495b27ca64004324
8+
4: 0x101a9d1bc - std::panicking::panic_hook_with_disk_dump::{{closure}}::hf57b42f9c3e59ef7
9+
5: 0x101a9ce74 - std::panicking::panic_hook_with_disk_dump::h135d0abfa60c8e17
10+
6: 0x109abaf90 - rustc_driver_impl[df94399f4d5d78c1]::install_ice_hook::{closure#0}
11+
7: 0x101a9d9a4 - std::panicking::rust_panic_with_hook::ha2501c1cf653f30c
12+
8: 0x101a9d774 - std::panicking::begin_panic_handler::{{closure}}::h40334808c318e5be
13+
9: 0x101a9aa18 - std::sys_common::backtrace::__rust_end_short_backtrace::heebf5ae015c51044
14+
10: 0x101a9d4e0 - _rust_begin_unwind
15+
11: 0x101b1900c - core::panicking::panic_fmt::hb27f9126bf124657
16+
12: 0x101a8e9e8 - std::io::stdio::_eprint::hae02168798538215
17+
13: 0x1008816c0 - <rustfmt_nightly[6e674ee5e038b084]::config::Config>::from_toml_path
18+
14: 0x100883198 - <rustfmt_nightly[6e674ee5e038b084]::config::Config>::from_resolved_toml_path
19+
15: 0x100867aa0 - rustfmt_nightly[6e674ee5e038b084]::config::load_config::<rustfmt[a3b2e46fc0dbbe4f]::GetOptsOptions>
20+
16: 0x10085d0ac - rustfmt[a3b2e46fc0dbbe4f]::execute
21+
17: 0x10085b930 - rustfmt[a3b2e46fc0dbbe4f]::main
22+
18: 0x100869494 - std[c2739d50c071c0d9]::sys_common::backtrace::__rust_begin_short_backtrace::<fn(), ()>
23+
19: 0x10086c030 - std[c2739d50c071c0d9]::rt::lang_start::<()>::{closure#0}
24+
20: 0x101a826f4 - std::rt::lang_start_internal::h16f195600b3e1cf0
25+
21: 0x10085eaf0 - _main

0 commit comments

Comments
 (0)