Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(client): call stream_close_send after GET #1564

Merged
merged 1 commit into from
Jan 18, 2024
Merged

fix(client): call stream_close_send after GET #1564

merged 1 commit into from
Jan 18, 2024

Commits on Jan 18, 2024

  1. 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.
    @mxinden
    mxinden committed Jan 18, 2024
    Configuration menu
    Copy the full SHA
    22730ea View commit details
    Browse the repository at this point in the history