neqo icon indicating copy to clipboard operation
neqo copied to clipboard

webtransport: `DataReadable` event on new unreadable stream

Open mxinden opened this issue 3 months ago • 0 comments

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];

mxinden avatar Aug 30 '25 08:08 mxinden