Skip to content

Commit fafb0fb

Browse files
authored
fix: return clone of http.DefaultTransport from GetTransport() with no args (#1296)
1 parent 567114a commit fafb0fb

File tree

2 files changed

+22
-1
lines changed

2 files changed

+22
-1
lines changed

backend/httpclient/http_client.go

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ func New(opts ...Options) (*http.Client, error) {
5050
// Note: If more than one Options is provided a panic is raised.
5151
func GetTransport(opts ...Options) (http.RoundTripper, error) {
5252
if opts == nil {
53-
return http.DefaultTransport, nil
53+
return GetDefaultTransport()
5454
}
5555

5656
clientOpts := createOptions(opts...)
@@ -96,6 +96,16 @@ func GetTransport(opts ...Options) (http.RoundTripper, error) {
9696
return roundTripperFromMiddlewares(clientOpts, clientOpts.Middlewares, transport)
9797
}
9898

99+
// GetDefaultTransport returns a clone of http.DefaultTransport, if it's of the
100+
// correct type, or an error otherwise. There are a number of places where plugin
101+
// code uses http.DefaultTransport; this supports doing so in a safer way.
102+
func GetDefaultTransport() (http.RoundTripper, error) {
103+
if transport, ok := http.DefaultTransport.(*http.Transport); ok {
104+
return transport.Clone(), nil
105+
}
106+
return nil, fmt.Errorf("http.DefaultTransport is not *http.Transport but %T", http.DefaultTransport)
107+
}
108+
99109
// GetTLSConfig creates a new tls.Config given provided options.
100110
// Note: If more than one Options is provided a panic is raised.
101111
func GetTLSConfig(opts ...Options) (*tls.Config, error) {

backend/httpclient/http_client_test.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -241,3 +241,14 @@ func TestReverseMiddlewares(t *testing.T) {
241241
require.Equal(t, "mw1", reversed[3].(MiddlewareName).MiddlewareName())
242242
})
243243
}
244+
245+
func TestDefaultTransport(t *testing.T) {
246+
t.Run("Transport returned from GetTransport() with no arguments is not http.DefaultTransport", func(t *testing.T) {
247+
transport, err := GetTransport()
248+
require.NoError(t, err)
249+
// This is essentially the same check added to secure_socks_proxy.go in
250+
// https://github.com/grafana/grafana-plugin-sdk-go/pull/1295; since that's
251+
// addressing the issue we're concerned with here, it should suffice.
252+
require.NotEqual(t, transport, http.DefaultTransport.(*http.Transport))
253+
})
254+
}

0 commit comments

Comments
 (0)