Skip to content

Commit ad947e8

Browse files
committed
Attempt to workaround golang/go#59690
1 parent 42281ab commit ad947e8

File tree

3 files changed

+76
-47
lines changed

3 files changed

+76
-47
lines changed

api/auth.go

Lines changed: 31 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,7 @@ import (
55
"net/http"
66
)
77

8-
type canceler interface {
9-
CancelRequest(*http.Request)
10-
}
11-
12-
// authenticatedTransport manages injection of the API token
8+
// authenticatedTransport manages injection of the API token.
139
type authenticatedTransport struct {
1410
// The Token used for authentication. This can either the be
1511
// organizations registration token, or the agents access token.
@@ -19,19 +15,45 @@ type authenticatedTransport struct {
1915
Delegate http.RoundTripper
2016
}
2117

22-
// RoundTrip invoked each time a request is made
18+
// RoundTrip invoked each time a request is made.
2319
func (t authenticatedTransport) RoundTrip(req *http.Request) (*http.Response, error) {
2420
if t.Token == "" {
2521
return nil, fmt.Errorf("Invalid token, empty string supplied")
2622
}
2723

24+
// Per net/http#RoundTripper:
25+
//
26+
// "RoundTrip should not modify the request, except for
27+
// consuming and closing the Request's Body. RoundTrip may
28+
// read fields of the request in a separate goroutine. Callers
29+
// should not mutate or reuse the request until the Response's
30+
// Body has been closed."
31+
//
32+
// But we can pass a _different_ request to t.Delegate.RoundTrip.
33+
// req.Clone does a sufficiently deep clone (including Header which we
34+
// modify).
35+
req = req.Clone(req.Context())
2836
req.Header.Set("Authorization", fmt.Sprintf("Token %s", t.Token))
2937

3038
return t.Delegate.RoundTrip(req)
3139
}
3240

33-
// CancelRequest cancels an in-flight request by closing its connection.
41+
// CancelRequest forwards the call to t.Delegate, if it implements CancelRequest
42+
// itself.
3443
func (t *authenticatedTransport) CancelRequest(req *http.Request) {
35-
cancelableTransport := t.Delegate.(canceler)
36-
cancelableTransport.CancelRequest(req)
44+
canceler, ok := t.Delegate.(interface{ CancelRequest(*http.Request) })
45+
if !ok {
46+
return
47+
}
48+
canceler.CancelRequest(req)
49+
}
50+
51+
// CloseIdleConnections forwards the call to t.Delegate, if it implements
52+
// CloseIdleConnections itself.
53+
func (t *authenticatedTransport) CloseIdleConnections() {
54+
closer, ok := t.Delegate.(interface{ CloseIdleConnections() })
55+
if !ok {
56+
return
57+
}
58+
closer.CloseIdleConnections()
3759
}

api/client.go

Lines changed: 44 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import (
1010
"errors"
1111
"fmt"
1212
"io"
13-
"net"
1413
"net/http"
1514
"net/http/httptrace"
1615
"net/http/httputil"
@@ -83,46 +82,54 @@ func NewClient(l logger.Logger, conf Config) *Client {
8382

8483
httpClient := conf.HTTPClient
8584

86-
if conf.HTTPClient == nil {
87-
if conf.DisableHTTP2 {
88-
tr := &http.Transport{
89-
Proxy: http.ProxyFromEnvironment,
90-
DisableCompression: false,
91-
DisableKeepAlives: false,
92-
DialContext: (&net.Dialer{
93-
Timeout: 30 * time.Second,
94-
KeepAlive: 30 * time.Second,
95-
}).DialContext,
96-
MaxIdleConns: 100,
97-
IdleConnTimeout: 90 * time.Second,
98-
TLSHandshakeTimeout: 30 * time.Second,
99-
TLSNextProto: make(map[string]func(authority string, c *tls.Conn) http.RoundTripper),
100-
}
101-
httpClient = &http.Client{
102-
Timeout: 60 * time.Second,
103-
Transport: &authenticatedTransport{
104-
Token: conf.Token,
105-
Delegate: tr,
106-
},
107-
}
108-
} else {
109-
tr := &http2.Transport{
110-
ReadIdleTimeout: 30 * time.Second,
111-
}
112-
if conf.TLSConfig != nil {
113-
tr.TLSClientConfig = conf.TLSConfig
114-
}
85+
if httpClient != nil {
86+
return &Client{
87+
logger: l,
88+
client: httpClient,
89+
conf: conf,
90+
}
91+
}
11592

116-
httpClient = &http.Client{
117-
Timeout: 60 * time.Second,
118-
Transport: &authenticatedTransport{
119-
Token: conf.Token,
120-
Delegate: tr,
121-
},
122-
}
93+
// Base any modifications on the default transport.
94+
transport := http.DefaultTransport.(*http.Transport).Clone()
95+
// Allow override of TLSConfig. This must be set prior to calling
96+
// http2.ConfigureTransports.
97+
if conf.TLSConfig != nil {
98+
transport.TLSClientConfig = conf.TLSConfig
99+
}
100+
101+
if conf.DisableHTTP2 {
102+
transport.TLSNextProto = make(map[string]func(string, *tls.Conn) http.RoundTripper)
103+
// The default TLSClientConfig has h2 in NextProtos, so the
104+
// negotiated TLS connection will assume h2 support.
105+
// see https://github.com/golang/go/issues/50571
106+
transport.TLSClientConfig.NextProtos = []string{"http/1.1"}
107+
} else {
108+
// There is a bug in http2 on Linux regarding using dead connections.
109+
// This is a workaround. See https://github.com/golang/go/issues/59690
110+
//
111+
// Note that http2.ConfigureTransports alters its argument in order to
112+
// supply http2 functionality, and the http2.Transport does not support
113+
// HTTP/1.1 as a protocol, so we get slightly odd-looking code where
114+
// we use `transport` later on instead of the just-returned `tr2`.
115+
// tr2 is needed merely to configure the http2 option.
116+
tr2, err := http2.ConfigureTransports(transport)
117+
if err != nil {
118+
l.Warn("Failed to configure HTTP2 transports: %v", err)
119+
}
120+
if tr2 != nil {
121+
tr2.ReadIdleTimeout = 30 * time.Second
123122
}
124123
}
125124

125+
httpClient = &http.Client{
126+
Timeout: 60 * time.Second,
127+
Transport: &authenticatedTransport{
128+
Token: conf.Token,
129+
Delegate: transport,
130+
},
131+
}
132+
126133
return &Client{
127134
logger: l,
128135
client: httpClient,

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ require (
5050
go.opentelemetry.io/otel/trace v1.30.0
5151
golang.org/x/crypto v0.27.0
5252
golang.org/x/exp v0.0.0-20231108232855-2478ac86f678
53+
golang.org/x/net v0.29.0
5354
golang.org/x/oauth2 v0.23.0
5455
golang.org/x/sys v0.25.0
5556
golang.org/x/term v0.24.0
@@ -128,7 +129,6 @@ require (
128129
go.uber.org/atomic v1.11.0 // indirect
129130
go.uber.org/multierr v1.11.0 // indirect
130131
golang.org/x/mod v0.17.0 // indirect
131-
golang.org/x/net v0.29.0 // indirect
132132
golang.org/x/sync v0.8.0 // indirect
133133
golang.org/x/text v0.18.0 // indirect
134134
golang.org/x/time v0.6.0 // indirect

0 commit comments

Comments
 (0)