Skip to content

Commit 5a920b1

Browse files
neildgopherbot
neild
authored and
gopherbot
committed
http3: rework registration to allow using a fake network
This is the x/net half of a paired set of changes which modify the mechanism by which x/net/http3 integrates with net/http. The other half of the change is CL 803380. x/net/http3 exports two functions via linkname for use by net/http's tests. These are now: registerServer(*http.Server, opts any) error registerTransport(*http.Transport, opts any) error These are now thin wrappers around the interal/http3 registration functions, with a bit of reflection-based struct copying to permit passing in options without exporting the options types. (The end state is for these functions to be exported and take exported options types.) Server registration no longer uses TLSNextProto. Instead, the HTTP/3 server registers itself via net/http.Server.Serve, which is the same approach used by x/net/http2. This gives us a simpler link between the two packages. For example, net/http can just call a Shutdown method on the HTTP/3 server and pass it a Context, rather than the prior complex dance. The HTTP/3 server no longer calls net.Listen. It always accepts a net.PacketConn from net/http. TransportOpts now has a ListenPacket field, in addition to ListenQUIC. Server and Transport now both accept their *tls.Configs from net/http. net/http is responsible for correctly configuring NextProtos. This makes TLS config construction more consistent overall, since net/http already sets up the HTTP/1 and HTTP/2 configs. These changes simplify the registration machinery in some places, but mainly permit us to now pass a fake net.PacketConn from net/http's tests into the http package. As part of this change, the HTTP/3 server's handler and base context are now per-endpoint rather than per-server. net/http Servers permit setting a per-net.Listener base context for request handlers. Treating the HTTP/3 base context as per-endpoint maintains the equivalent ability. Making the handler per-endpoint isn't strictly necessary, but is consistent with the context and simplifies the connection between net/http and x/net/http3. For golang/go#80480 Change-Id: I10d6e998c11cade33e517d1d2f03a2d86a6a6964 Reviewed-on: https://go-review.googlesource.com/c/net/+/801940 Reviewed-by: Nicholas Husin <husin@google.com> Reviewed-by: Nicholas Husin <nsh@golang.org> Auto-Submit: Damien Neil <dneil@google.com> LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
1 parent 7fd2842 commit 5a920b1

7 files changed

Lines changed: 197 additions & 206 deletions

File tree

‎http3/http3.go‎

Lines changed: 44 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -5,42 +5,57 @@
55
package http3
66

77
import (
8+
"fmt"
89
"net/http"
9-
"time"
10+
"reflect"
1011
_ "unsafe" // for linkname
1112

1213
. "golang.org/x/net/internal/http3"
13-
"golang.org/x/net/quic"
1414
)
1515

16-
// Be extra generous with the handshake timeout. On some builders, the default
17-
// handshake timeout seems to be insufficient, causing rare test flakes.
18-
const handshakeTimeout = 1 * time.Minute
16+
//go:linkname registerServer net/http_test.registerHTTP3Server
17+
func registerServer(s *http.Server, opts any) error {
18+
var o ServerOpts
19+
if err := shallowStructCopy(&o, opts); err != nil {
20+
return err
21+
}
22+
return RegisterServer(s, o)
23+
}
1924

