Skip to content

Commit

Permalink
fix(client): call stream_close_send after GET
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
mxinden committed Jan 18, 2024
1 parent 64fb41f commit 22730ea
Showing 1 changed file with 5 additions and 0 deletions.
5 changes: 5 additions & 0 deletions neqo-client/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -499,10 +499,13 @@ impl StreamHandlerType {
url: &Url,
args: &Args,
all_paths: &mut Vec<PathBuf>,
client: &mut Http3Client,
client_stream_id: StreamId,
) -> Box<dyn StreamHandler> {
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 {
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 22730ea

Please sign in to comment.