webtransport: `DataReadable` event on new unreadable stream
Given a WebTransport session where a server initiates a stream to the client, but does not (yet) write to it, the client will correctly emit a Http3ClientEvent::WebTransport(WebTransportEvent::NewSession(_)), but in addition incorrectly emit a Http3ClientEvent::DataReadable{..}.
More specifically in Http3Client::handle_stream_readable the client first reads from the new stream (base_handler.handle_stream_readable()), identifies it as a WebTransport stream, then reads from the stream again (base_handler.handle_stream_readable()). The latter call triggers a DataReadable event.
https://github.com/mozilla/neqo/blob/ea6cde436bcf67e71deba0809978312fbf3ee734/neqo-http3/src/connection_client.rs#L1070-L1091
I assume this is not causing issues in Firefox today, as DataReadable is a best-effort event, where downstream code ignores a consecutive zero read.
A fix should as well cover the case where the server session acceptance arrives after the new stream. See https://github.com/mozilla/neqo/pull/2806/files#r2300088780.
Reproducible via:
diff --git a/neqo-http3/src/features/extended_connect/tests/webtransport/mod.rs b/neqo-http3/src/features/extended_connect/tests/webtransport/mod.rs
index 8fb9892d..b7671efb 100644
--- a/neqo-http3/src/features/extended_connect/tests/webtransport/mod.rs
+++ b/neqo-http3/src/features/extended_connect/tests/webtransport/mod.rs
@@ -657,4 +657,9 @@ impl WtTest {
};
assert!(!self.server.events().any(wt_datagram_event));
}
+
+ fn check_no_data_readable_received_client(&mut self) {
+ let wt_stream_readable_event = |e| matches!(e, Http3ClientEvent::DataReadable { .. });
+ assert!(!self.client.events().any(wt_stream_readable_event));
+ }
}
diff --git a/neqo-http3/src/features/extended_connect/tests/webtransport/streams.rs b/neqo-http3/src/features/extended_connect/tests/webtransport/streams.rs
index 5f168b53..7110558b 100644
--- a/neqo-http3/src/features/extended_connect/tests/webtransport/streams.rs
+++ b/neqo-http3/src/features/extended_connect/tests/webtransport/streams.rs
@@ -81,6 +81,19 @@ fn wt_server_stream_uni() {
assert_eq!(recv_stats.bytes_read(), BUF_SERVER.len() as u64);
}
+#[test]
+fn no_write_thus_no_readable_event() {
+ let mut wt = WtTest::new();
+ let wt_session = wt.create_wt_session();
+
+ // Create stream, but don't send on it.
+ let stream = WtTest::create_wt_stream_server(&wt_session, StreamType::BiDi);
+ wt.exchange_packets();
+
+ // wt.recv_stream_stats(stream.stream_id()).unwrap();
+ wt.check_no_data_readable_received_client();
+}
+
#[test]
fn wt_server_stream_bidi() {
const BUF_CLIENT: &[u8] = &[0; 10];