From 22730ea069cc04b0f68ed99f9e8df31bb8c2c98d Mon Sep 17 00:00:00 2001 From: Max Inden Date: Thu, 18 Jan 2024 12:37:36 +0100 Subject: [PATCH] fix(client): call stream_close_send after GET Previously `neqo-client` would not close the sending side of a stream after sending a GET request. Corresponding section in RFC 9114: > After sending a request, a client MUST close the stream for sending. https://www.rfc-editor.org/rfc/rfc9114.html#name-http-message-framing This surfaces in the Quic Interop Runner `http3` test. Here the client download 3 files from the server. 1. The client opens stream `0`, sends a GET for the first file. 2. The client opens stream `4`, sends a GET for the second file. 3. The client opens stream `8`, sends a GET for the third file. 4. ... 5. Eventually the client has read the whole response on stream `0`, [it removes the corresponding `StreamHandler` from `self.url_handler.stream_handlers`](https://github.com/mozilla/neqo/blob/64fb41f47cbbbb1101484dcd37533c7b295fa659/neqo-client/src/main.rs#L775-L777) and continues with the remaining requests. 6. Given that the client did not close the sending side of stream `0` after sending the GET request, it still handles `Http3ClientEvent::DataWritable` events for stream `0`. Given that it previously removed stream `0` from `self.url_handler.stream_handlers`, [it errors](https://github.com/mozilla/neqo/blob/64fb41f47cbbbb1101484dcd37533c7b295fa659/neqo-client/src/main.rs#L780-L784) and discontinues the [process_loop](https://github.com/mozilla/neqo/blob/64fb41f47cbbbb1101484dcd37533c7b295fa659/neqo-client/src/main.rs#L472-L474). 7. The second and third request don't finish and the Quic Interop Runner fails the test given that the second and third file are not fully downloaded. > File size of /tmp/download_sivy_1mt/cjwxjpvzjr doesn't match. Original: 10240 bytes, downloaded: 4056 bytes. --- neqo-client/src/main.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/neqo-client/src/main.rs b/neqo-client/src/main.rs index e204b179c..1ea52f8fb 100644 --- a/neqo-client/src/main.rs +++ b/neqo-client/src/main.rs @@ -499,10 +499,13 @@ impl StreamHandlerType { url: &Url, args: &Args, all_paths: &mut Vec, + client: &mut Http3Client, + client_stream_id: StreamId, ) -> Box { match handler_type { Self::Download => { let out_file = get_output_file(url, &args.output_dir, all_paths); + client.stream_close_send(client_stream_id).unwrap(); Box::new(DownloadStreamHandler { out_file }) } Self::Upload => Box::new(UploadStreamHandler { @@ -657,6 +660,8 @@ impl<'a> URLHandler<'a> { &url, self.args, &mut self.all_paths, + client, + client_stream_id, ); self.stream_handlers.insert(client_stream_id, handler); true