20-
//go:linkname registerHTTP3Server net/http_test.registerHTTP3Server
21-
func registerHTTP3Server(s *http.Server) <-chan *quic.Endpoint {
22-
endpointCh := make(chan *quic.Endpoint)
23-
RegisterServer(s, ServerOpts{
24-
ListenQUIC: func(addr string, config *quic.Config) (*quic.Endpoint, error) {
25-
e, err := quic.Listen("udp", addr, config)
26-
endpointCh <- e
27-
return e, err
28-
},
29-
QUICConfig: &quic.Config{HandshakeTimeout: handshakeTimeout},
30-
})
31-
return endpointCh
25+
//go:linkname registerTransport net/http_test.registerHTTP3Transport
26+
func registerTransport(tr *http.Transport, opts any) error {
27+
var o TransportOpts
28+
if err := shallowStructCopy(&o, opts); err != nil {
29+
return err
30+
}
31+
return RegisterTransport(tr, o)
3232
}
3333

34-
//go:linkname registerHTTP3Transport net/http_test.registerHTTP3Transport
35-
func registerHTTP3Transport(tr *http.Transport) <-chan *quic.Endpoint {
36-
endpointCh := make(chan *quic.Endpoint)
37-
RegisterTransport(tr, TransportOpts{
38-
ListenQUIC: func(addr string, config *quic.Config) (*quic.Endpoint, error) {
39-
e, err := quic.Listen("udp", addr, config)
40-
endpointCh <- e
41-
return e, err
42-
},
43-
QUICConfig: &quic.Config{HandshakeTimeout: handshakeTimeout},
44-
})
45-
return endpointCh
34+
// shallowStructCopy copies every field in src to *dst.
35+
// src must be a struct, and dst must be a pointer to a struct.
36+
//
37+
// We use this to let net/http tests pass their own version of an options struct to
38+
// registerHTTP3{Server,Transport}.
39+
//
40+
// This is a temporary measure pending this package having a public API,
41+
// at which time net/http tests can use the ServerOpts and TransportOpts directly.
42+
func shallowStructCopy(dst, src any) error {
43+
dv := reflect.ValueOf(dst).Elem()
44+
sv := reflect.ValueOf(src)
45+
for i := range sv.Type().NumField() {
46+
stype := sv.Type().Field(i)
47+
if !stype.IsExported() {
48+
return fmt.Errorf("%T contains unexported fields", src)
49+
}
50+
sf := sv.Field(i)
51+
df := dv.FieldByName(stype.Name)
52+
if !df.CanSet() {
53+
return fmt.Errorf("%T.%v: field does not exist or is unassignable", dst, stype.Name)
54+
}
55+
if !sf.Type().AssignableTo(df.Type()) {
56+
return fmt.Errorf("%T.%v: %v is not assignable to %v", dst, stype.Name, sf.Type(), df.Type())
57+
}
58+
df.Set(sf)
59+
}
60+
return nil
4661
}

‎internal/http3/nethttp_test.go‎

Lines changed: 31 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"context"
1111
"crypto/tls"
1212
"io"
13+
"net"
1314
"net/http"
1415
"slices"
1516
"testing"
@@ -19,7 +20,6 @@ import (
1920

2021
"golang.org/x/net/internal/http3"
2122
"golang.org/x/net/internal/testcert"
22-
"golang.org/x/net/quic"
2323
)
2424

2525
//go:linkname protocolSetHTTP3
@@ -51,40 +51,40 @@ func TestNetHTTPIntegration(t *testing.T) {
5151
Handler: handler,
5252
TLSConfig: newTestTLSConfig(),
5353
}
54+
if err := http3.RegisterServer(srv, http3.ServerOpts{}); err != nil {
55+
t.Skipf("cannot register server: %v", err)
56+
}
5457
srv.Protocols = &http.Protocols{}
5558
protocolSetHTTP3(srv.Protocols)
5659

57-
var listenAddr string
58-
listenAddrSet := make(chan any)
59-
http3.RegisterServer(srv, http3.ServerOpts{
60-
ListenQUIC: func(addr string, config *quic.Config) (*quic.Endpoint, error) {
61-
e, err := quic.Listen("udp", addr, config)
62-
listenAddr = e.LocalAddr().String()
63-
listenAddrSet <- struct{}{}
64-
return e, err
65-
},
66-
})
67-
go func() {
68-
if err := srv.ListenAndServeTLS("", ""); err != nil {
69-
panic(err)
70-
}
71-
}()
60+
// We do not yet have a public API for serving on a system-chosen port
61+
// that lets us find out what that port is. (ListenAndServeTLS will listen on
62+
// a system-chosen port, but we can't find out what port it picked.)
63+
// So use ServeTLS with an pro tem mechanism for passing in a PacketConn.
64+
nc, err := net.ListenPacket("udp", srv.Addr)
65+
if err != nil {
66+
t.Fatal(err)
67+
}
68+
defer nc.Close()
69+
go srv.ServeTLS(http3ServeConn{nc}, "", "")
7270

7371
tr := &http.Transport{TLSClientConfig: newTestTLSConfig()}
7472
tr.Protocols = &http.Protocols{}
7573
protocolSetHTTP3(tr.Protocols)
76-
http3.RegisterTransport(tr, http3.TransportOpts{})
74+
if err := http3.RegisterTransport(tr, http3.TransportOpts{}); err != nil {
75+
// If RegisterServer above succeeded, this should as well.
76+
t.Fatalf("cannot register transport: %v", err)
77+
}
7778

7879
client := &http.Client{
7980
Transport: tr,
8081
// Be extra generous with the timeout, to account for smaller builders
8182
// that we use for e.g. plan9.
8283
Timeout: 5 * time.Second,
8384
}
84-
<-listenAddrSet
8585

8686
for range 5 {
87-
req, err := http.NewRequest("GET", "https://"+listenAddr, nil)
87+
req, err := http.NewRequest("GET", "https://"+nc.LocalAddr().String(), nil)
8888
if err != nil {
8989
t.Fatal(err)
9090
}
@@ -118,3 +118,15 @@ func TestNetHTTPIntegration(t *testing.T) {
118118
t.Fatal(err)
119119
}
120120
}
121+
122+
type http3ServeConn struct {
123+
conn net.PacketConn
124+
}
125+
126+
func (c http3ServeConn) Accept() (net.Conn, error) { return nil, net.ErrClosed }
127+
func (c http3ServeConn) Close() error { return nil }
128+
func (c http3ServeConn) Addr() net.Addr { return nil }
129+
130+
func (c http3ServeConn) HTTP3PacketConn() net.PacketConn {
131+
return c.conn
132+
}

‎internal/http3/quic.go‎

Lines changed: 9 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -6,35 +6,23 @@ package http3
66

77
import (
88
"crypto/tls"
9+
"slices"
910

1011
"golang.org/x/net/quic"
1112
)
1213

13-
func initConfig(config *quic.Config) *quic.Config {
14+
func newQUICConfig(config *quic.Config, tlsConfig *tls.Config) *quic.Config {
15+
config = config.Clone()
1416
if config == nil {
1517
config = &quic.Config{}
1618
}
17-
18-
// maybeCloneTLSConfig clones the user-provided tls.Config (but only once)
19-
// prior to us modifying it.
20-
needCloneTLSConfig := true
21-
maybeCloneTLSConfig := func() *tls.Config {
22-
if needCloneTLSConfig {
23-
config.TLSConfig = config.TLSConfig.Clone()
24-
needCloneTLSConfig = false
19+
if !slices.Equal(tlsConfig.NextProtos, []string{"h3"}) {
20+
tlsConfig = tlsConfig.Clone()
21+
if tlsConfig == nil {
22+
tlsConfig = &tls.Config{}
2523
}
26-
return config.TLSConfig
27-
}
28-
29-
if config.TLSConfig == nil {
30-
config.TLSConfig = &tls.Config{}
31-
needCloneTLSConfig = false
32-
}
33-
if config.TLSConfig.MinVersion == 0 {
34-
maybeCloneTLSConfig().MinVersion = tls.VersionTLS13
35-
}
36-
if config.TLSConfig.NextProtos == nil {
37-
maybeCloneTLSConfig().NextProtos = []string{"h3"}
24+
tlsConfig.NextProtos = []string{"h3"}
3825
}
26+
config.TLSConfig = tlsConfig
3927
return config
4028
}

0 commit comments

Comments
 (0)