Skip to content

Commit

Permalink
http3: don't automatically set RoundTripper.QuicConfig.EnableDatagrams (
Browse files Browse the repository at this point in the history
quic-go#4340)

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.
  • Loading branch information
marten-seemann committed Mar 3, 2024
1 parent c786a46 commit 0405634
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 4 deletions.
5 changes: 4 additions & 1 deletion http3/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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 {
Expand Down
5 changes: 2 additions & 3 deletions http3/roundtrip.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
13 changes: 13 additions & 0 deletions http3/roundtrip_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down

0 comments on commit 0405634

Please sign in to comment.