Repository: quic-go/quic-go
Pull request #4340 (2024-03-03 06:24:42)

http3: don't automatically set RoundTripper.QuicConfig.EnableDatagrams
If the user provides a quic.Config, we shouldn't modify it. Instead, we should return an error if the user enables HTTP Datagrams but fails to enable datagrams on the QUIC layer.

This isn't required by RFC 9297. If the QUIC layer disables datagrams, one could - in principe - use the Capsule protocol. However, that's probably never (?) intended by the user.

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/quic-go/quic-go/pull/4340?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 84.83%. Comparing base [(`c786a46`)](https://app.codecov.io/gh/quic-go/quic-go/commit/c786a46f42c577b6edf789046c16b70eee13caf5?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go) to head [(`22b2a14`)](https://app.codecov.io/gh/quic-go/quic-go/pull/4340?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go).


<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##           master    #4340      +/-   ##
==========================================
+ Coverage   84.80%   84.83%   +0.03%     
==========================================
  Files         150      150              
  Lines       14251    14254       +3     
==========================================
+ Hits        12085    12092       +7     
+ Misses       1668     1665       -3     
+ Partials      498      497       -1     
```



</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/quic-go/quic-go/pull/4340?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go).


Patch:
diff --git a/http3/client.go b/http3/client.go
index e6e3d68a..61c153c1 100644
--- a/http3/client.go
+++ b/http3/client.go
@@ -73,6 +73,10 @@ var _ roundTripCloser = &client{}
 func newClient(hostname string, tlsConf *tls.Config, opts *roundTripperOpts, conf *quic.Config, dialer dialFunc) (roundTripCloser, error) {
 	if conf == nil {
 		conf = defaultQuicConfig.Clone()
+		conf.EnableDatagrams = opts.EnableDatagram
+	}
+	if opts.EnableDatagram && !conf.EnableDatagrams {
+		return nil, errors.New("HTTP Datagrams enabled, but QUIC Datagrams disabled")
 	}
 	if len(conf.Versions) == 0 {
 		conf = conf.Clone()
@@ -84,7 +88,6 @@ func newClient(hostname string, tlsConf *tls.Config, opts *roundTripperOpts, con
 	if conf.MaxIncomingStreams == 0 {
 		conf.MaxIncomingStreams = -1 // don't allow any bidirectional streams
 	}
-	conf.EnableDatagrams = opts.EnableDatagram
 	logger := utils.DefaultLogger.WithPrefix("h3 client")
 
 	if tlsConf == nil {
diff --git a/http3/roundtrip.go b/http3/roundtrip.go
index d86b0bb2..86d3d3d0 100644
--- a/http3/roundtrip.go
+++ b/http3/roundtrip.go
@@ -50,9 +50,8 @@ type RoundTripper struct {
 	// If nil, reasonable default values will be used.
 	QuicConfig *quic.Config
 
-	// Enable support for HTTP/3 datagrams.
-	// If set to true, QuicConfig.EnableDatagram will be set.
-	// See https://datatracker.ietf.org/doc/html/rfc9297.
+	// Enable support for HTTP/3 datagrams (RFC 9297).
+	// If a QuicConfig is set, datagram support also needs to be enabled on the QUIC layer by setting EnableDatagrams.
 	EnableDatagrams bool
 
 	// Additional HTTP/3 settings.


Test patch:
diff --git a/http3/roundtrip_test.go b/http3/roundtrip_test.go
index 8b59a8bb..1ab767c9 100644
--- a/http3/roundtrip_test.go
+++ b/http3/roundtrip_test.go
@@ -99,6 +99,19 @@ var _ = Describe("RoundTripper", func() {
 			Expect(receivedConfig.HandshakeIdleTimeout).To(Equal(config.HandshakeIdleTimeout))
 		})
 
+		It("requires quic.Config.EnableDatagram if HTTP datagrams are enabled", func() {
+			rt.QuicConfig = &quic.Config{EnableDatagrams: false}
+			rt.Dial = func(_ context.Context, _ string, _ *tls.Config, config *quic.Config) (quic.EarlyConnection, error) {
+				return nil, errors.New("handshake error")
+			}
+			rt.EnableDatagrams = true
+			_, err := rt.RoundTrip(req)
+			Expect(err).To(MatchError("HTTP Datagrams enabled, but QUIC Datagrams disabled"))
+			rt.QuicConfig.EnableDatagrams = true
+			_, err = rt.RoundTrip(req)
+			Expect(err).To(MatchError("handshake error"))
+		})
+
 		It("uses the custom dialer, if provided", func() {
 			var dialed bool
 			dialer := func(_ context.Context, _ string, tlsCfgP *tls.Config, cfg *quic.Config) (quic.EarlyConnection, error) {