From 1b61eb0b3360ef904f841825492bd637189721cd Mon Sep 17 00:00:00 2001 From: swithek Date: Tue, 22 Sep 2020 13:55:08 +0300 Subject: [PATCH 1/3] Copy http client --- dial.go | 14 +++++++++----- dial_test.go | 9 --------- 2 files changed, 9 insertions(+), 14 deletions(-) diff --git a/dial.go b/dial.go index 2b25e351..35516943 100644 --- a/dial.go +++ b/dial.go @@ -8,7 +8,6 @@ import ( "context" "crypto/rand" "encoding/base64" - "errors" "fmt" "io" "io/ioutil" @@ -26,6 +25,7 @@ type DialOptions struct { // HTTPClient is used for the connection. // Its Transport must return writable bodies for WebSocket handshakes. // http.Transport does beginning with Go 1.12. + // Non-zero timeout will be ignored, see https://github.com/nhooyr/websocket/issues/67. HTTPClient *http.Client // HTTPHeader specifies the HTTP headers included in the handshake request. @@ -74,7 +74,15 @@ func dial(ctx context.Context, urls string, opts *DialOptions, rand io.Reader) ( opts = &*opts if opts.HTTPClient == nil { opts.HTTPClient = http.DefaultClient + } else if opts.HTTPClient.Timeout > 0 { + // remove timeout + opts.HTTPClient = &http.Client{ + Transport: opts.HTTPClient.Transport, + CheckRedirect: opts.HTTPClient.CheckRedirect, + Jar: opts.HTTPClient.Jar, + } } + if opts.HTTPHeader == nil { opts.HTTPHeader = http.Header{} } @@ -133,10 +141,6 @@ func dial(ctx context.Context, urls string, opts *DialOptions, rand io.Reader) ( } func handshakeRequest(ctx context.Context, urls string, opts *DialOptions, copts *compressionOptions, secWebSocketKey string) (*http.Response, error) { - if opts.HTTPClient.Timeout > 0 { - return nil, errors.New("use context for cancellation instead of http.Client.Timeout; see https://github.com/nhooyr/websocket/issues/67") - } - u, err := url.Parse(urls) if err != nil { return nil, fmt.Errorf("failed to parse url: %w", err) diff --git a/dial_test.go b/dial_test.go index 7f13a934..28c255c6 100644 --- a/dial_test.go +++ b/dial_test.go @@ -36,15 +36,6 @@ func TestBadDials(t *testing.T) { name: "badURLScheme", url: "ftp://nhooyr.io", }, - { - name: "badHTTPClient", - url: "ws://nhooyr.io", - opts: &DialOptions{ - HTTPClient: &http.Client{ - Timeout: time.Minute, - }, - }, - }, { name: "badTLS", url: "wss://totallyfake.nhooyr.io", From dbaf6f8f37bc74b7176f9cada3cbf454e2fcf148 Mon Sep 17 00:00:00 2001 From: swithek Date: Thu, 24 Sep 2020 10:33:22 +0300 Subject: [PATCH 2/3] Create context with http client timeout --- dial.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/dial.go b/dial.go index 35516943..849515e8 100644 --- a/dial.go +++ b/dial.go @@ -25,7 +25,6 @@ type DialOptions struct { // HTTPClient is used for the connection. // Its Transport must return writable bodies for WebSocket handshakes. // http.Transport does beginning with Go 1.12. - // Non-zero timeout will be ignored, see https://github.com/nhooyr/websocket/issues/67. HTTPClient *http.Client // HTTPHeader specifies the HTTP headers included in the handshake request. @@ -75,7 +74,11 @@ func dial(ctx context.Context, urls string, opts *DialOptions, rand io.Reader) ( if opts.HTTPClient == nil { opts.HTTPClient = http.DefaultClient } else if opts.HTTPClient.Timeout > 0 { - // remove timeout + var cancel context.CancelFunc + + ctx, cancel = context.WithTimeout(ctx, opts.HTTPClient.Timeout) + defer cancel() + opts.HTTPClient = &http.Client{ Transport: opts.HTTPClient.Transport, CheckRedirect: opts.HTTPClient.CheckRedirect, From f67b03bef8172d988704e0a9fc15bd6415b3940b Mon Sep 17 00:00:00 2001 From: swithek Date: Fri, 25 Sep 2020 09:48:19 +0300 Subject: [PATCH 3/3] Improve http client copying --- dial.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/dial.go b/dial.go index 849515e8..509882e0 100644 --- a/dial.go +++ b/dial.go @@ -79,11 +79,9 @@ func dial(ctx context.Context, urls string, opts *DialOptions, rand io.Reader) ( ctx, cancel = context.WithTimeout(ctx, opts.HTTPClient.Timeout) defer cancel() - opts.HTTPClient = &http.Client{ - Transport: opts.HTTPClient.Transport, - CheckRedirect: opts.HTTPClient.CheckRedirect, - Jar: opts.HTTPClient.Jar, - } + newClient := *opts.HTTPClient + newClient.Timeout = 0 + opts.HTTPClient = &newClient } if opts.HTTPHeader == nil {