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.

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

http3: change code point for HTTP datagrams to RFC 9297 #3588

Merged
merged 2 commits into from Aug 9, 2023

Conversation

kokes
Copy link
Contributor

@kokes kokes commented Oct 11, 2022

I found a few references to HTTP/3 Datagrams that seem to be out of date. The referred doc has been superseeded by RFC 9297, so I updated the links and changed a few error/frame type values that have changed since the draft (see the draft where it says "note that this will switch to a lower value before publication").

I have not updated the README to say quic-go supports RFC 9297 as I don't know if it's the case, especially since I don't see any references to Quarter Stream IDs for instance.

@marten-seemann
Copy link
Member

I'm hesitant to merge this PR. First of all, we don't actually support this RFC. We support QUIC DATAGRAMs, but not HTTP/3 DATAGRAMs. Second, Chrome's WebTransport implementation does some datagram support detection (not sure if QUIC or H3), and refuses to establish a WebTransport connection if datagrams are disabled, so we'd have to be extra careful not to break things there.

@kokes
Copy link
Contributor Author

kokes commented Nov 4, 2022

Capsule support has been merged in #3607, just adding it here for context.

@marten-seemann
Copy link
Member

@kokes Sorry for the long delay. Can you please rebase?

@marten-seemann marten-seemann added this to the v0.38 milestone Aug 2, 2023
@kokes
Copy link
Contributor Author

kokes commented Aug 2, 2023

Rebased.

@marten-seemann marten-seemann changed the title HTTP/3 Datagrams are now RFC 9297 http3: change codepoint for HTTP datagrams to the RFC version Aug 2, 2023
@marten-seemann marten-seemann changed the title http3: change codepoint for HTTP datagrams to the RFC version http3: change code point for HTTP datagrams to RFC 9297 Aug 2, 2023
@codecov
Copy link

codecov bot commented Aug 2, 2023

Codecov Report

Merging #3588 (c362c50) into master (1c47ebe) will increase coverage by 0.04%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #3588      +/-   ##
==========================================
+ Coverage   82.80%   82.84%   +0.04%     
==========================================
  Files         147      147              
  Lines       14758    14758              
==========================================
+ Hits        12220    12226       +6     
+ Misses       2036     2031       -5     
+ Partials      502      501       -1     
Files Changed Coverage Δ
http3/error_codes.go 100.00% <ø> (ø)
http3/frames.go 84.91% <ø> (ø)
http3/roundtrip.go 83.12% <ø> (ø)
http3/server.go 77.58% <ø> (+0.66%) ⬆️

... and 1 file with indirect coverage changes

Copy link

@dongcarl dongcarl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I have a longer branch with a fully working RFC 9297 and this is what was needed to interop with hyper/h3 for RFC 9297.

@marten-seemann marten-seemann merged commit 05db808 into quic-go:master Aug 9, 2023
16 checks passed
mxinden added a commit to mxinden/rust-libp2p that referenced this pull request Aug 23, 2023
Our WASM Webtransport interoperability tests previously used Chrome 112. This Chrome version fails
to connect to go-libp2p with quic-go v0.38.0. See libp2p/go-libp2p#2506 for
failure. This is due to quic-go v0.38.0 moving to the updated code point for HTTP datagrams. See
quic-go/quic-go#3588 for details.

This commit upgrades our interop tests to use Chrome 115.
mergify bot pushed a commit to libp2p/rust-libp2p that referenced this pull request Aug 23, 2023
Our WASM Webtransport interoperability tests previously used Chrome 112. This Chrome version fails to connect to go-libp2p with quic-go v0.38.0. See libp2p/go-libp2p#2506 for failure. This is due to quic-go v0.38.0 moving to the updated code point for HTTP datagrams. See quic-go/quic-go#3588 for details.

This commit upgrades our interop tests to use Chrome 115.

Pull-Request: #4383.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants