From 873e67e4d598af0769fdf077ff074dfe82add46a Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Thu, 20 Aug 2020 16:43:18 +0300 Subject: [PATCH 01/21] Fix how the client checks for presence of Upgrade: websocket, Connection: upgrade (#604) The values of the `Upgrade` and `Connection` response headers can contain multiple tokens, for example Connection: upgrade, keep-alive The WebSocket RFC describes the checking of these as follows: 2. If the response lacks an |Upgrade| header field or the |Upgrade| header field contains a value that is not an ASCII case- insensitive match for the value "websocket", the client MUST _Fail the WebSocket Connection_. 3. If the response lacks a |Connection| header field or the |Connection| header field doesn't contain a token that is an ASCII case-insensitive match for the value "Upgrade", the client MUST _Fail the WebSocket Connection_. It is careful to note "contains a value", "contains a token". Previously, the client would reject with "bad handshake" if the header doesn't contain exactly the value it looks for. Change the checks to use `tokenListContainsValue` instead, which is incidentally what the server is already doing for similar checks. --- client.go | 4 ++-- client_server_test.go | 17 +++++++++++++++++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/client.go b/client.go index 962c06a..c4b62fb 100644 --- a/client.go +++ b/client.go @@ -348,8 +348,8 @@ func (d *Dialer) DialContext(ctx context.Context, urlStr string, requestHeader h } if resp.StatusCode != 101 || - !strings.EqualFold(resp.Header.Get("Upgrade"), "websocket") || - !strings.EqualFold(resp.Header.Get("Connection"), "upgrade") || + !tokenListContainsValue(resp.Header, "Upgrade", "websocket") || + !tokenListContainsValue(resp.Header, "Connection", "upgrade") || resp.Header.Get("Sec-Websocket-Accept") != computeAcceptKey(challengeKey) { // Before closing the network connection on return from this // function, slurp up some of the response to aid application diff --git a/client_server_test.go b/client_server_test.go index 7e7636f..5fd2c85 100644 --- a/client_server_test.go +++ b/client_server_test.go @@ -481,6 +481,23 @@ func TestBadMethod(t *testing.T) { } } +func TestDialExtraTokensInRespHeaders(t *testing.T) { + s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + challengeKey := r.Header.Get("Sec-Websocket-Key") + w.Header().Set("Upgrade", "foo, websocket") + w.Header().Set("Connection", "upgrade, keep-alive") + w.Header().Set("Sec-Websocket-Accept", computeAcceptKey(challengeKey)) + w.WriteHeader(101) + })) + defer s.Close() + + ws, _, err := cstDialer.Dial(makeWsProto(s.URL), nil) + if err != nil { + t.Fatalf("Dial: %v", err) + } + defer ws.Close() +} + func TestHandshake(t *testing.T) { s := newServer(t) defer s.Close() From 78ab81e2420a3a26d885919891da9e7903bf3342 Mon Sep 17 00:00:00 2001 From: Matt Silverlock Date: Sat, 22 Aug 2020 14:03:32 -0700 Subject: [PATCH 02/21] docs: clarify that sub protocols are not set via responseHeader arg. --- server.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server.go b/server.go index 887d558..152ebf8 100644 --- a/server.go +++ b/server.go @@ -115,8 +115,8 @@ func (u *Upgrader) selectSubprotocol(r *http.Request, responseHeader http.Header // Upgrade upgrades the HTTP server connection to the WebSocket protocol. // // The responseHeader is included in the response to the client's upgrade -// request. Use the responseHeader to specify cookies (Set-Cookie) and the -// application negotiated subprotocol (Sec-WebSocket-Protocol). +// request. Use the responseHeader to specify cookies (Set-Cookie). To specify +// subprotocols supported by the server, set Upgrader.Subprotocols directly. // // If the upgrade fails, then Upgrade replies to the client with an HTTP error // response. From c3dd95aea9779669bb3daafbd84ee0530c8ce1c1 Mon Sep 17 00:00:00 2001 From: Matt Silverlock Date: Sat, 12 Sep 2020 12:32:13 -0700 Subject: [PATCH 03/21] build: use build matrix; drop Go <= 1.10 (#629) --- .circleci/config.yml | 130 +++++++++++++++++++++---------------------- 1 file changed, 62 insertions(+), 68 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 1240d78..554a446 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -1,76 +1,70 @@ -version: 2.0 +version: 2.1 jobs: - # Base test configuration for Go library tests Each distinct version should - # inherit this base, and override (at least) the container image used. - "test": &test + "test": + parameters: + version: + type: string + default: "latest" + golint: + type: boolean + default: true + modules: + type: boolean + default: true + goproxy: + type: string + default: "" docker: - - image: circleci/golang:latest + - image: "circleci/golang:<< parameters.version >>" working_directory: /go/src/github.com/gorilla/websocket - steps: &steps - - checkout - - run: go version - - run: go get -t -v ./... - # Only run gofmt, vet & lint against the latest Go version - - run: > - if [[ "$LATEST" = true ]]; then - go get -u golang.org/x/lint/golint - golint ./... - fi - - run: > - if [[ "$LATEST" = true ]]; then - diff -u <(echo -n) <(gofmt -d .) - fi - - run: > - if [[ "$LATEST" = true ]]; then - go vet -v . - fi - - run: if [[ "$LATEST" = true ]]; then go vet -v .; fi - - run: go test -v -race ./... - - "latest": - <<: *test environment: - LATEST: true - - "1.12": - <<: *test - docker: - - image: circleci/golang:1.12 - - "1.11": - <<: *test - docker: - - image: circleci/golang:1.11 - - "1.10": - <<: *test - docker: - - image: circleci/golang:1.10 - - "1.9": - <<: *test - docker: - - image: circleci/golang:1.9 - - "1.8": - <<: *test - docker: - - image: circleci/golang:1.8 - - "1.7": - <<: *test - docker: - - image: circleci/golang:1.7 + GO111MODULE: "on" + GOPROXY: "<< parameters.goproxy >>" + steps: + - checkout + - run: + name: "Print the Go version" + command: > + go version + - run: + name: "Fetch dependencies" + command: > + if [[ << parameters.modules >> = true ]]; then + go mod download + export GO111MODULE=on + else + go get -v ./... + fi + # Only run gofmt, vet & lint against the latest Go version + - run: + name: "Run golint" + command: > + if [ << parameters.version >> = "latest" ] && [ << parameters.golint >> = true ]; then + go get -u golang.org/x/lint/golint + golint ./... + fi + - run: + name: "Run gofmt" + command: > + if [[ << parameters.version >> = "latest" ]]; then + diff -u <(echo -n) <(gofmt -d -e .) + fi + - run: + name: "Run go vet" + command: > + if [[ << parameters.version >> = "latest" ]]; then + go vet -v ./... + fi + - run: + name: "Run go test (+ race detector)" + command: > + go test -v -race ./... workflows: - version: 2 - build: + tests: jobs: - - "latest" - - "1.12" - - "1.11" - - "1.10" - - "1.9" - - "1.8" - - "1.7" + - test: + matrix: + parameters: + version: ["latest", "1.15", "1.14", "1.13", "1.12", "1.11"] From e8629af678b7fe13f35dff5e197de93b4148a909 Mon Sep 17 00:00:00 2001 From: hellflame Date: Sun, 25 Apr 2021 00:20:22 +0800 Subject: [PATCH 04/21] improve echo example (#671) --- examples/echo/server.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/examples/echo/server.go b/examples/echo/server.go index 2f5305f..61f3dd9 100644 --- a/examples/echo/server.go +++ b/examples/echo/server.go @@ -69,6 +69,7 @@ window.addEventListener("load", function(evt) { var d = document.createElement("div"); d.textContent = message; output.appendChild(d); + output.scroll(0, output.scrollHeight); }; document.getElementById("open").onclick = function(evt) { @@ -126,7 +127,7 @@ You can change the message and send multiple times. -
+
From b4b5d887ad624c8886795879af90a07a5fd1f48d Mon Sep 17 00:00:00 2001 From: Gary Burd Date: Thu, 16 Dec 2021 11:07:50 -0800 Subject: [PATCH 05/21] Document the allowed concurrency on Upgrader and Dialer (#636) * Document allowed concurrency on Dialer. * Document allowed concurrency on Upgrader. --- client.go | 2 ++ server.go | 2 ++ 2 files changed, 4 insertions(+) diff --git a/client.go b/client.go index c4b62fb..e4fc18f 100644 --- a/client.go +++ b/client.go @@ -48,6 +48,8 @@ func NewClient(netConn net.Conn, u *url.URL, requestHeader http.Header, readBufS } // A Dialer contains options for connecting to WebSocket server. +// +// It is safe to call Dialer's methods concurrently. type Dialer struct { // NetDial specifies the dial function for creating TCP connections. If // NetDial is nil, net.Dial is used. diff --git a/server.go b/server.go index 152ebf8..406f307 100644 --- a/server.go +++ b/server.go @@ -23,6 +23,8 @@ func (e HandshakeError) Error() string { return e.message } // Upgrader specifies parameters for upgrading an HTTP connection to a // WebSocket connection. +// +// It is safe to call Upgrader's methods concurrently. type Upgrader struct { // HandshakeTimeout specifies the duration for the handshake to complete. HandshakeTimeout time.Duration From 1905f7e442f05e3529c2a41f419b07c43f73f701 Mon Sep 17 00:00:00 2001 From: Gary Burd Date: Fri, 17 Dec 2021 22:48:51 -0500 Subject: [PATCH 06/21] Update source to match output from gofmt 1.17 --- client_clone.go | 1 + client_clone_legacy.go | 1 + conn_write.go | 1 + conn_write_legacy.go | 1 + examples/echo/client.go | 1 + examples/echo/server.go | 1 + mask.go | 1 + mask_safe.go | 1 + trace.go | 1 + trace_17.go | 1 + 10 files changed, 10 insertions(+) diff --git a/client_clone.go b/client_clone.go index 4f0d943..4179c7a 100644 --- a/client_clone.go +++ b/client_clone.go @@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. +//go:build go1.8 // +build go1.8 package websocket diff --git a/client_clone_legacy.go b/client_clone_legacy.go index babb007..7e241a8 100644 --- a/client_clone_legacy.go +++ b/client_clone_legacy.go @@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. +//go:build !go1.8 // +build !go1.8 package websocket diff --git a/conn_write.go b/conn_write.go index a509a21..497467a 100644 --- a/conn_write.go +++ b/conn_write.go @@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. +//go:build go1.8 // +build go1.8 package websocket diff --git a/conn_write_legacy.go b/conn_write_legacy.go index 37edaff..8501a23 100644 --- a/conn_write_legacy.go +++ b/conn_write_legacy.go @@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. +//go:build !go1.8 // +build !go1.8 package websocket diff --git a/examples/echo/client.go b/examples/echo/client.go index bf0e657..7d870bd 100644 --- a/examples/echo/client.go +++ b/examples/echo/client.go @@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. +//go:build ignore // +build ignore package main diff --git a/examples/echo/server.go b/examples/echo/server.go index 61f3dd9..f9a0b7b 100644 --- a/examples/echo/server.go +++ b/examples/echo/server.go @@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. +//go:build ignore // +build ignore package main diff --git a/mask.go b/mask.go index 577fce9..d0742bf 100644 --- a/mask.go +++ b/mask.go @@ -2,6 +2,7 @@ // this source code is governed by a BSD-style license that can be found in the // LICENSE file. +//go:build !appengine // +build !appengine package websocket diff --git a/mask_safe.go b/mask_safe.go index 2aac060..36250ca 100644 --- a/mask_safe.go +++ b/mask_safe.go @@ -2,6 +2,7 @@ // this source code is governed by a BSD-style license that can be found in the // LICENSE file. +//go:build appengine // +build appengine package websocket diff --git a/trace.go b/trace.go index 834f122..246a5d3 100644 --- a/trace.go +++ b/trace.go @@ -1,3 +1,4 @@ +//go:build go1.8 // +build go1.8 package websocket diff --git a/trace_17.go b/trace_17.go index 77d05a0..f4be940 100644 --- a/trace_17.go +++ b/trace_17.go @@ -1,3 +1,4 @@ +//go:build !go1.8 // +build !go1.8 package websocket From 2c8965691051f6fbde50d62676afc709f23249dd Mon Sep 17 00:00:00 2001 From: Rn <35656609+thak1411@users.noreply.github.com> Date: Mon, 20 Dec 2021 01:21:45 +0900 Subject: [PATCH 07/21] Modify http Method String Literal to Variable (#728) --- client.go | 2 +- client_server_test.go | 8 ++++---- examples/autobahn/server.go | 2 +- examples/chat/main.go | 2 +- examples/command/main.go | 2 +- examples/filewatch/main.go | 2 +- proxy.go | 2 +- server.go | 2 +- server_test.go | 2 +- 9 files changed, 12 insertions(+), 12 deletions(-) diff --git a/client.go b/client.go index e4fc18f..196a659 100644 --- a/client.go +++ b/client.go @@ -178,7 +178,7 @@ func (d *Dialer) DialContext(ctx context.Context, urlStr string, requestHeader h } req := &http.Request{ - Method: "GET", + Method: http.MethodGet, URL: u, Proto: "HTTP/1.1", ProtoMajor: 1, diff --git a/client_server_test.go b/client_server_test.go index 5fd2c85..c03f2a9 100644 --- a/client_server_test.go +++ b/client_server_test.go @@ -163,7 +163,7 @@ func TestProxyDial(t *testing.T) { // Capture the request Host header. s.Server.Config.Handler = http.HandlerFunc( func(w http.ResponseWriter, r *http.Request) { - if r.Method == "CONNECT" { + if r.Method == http.MethodConnect { connect = true w.WriteHeader(http.StatusOK) return @@ -203,7 +203,7 @@ func TestProxyAuthorizationDial(t *testing.T) { func(w http.ResponseWriter, r *http.Request) { proxyAuth := r.Header.Get("Proxy-Authorization") expectedProxyAuth := "Basic " + base64.StdEncoding.EncodeToString([]byte("username:password")) - if r.Method == "CONNECT" && proxyAuth == expectedProxyAuth { + if r.Method == http.MethodConnect && proxyAuth == expectedProxyAuth { connect = true w.WriteHeader(http.StatusOK) return @@ -463,7 +463,7 @@ func TestBadMethod(t *testing.T) { })) defer s.Close() - req, err := http.NewRequest("POST", s.URL, strings.NewReader("")) + req, err := http.NewRequest(http.MethodPost, s.URL, strings.NewReader("")) if err != nil { t.Fatalf("NewRequest returned error %v", err) } @@ -735,7 +735,7 @@ func TestHost(t *testing.T) { Dial: dialer.NetDial, TLSClientConfig: dialer.TLSClientConfig, } - req, _ := http.NewRequest("GET", httpProtos[tt.server]+tt.url+"/", nil) + req, _ := http.NewRequest(http.MethodGet, httpProtos[tt.server]+tt.url+"/", nil) if tt.header != "" { req.Host = tt.header } diff --git a/examples/autobahn/server.go b/examples/autobahn/server.go index c2d6ee5..8b17fe3 100644 --- a/examples/autobahn/server.go +++ b/examples/autobahn/server.go @@ -160,7 +160,7 @@ func serveHome(w http.ResponseWriter, r *http.Request) { http.Error(w, "Not found.", http.StatusNotFound) return } - if r.Method != "GET" { + if r.Method != http.MethodGet { http.Error(w, "Method not allowed", http.StatusMethodNotAllowed) return } diff --git a/examples/chat/main.go b/examples/chat/main.go index 9d4737a..474709f 100644 --- a/examples/chat/main.go +++ b/examples/chat/main.go @@ -18,7 +18,7 @@ func serveHome(w http.ResponseWriter, r *http.Request) { http.Error(w, "Not found", http.StatusNotFound) return } - if r.Method != "GET" { + if r.Method != http.MethodGet { http.Error(w, "Method not allowed", http.StatusMethodNotAllowed) return } diff --git a/examples/command/main.go b/examples/command/main.go index 304f1a5..38d9f6c 100644 --- a/examples/command/main.go +++ b/examples/command/main.go @@ -170,7 +170,7 @@ func serveHome(w http.ResponseWriter, r *http.Request) { http.Error(w, "Not found", http.StatusNotFound) return } - if r.Method != "GET" { + if r.Method != http.MethodGet { http.Error(w, "Method not allowed", http.StatusMethodNotAllowed) return } diff --git a/examples/filewatch/main.go b/examples/filewatch/main.go index b834ed3..d4bf80e 100644 --- a/examples/filewatch/main.go +++ b/examples/filewatch/main.go @@ -133,7 +133,7 @@ func serveHome(w http.ResponseWriter, r *http.Request) { http.Error(w, "Not found", http.StatusNotFound) return } - if r.Method != "GET" { + if r.Method != http.MethodGet { http.Error(w, "Method not allowed", http.StatusMethodNotAllowed) return } diff --git a/proxy.go b/proxy.go index e87a8c9..e0f466b 100644 --- a/proxy.go +++ b/proxy.go @@ -48,7 +48,7 @@ func (hpd *httpProxyDialer) Dial(network string, addr string) (net.Conn, error) } connectReq := &http.Request{ - Method: "CONNECT", + Method: http.MethodConnect, URL: &url.URL{Opaque: addr}, Host: addr, Header: connectHeader, diff --git a/server.go b/server.go index 406f307..24d53b3 100644 --- a/server.go +++ b/server.go @@ -133,7 +133,7 @@ func (u *Upgrader) Upgrade(w http.ResponseWriter, r *http.Request, responseHeade return u.returnError(w, r, http.StatusBadRequest, badHandshake+"'websocket' token not found in 'Upgrade' header") } - if r.Method != "GET" { + if r.Method != http.MethodGet { return u.returnError(w, r, http.StatusMethodNotAllowed, badHandshake+"request method is not GET") } diff --git a/server_test.go b/server_test.go index 456c1db..3029aa8 100644 --- a/server_test.go +++ b/server_test.go @@ -98,7 +98,7 @@ func TestBufioReuse(t *testing.T) { } upgrader := Upgrader{} c, err := upgrader.Upgrade(resp, &http.Request{ - Method: "GET", + Method: http.MethodGet, Header: http.Header{ "Upgrade": []string{"websocket"}, "Connection": []string{"upgrade"}, From bcef8431c98087addcb2f0ab484ff295abe41a74 Mon Sep 17 00:00:00 2001 From: Gary Burd Date: Sat, 1 Jan 2022 08:43:22 -0800 Subject: [PATCH 08/21] Use context.Context in TLS handshake (#751) Continued work on #730. --- .circleci/config.yml | 2 +- client.go | 23 ++++++----------------- tls_handshake.go | 21 +++++++++++++++++++++ tls_handshake_116.go | 21 +++++++++++++++++++++ trace.go | 20 -------------------- trace_17.go | 13 ------------- 6 files changed, 49 insertions(+), 51 deletions(-) create mode 100644 tls_handshake.go create mode 100644 tls_handshake_116.go delete mode 100644 trace.go delete mode 100644 trace_17.go diff --git a/.circleci/config.yml b/.circleci/config.yml index 554a446..a0eb0ed 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -67,4 +67,4 @@ workflows: - test: matrix: parameters: - version: ["latest", "1.15", "1.14", "1.13", "1.12", "1.11"] + version: ["latest", "1.17", "1.16", "1.15", "1.14", "1.13", "1.12", "1.11"] diff --git a/client.go b/client.go index 196a659..a24c3ce 100644 --- a/client.go +++ b/client.go @@ -314,11 +314,12 @@ func (d *Dialer) DialContext(ctx context.Context, urlStr string, requestHeader h tlsConn := tls.Client(netConn, cfg) netConn = tlsConn - var err error - if trace != nil { - err = doHandshakeWithTrace(trace, tlsConn, cfg) - } else { - err = doHandshake(tlsConn, cfg) + if trace != nil && trace.TLSHandshakeStart != nil { + trace.TLSHandshakeStart() + } + err := doHandshake(ctx, tlsConn, cfg) + if trace != nil && trace.TLSHandshakeDone != nil { + trace.TLSHandshakeDone(tlsConn.ConnectionState(), err) } if err != nil { @@ -383,15 +384,3 @@ func (d *Dialer) DialContext(ctx context.Context, urlStr string, requestHeader h netConn = nil // to avoid close in defer. return conn, resp, nil } - -func doHandshake(tlsConn *tls.Conn, cfg *tls.Config) error { - if err := tlsConn.Handshake(); err != nil { - return err - } - if !cfg.InsecureSkipVerify { - if err := tlsConn.VerifyHostname(cfg.ServerName); err != nil { - return err - } - } - return nil -} diff --git a/tls_handshake.go b/tls_handshake.go new file mode 100644 index 0000000..a62b68c --- /dev/null +++ b/tls_handshake.go @@ -0,0 +1,21 @@ +//go:build go1.17 +// +build go1.17 + +package websocket + +import ( + "context" + "crypto/tls" +) + +func doHandshake(ctx context.Context, tlsConn *tls.Conn, cfg *tls.Config) error { + if err := tlsConn.HandshakeContext(ctx); err != nil { + return err + } + if !cfg.InsecureSkipVerify { + if err := tlsConn.VerifyHostname(cfg.ServerName); err != nil { + return err + } + } + return nil +} diff --git a/tls_handshake_116.go b/tls_handshake_116.go new file mode 100644 index 0000000..e1b2b44 --- /dev/null +++ b/tls_handshake_116.go @@ -0,0 +1,21 @@ +//go:build !go1.17 +// +build !go1.17 + +package websocket + +import ( + "context" + "crypto/tls" +) + +func doHandshake(ctx context.Context, tlsConn *tls.Conn, cfg *tls.Config) error { + if err := tlsConn.Handshake(); err != nil { + return err + } + if !cfg.InsecureSkipVerify { + if err := tlsConn.VerifyHostname(cfg.ServerName); err != nil { + return err + } + } + return nil +} diff --git a/trace.go b/trace.go deleted file mode 100644 index 246a5d3..0000000 --- a/trace.go +++ /dev/null @@ -1,20 +0,0 @@ -//go:build go1.8 -// +build go1.8 - -package websocket - -import ( - "crypto/tls" - "net/http/httptrace" -) - -func doHandshakeWithTrace(trace *httptrace.ClientTrace, tlsConn *tls.Conn, cfg *tls.Config) error { - if trace.TLSHandshakeStart != nil { - trace.TLSHandshakeStart() - } - err := doHandshake(tlsConn, cfg) - if trace.TLSHandshakeDone != nil { - trace.TLSHandshakeDone(tlsConn.ConnectionState(), err) - } - return err -} diff --git a/trace_17.go b/trace_17.go deleted file mode 100644 index f4be940..0000000 --- a/trace_17.go +++ /dev/null @@ -1,13 +0,0 @@ -//go:build !go1.8 -// +build !go1.8 - -package websocket - -import ( - "crypto/tls" - "net/http/httptrace" -) - -func doHandshakeWithTrace(trace *httptrace.ClientTrace, tlsConn *tls.Conn, cfg *tls.Config) error { - return doHandshake(tlsConn, cfg) -} From beca1d39409212eff6678719a8ecf7761184adc8 Mon Sep 17 00:00:00 2001 From: Alexander Emelin Date: Sun, 2 Jan 2022 18:35:34 +0300 Subject: [PATCH 09/21] Fix broadcast benchmarks (#542) * do not use cached PreparedMessage in broadcast benchmarks * pick better name for benchmark method --- conn_broadcast_test.go | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/conn_broadcast_test.go b/conn_broadcast_test.go index cb88cbb..6e744fc 100644 --- a/conn_broadcast_test.go +++ b/conn_broadcast_test.go @@ -18,7 +18,6 @@ import ( // scenarios with many subscribers in one channel. type broadcastBench struct { w io.Writer - message *broadcastMessage closeCh chan struct{} doneCh chan struct{} count int32 @@ -52,14 +51,6 @@ func newBroadcastBench(usePrepared, compression bool) *broadcastBench { usePrepared: usePrepared, compression: compression, } - msg := &broadcastMessage{ - payload: textMessages(1)[0], - } - if usePrepared { - pm, _ := NewPreparedMessage(TextMessage, msg.payload) - msg.prepared = pm - } - bench.message = msg bench.makeConns(10000) return bench } @@ -78,7 +69,7 @@ func (b *broadcastBench) makeConns(numConns int) { for { select { case msg := <-c.msgCh: - if b.usePrepared { + if msg.prepared != nil { c.conn.WritePreparedMessage(msg.prepared) } else { c.conn.WriteMessage(TextMessage, msg.payload) @@ -100,9 +91,9 @@ func (b *broadcastBench) close() { close(b.closeCh) } -func (b *broadcastBench) runOnce() { +func (b *broadcastBench) broadcastOnce(msg *broadcastMessage) { for _, c := range b.conns { - c.msgCh <- b.message + c.msgCh <- msg } <-b.doneCh } @@ -114,17 +105,25 @@ func BenchmarkBroadcast(b *testing.B) { compression bool }{ {"NoCompression", false, false}, - {"WithCompression", false, true}, + {"Compression", false, true}, {"NoCompressionPrepared", true, false}, - {"WithCompressionPrepared", true, true}, + {"CompressionPrepared", true, true}, } + payload := textMessages(1)[0] for _, bm := range benchmarks { b.Run(bm.name, func(b *testing.B) { bench := newBroadcastBench(bm.usePrepared, bm.compression) defer bench.close() b.ResetTimer() for i := 0; i < b.N; i++ { - bench.runOnce() + message := &broadcastMessage{ + payload: payload, + } + if bench.usePrepared { + pm, _ := NewPreparedMessage(TextMessage, message.payload) + message.prepared = pm + } + bench.broadcastOnce(message) } b.ReportAllocs() }) From 2d6ee4c55cc9e9dc0eb5929f32a999213e25256f Mon Sep 17 00:00:00 2001 From: Gary Burd Date: Sun, 2 Jan 2022 11:21:21 -0800 Subject: [PATCH 10/21] Update autobahn example - Update instructions to use docker. - Cleanup config file. --- examples/autobahn/README.md | 9 +++++-- examples/autobahn/config/fuzzingclient.json | 29 +++++++++++++++++++++ examples/autobahn/fuzzingclient.json | 15 ----------- 3 files changed, 36 insertions(+), 17 deletions(-) create mode 100644 examples/autobahn/config/fuzzingclient.json delete mode 100644 examples/autobahn/fuzzingclient.json diff --git a/examples/autobahn/README.md b/examples/autobahn/README.md index dde8525..cc954fe 100644 --- a/examples/autobahn/README.md +++ b/examples/autobahn/README.md @@ -8,6 +8,11 @@ To test the server, run and start the client test driver - wstest -m fuzzingclient -s fuzzingclient.json + mkdir -p reports + docker run -it --rm \ + -v ${PWD}/config:/config \ + -v ${PWD}/reports:/reports \ + crossbario/autobahn-testsuite \ + wstest -m fuzzingclient -s /config/fuzzingclient.json -When the client completes, it writes a report to reports/clients/index.html. +When the client completes, it writes a report to reports/index.html. diff --git a/examples/autobahn/config/fuzzingclient.json b/examples/autobahn/config/fuzzingclient.json new file mode 100644 index 0000000..eda4e66 --- /dev/null +++ b/examples/autobahn/config/fuzzingclient.json @@ -0,0 +1,29 @@ +{ + "cases": ["*"], + "exclude-cases": [], + "exclude-agent-cases": {}, + "outdir": "/reports", + "options": {"failByDrop": false}, + "servers": [ + { + "agent": "ReadAllWriteMessage", + "url": "ws://host.docker.internal:9000/m" + }, + { + "agent": "ReadAllWritePreparedMessage", + "url": "ws://host.docker.internal:9000/p" + }, + { + "agent": "CopyFull", + "url": "ws://host.docker.internal:9000/f" + }, + { + "agent": "ReadAllWrite", + "url": "ws://host.docker.internal:9000/r" + }, + { + "agent": "CopyWriterOnly", + "url": "ws://host.docker.internal:9000/c" + } + ] +} diff --git a/examples/autobahn/fuzzingclient.json b/examples/autobahn/fuzzingclient.json deleted file mode 100644 index aa3a0bc..0000000 --- a/examples/autobahn/fuzzingclient.json +++ /dev/null @@ -1,15 +0,0 @@ - -{ - "options": {"failByDrop": false}, - "outdir": "./reports/clients", - "servers": [ - {"agent": "ReadAllWriteMessage", "url": "ws://localhost:9000/m", "options": {"version": 18}}, - {"agent": "ReadAllWritePreparedMessage", "url": "ws://localhost:9000/p", "options": {"version": 18}}, - {"agent": "ReadAllWrite", "url": "ws://localhost:9000/r", "options": {"version": 18}}, - {"agent": "CopyFull", "url": "ws://localhost:9000/f", "options": {"version": 18}}, - {"agent": "CopyWriterOnly", "url": "ws://localhost:9000/c", "options": {"version": 18}} - ], - "cases": ["*"], - "exclude-cases": [], - "exclude-agent-cases": {} -} From f0643a3a18bd24604a6131076f6419c7c518a956 Mon Sep 17 00:00:00 2001 From: Gary Burd Date: Sun, 2 Jan 2022 12:16:08 -0800 Subject: [PATCH 11/21] Improve protocol error messages To aid protocol error debugging, report all errors found in the first two bytes of a message header. --- conn.go | 57 ++++++++++++++++++++++++++++++++++++++++----------------- 1 file changed, 40 insertions(+), 17 deletions(-) diff --git a/conn.go b/conn.go index ca46d2f..2296bf3 100644 --- a/conn.go +++ b/conn.go @@ -13,6 +13,7 @@ import ( "math/rand" "net" "strconv" + "strings" "sync" "time" "unicode/utf8" @@ -794,47 +795,69 @@ func (c *Conn) advanceFrame() (int, error) { } // 2. Read and parse first two bytes of frame header. + // To aid debugging, collect and report all errors in the first two bytes + // of the header. + + var errors []string p, err := c.read(2) if err != nil { return noFrame, err } - final := p[0]&finalBit != 0 frameType := int(p[0] & 0xf) + final := p[0]&finalBit != 0 + rsv1 := p[0]&rsv1Bit != 0 + rsv2 := p[0]&rsv2Bit != 0 + rsv3 := p[0]&rsv3Bit != 0 mask := p[1]&maskBit != 0 c.setReadRemaining(int64(p[1] & 0x7f)) c.readDecompress = false - if c.newDecompressionReader != nil && (p[0]&rsv1Bit) != 0 { - c.readDecompress = true - p[0] &^= rsv1Bit + if rsv1 { + if c.newDecompressionReader != nil { + c.readDecompress = true + } else { + errors = append(errors, "RSV1 set") + } } - if rsv := p[0] & (rsv1Bit | rsv2Bit | rsv3Bit); rsv != 0 { - return noFrame, c.handleProtocolError("unexpected reserved bits 0x" + strconv.FormatInt(int64(rsv), 16)) + if rsv2 { + errors = append(errors, "RSV2 set") + } + + if rsv3 { + errors = append(errors, "RSV3 set") } switch frameType { case CloseMessage, PingMessage, PongMessage: if c.readRemaining > maxControlFramePayloadSize { - return noFrame, c.handleProtocolError("control frame length > 125") + errors = append(errors, "len > 125 for control") } if !final { - return noFrame, c.handleProtocolError("control frame not final") + errors = append(errors, "FIN not set on control") } case TextMessage, BinaryMessage: if !c.readFinal { - return noFrame, c.handleProtocolError("message start before final message frame") + errors = append(errors, "data before FIN") } c.readFinal = final case continuationFrame: if c.readFinal { - return noFrame, c.handleProtocolError("continuation after final message frame") + errors = append(errors, "continuation after FIN") } c.readFinal = final default: - return noFrame, c.handleProtocolError("unknown opcode " + strconv.Itoa(frameType)) + errors = append(errors, "bad opcode "+strconv.Itoa(frameType)) + } + + if mask != c.isServer { + errors = append(errors, "bad MASK") + } + + if len(errors) > 0 { + return noFrame, c.handleProtocolError(strings.Join(errors, ", ")) } // 3. Read and parse frame length as per @@ -872,10 +895,6 @@ func (c *Conn) advanceFrame() (int, error) { // 4. Handle frame masking. - if mask != c.isServer { - return noFrame, c.handleProtocolError("incorrect mask flag") - } - if mask { c.readMaskPos = 0 p, err := c.read(len(c.readMaskKey)) @@ -935,7 +954,7 @@ func (c *Conn) advanceFrame() (int, error) { if len(payload) >= 2 { closeCode = int(binary.BigEndian.Uint16(payload)) if !isValidReceivedCloseCode(closeCode) { - return noFrame, c.handleProtocolError("invalid close code") + return noFrame, c.handleProtocolError("bad close code " + strconv.Itoa(closeCode)) } closeText = string(payload[2:]) if !utf8.ValidString(closeText) { @@ -952,7 +971,11 @@ func (c *Conn) advanceFrame() (int, error) { } func (c *Conn) handleProtocolError(message string) error { - c.WriteControl(CloseMessage, FormatCloseMessage(CloseProtocolError, message), time.Now().Add(writeWait)) + data := FormatCloseMessage(CloseProtocolError, message) + if len(data) > maxControlFramePayloadSize { + data = data[:maxControlFramePayloadSize] + } + c.WriteControl(CloseMessage, data, time.Now().Add(writeWait)) return errors.New("websocket: " + message) } From 4fad4036191b2a16b723b60a13a1690c5b8c27a9 Mon Sep 17 00:00:00 2001 From: Gary Burd Date: Sun, 2 Jan 2022 15:53:55 -0800 Subject: [PATCH 12/21] Remove support for Go 1.8 --- client.go | 7 +++++++ client_clone.go | 17 ----------------- client_clone_legacy.go | 39 --------------------------------------- conn.go | 6 ++++++ conn_write.go | 16 ---------------- conn_write_legacy.go | 19 ------------------- 6 files changed, 13 insertions(+), 91 deletions(-) delete mode 100644 client_clone.go delete mode 100644 client_clone_legacy.go delete mode 100644 conn_write.go delete mode 100644 conn_write_legacy.go diff --git a/client.go b/client.go index a24c3ce..ecbe584 100644 --- a/client.go +++ b/client.go @@ -384,3 +384,10 @@ func (d *Dialer) DialContext(ctx context.Context, urlStr string, requestHeader h netConn = nil // to avoid close in defer. return conn, resp, nil } + +func cloneTLSConfig(cfg *tls.Config) *tls.Config { + if cfg == nil { + return &tls.Config{} + } + return cfg.Clone() +} diff --git a/client_clone.go b/client_clone.go deleted file mode 100644 index 4179c7a..0000000 --- a/client_clone.go +++ /dev/null @@ -1,17 +0,0 @@ -// Copyright 2013 The Gorilla WebSocket Authors. All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -//go:build go1.8 -// +build go1.8 - -package websocket - -import "crypto/tls" - -func cloneTLSConfig(cfg *tls.Config) *tls.Config { - if cfg == nil { - return &tls.Config{} - } - return cfg.Clone() -} diff --git a/client_clone_legacy.go b/client_clone_legacy.go deleted file mode 100644 index 7e241a8..0000000 --- a/client_clone_legacy.go +++ /dev/null @@ -1,39 +0,0 @@ -// Copyright 2013 The Gorilla WebSocket Authors. All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -//go:build !go1.8 -// +build !go1.8 - -package websocket - -import "crypto/tls" - -// cloneTLSConfig clones all public fields except the fields -// SessionTicketsDisabled and SessionTicketKey. This avoids copying the -// sync.Mutex in the sync.Once and makes it safe to call cloneTLSConfig on a -// config in active use. -func cloneTLSConfig(cfg *tls.Config) *tls.Config { - if cfg == nil { - return &tls.Config{} - } - return &tls.Config{ - Rand: cfg.Rand, - Time: cfg.Time, - Certificates: cfg.Certificates, - NameToCertificate: cfg.NameToCertificate, - GetCertificate: cfg.GetCertificate, - RootCAs: cfg.RootCAs, - NextProtos: cfg.NextProtos, - ServerName: cfg.ServerName, - ClientAuth: cfg.ClientAuth, - ClientCAs: cfg.ClientCAs, - InsecureSkipVerify: cfg.InsecureSkipVerify, - CipherSuites: cfg.CipherSuites, - PreferServerCipherSuites: cfg.PreferServerCipherSuites, - ClientSessionCache: cfg.ClientSessionCache, - MinVersion: cfg.MinVersion, - MaxVersion: cfg.MaxVersion, - CurvePreferences: cfg.CurvePreferences, - } -} diff --git a/conn.go b/conn.go index 2296bf3..331eebc 100644 --- a/conn.go +++ b/conn.go @@ -402,6 +402,12 @@ func (c *Conn) write(frameType int, deadline time.Time, buf0, buf1 []byte) error return nil } +func (c *Conn) writeBufs(bufs ...[]byte) error { + b := net.Buffers(bufs) + _, err := b.WriteTo(c.conn) + return err +} + // WriteControl writes a control message with the given deadline. The allowed // message types are CloseMessage, PingMessage and PongMessage. func (c *Conn) WriteControl(messageType int, data []byte, deadline time.Time) error { diff --git a/conn_write.go b/conn_write.go deleted file mode 100644 index 497467a..0000000 --- a/conn_write.go +++ /dev/null @@ -1,16 +0,0 @@ -// Copyright 2016 The Gorilla WebSocket Authors. All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -//go:build go1.8 -// +build go1.8 - -package websocket - -import "net" - -func (c *Conn) writeBufs(bufs ...[]byte) error { - b := net.Buffers(bufs) - _, err := b.WriteTo(c.conn) - return err -} diff --git a/conn_write_legacy.go b/conn_write_legacy.go deleted file mode 100644 index 8501a23..0000000 --- a/conn_write_legacy.go +++ /dev/null @@ -1,19 +0,0 @@ -// Copyright 2016 The Gorilla WebSocket Authors. All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -//go:build !go1.8 -// +build !go1.8 - -package websocket - -func (c *Conn) writeBufs(bufs ...[]byte) error { - for _, buf := range bufs { - if len(buf) > 0 { - if _, err := c.conn.Write(buf); err != nil { - return err - } - } - } - return nil -} From 2f25f7843d3d0e4889e5e008dcbdd77fec378deb Mon Sep 17 00:00:00 2001 From: Gary Burd Date: Mon, 3 Jan 2022 17:49:10 -0800 Subject: [PATCH 13/21] Update README (#757) - Note that a new maintainer is needed. - Remove comparison with x/net/websocket. There's no need to describe the issues with that package now that the package's documentation points people here and elsewhere. --- README.md | 39 +++++++-------------------------------- 1 file changed, 7 insertions(+), 32 deletions(-) diff --git a/README.md b/README.md index 19aa2e7..2517a28 100644 --- a/README.md +++ b/README.md @@ -6,6 +6,13 @@ Gorilla WebSocket is a [Go](http://golang.org/) implementation of the [WebSocket](http://www.rfc-editor.org/rfc/rfc6455.txt) protocol. + +--- + +⚠️ **[The Gorilla WebSocket Package is looking for a new maintainer](https://github.com/gorilla/websocket/issues/370)** + +--- + ### Documentation * [API Reference](https://pkg.go.dev/github.com/gorilla/websocket?tab=doc) @@ -30,35 +37,3 @@ The Gorilla WebSocket package passes the server tests in the [Autobahn Test Suite](https://github.com/crossbario/autobahn-testsuite) using the application in the [examples/autobahn subdirectory](https://github.com/gorilla/websocket/tree/master/examples/autobahn). -### Gorilla WebSocket compared with other packages - - - - - - - - - - - - - - - - - - -
github.com/gorillagolang.org/x/net
RFC 6455 Features
Passes Autobahn Test SuiteYesNo
Receive fragmented messageYesNo, see note 1
Send close messageYesNo
Send pings and receive pongsYesNo
Get the type of a received data messageYesYes, see note 2
Other Features
Compression ExtensionsExperimentalNo
Read message using io.ReaderYesNo, see note 3
Write message using io.WriteCloserYesNo, see note 3
- -Notes: - -1. Large messages are fragmented in [Chrome's new WebSocket implementation](http://www.ietf.org/mail-archive/web/hybi/current/msg10503.html). -2. The application can get the type of a received data message by implementing - a [Codec marshal](http://godoc.org/golang.org/x/net/websocket#Codec.Marshal) - function. -3. The go.net io.Reader and io.Writer operate across WebSocket frame boundaries. - Read returns when the input buffer is full or a frame boundary is - encountered. Each call to Write sends a single frame message. The Gorilla - io.Reader and io.WriteCloser operate on a single WebSocket message. - From 9111bb834a68b893cebbbaed5060bdbc1d9ab7d2 Mon Sep 17 00:00:00 2001 From: Lluis Campos Date: Tue, 4 Jan 2022 02:59:52 +0100 Subject: [PATCH 14/21] Dialer: add optional method NetDialTLSContext (#746) Fixes issue: https://github.com/gorilla/websocket/issues/745 With the previous interface, NetDial and NetDialContext were used for both TLS and non-TLS TCP connections, and afterwards TLSClientConfig was used to do the TLS handshake. While this API works for most cases, it prevents from using more advance authentication methods during the TLS handshake, as this is out of the control of the user. This commits introduces another a new dial method, NetDialTLSContext, which is used when dialing for TLS/TCP. The code then assumes that the handshake is done there and TLSClientConfig is not used. This API change is fully backwards compatible and it better aligns with net/http.Transport API, which has these two dial flavors. See: https://pkg.go.dev/net/http#Transport Signed-off-by: Lluis Campos --- client.go | 45 +++++++++-- client_server_test.go | 178 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 215 insertions(+), 8 deletions(-) diff --git a/client.go b/client.go index ecbe584..2efd835 100644 --- a/client.go +++ b/client.go @@ -56,9 +56,15 @@ type Dialer struct { NetDial func(network, addr string) (net.Conn, error) // NetDialContext specifies the dial function for creating TCP connections. If - // NetDialContext is nil, net.DialContext is used. + // NetDialContext is nil, NetDial is used. NetDialContext func(ctx context.Context, network, addr string) (net.Conn, error) + // NetDialTLSContext specifies the dial function for creating TLS/TCP connections. If + // NetDialTLSContext is nil, NetDialContext is used. + // If NetDialTLSContext is set, Dial assumes the TLS handshake is done there and + // TLSClientConfig is ignored. + NetDialTLSContext func(ctx context.Context, network, addr string) (net.Conn, error) + // Proxy specifies a function to return a proxy for a given // Request. If the function returns a non-nil error, the // request is aborted with the provided error. @@ -67,6 +73,8 @@ type Dialer struct { // TLSClientConfig specifies the TLS configuration to use with tls.Client. // If nil, the default configuration is used. + // If either NetDialTLS or NetDialTLSContext are set, Dial assumes the TLS handshake + // is done there and TLSClientConfig is ignored. TLSClientConfig *tls.Config // HandshakeTimeout specifies the duration for the handshake to complete. @@ -239,13 +247,32 @@ func (d *Dialer) DialContext(ctx context.Context, urlStr string, requestHeader h // Get network dial function. var netDial func(network, add string) (net.Conn, error) - if d.NetDialContext != nil { - netDial = func(network, addr string) (net.Conn, error) { - return d.NetDialContext(ctx, network, addr) + switch u.Scheme { + case "http": + if d.NetDialContext != nil { + netDial = func(network, addr string) (net.Conn, error) { + return d.NetDialContext(ctx, network, addr) + } + } else if d.NetDial != nil { + netDial = d.NetDial } - } else if d.NetDial != nil { - netDial = d.NetDial - } else { + case "https": + if d.NetDialTLSContext != nil { + netDial = func(network, addr string) (net.Conn, error) { + return d.NetDialTLSContext(ctx, network, addr) + } + } else if d.NetDialContext != nil { + netDial = func(network, addr string) (net.Conn, error) { + return d.NetDialContext(ctx, network, addr) + } + } else if d.NetDial != nil { + netDial = d.NetDial + } + default: + return nil, nil, errMalformedURL + } + + if netDial == nil { netDialer := &net.Dialer{} netDial = func(network, addr string) (net.Conn, error) { return netDialer.DialContext(ctx, network, addr) @@ -306,7 +333,9 @@ func (d *Dialer) DialContext(ctx context.Context, urlStr string, requestHeader h } }() - if u.Scheme == "https" { + if u.Scheme == "https" && d.NetDialTLSContext == nil { + // If NetDialTLSContext is set, assume that the TLS handshake has already been done + cfg := cloneTLSConfig(d.TLSClientConfig) if cfg.ServerName == "" { cfg.ServerName = hostNoPort diff --git a/client_server_test.go b/client_server_test.go index c03f2a9..e975e51 100644 --- a/client_server_test.go +++ b/client_server_test.go @@ -11,6 +11,7 @@ import ( "crypto/x509" "encoding/base64" "encoding/binary" + "errors" "fmt" "io" "io/ioutil" @@ -920,3 +921,180 @@ func TestEmptyTracingDialWithContext(t *testing.T) { defer ws.Close() sendRecv(t, ws) } + +// TestNetDialConnect tests selection of dial method between NetDial, NetDialContext, NetDialTLS or NetDialTLSContext +func TestNetDialConnect(t *testing.T) { + + upgrader := Upgrader{} + handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if IsWebSocketUpgrade(r) { + c, err := upgrader.Upgrade(w, r, http.Header{"X-Test-Host": {r.Host}}) + if err != nil { + t.Fatal(err) + } + c.Close() + } else { + w.Header().Set("X-Test-Host", r.Host) + } + }) + + server := httptest.NewServer(handler) + defer server.Close() + + tlsServer := httptest.NewTLSServer(handler) + defer tlsServer.Close() + + testUrls := map[*httptest.Server]string{ + server: "ws://" + server.Listener.Addr().String() + "/", + tlsServer: "wss://" + tlsServer.Listener.Addr().String() + "/", + } + + cas := rootCAs(t, tlsServer) + tlsConfig := &tls.Config{ + RootCAs: cas, + ServerName: "example.com", + InsecureSkipVerify: false, + } + + tests := []struct { + name string + server *httptest.Server // server to use + netDial func(network, addr string) (net.Conn, error) + netDialContext func(ctx context.Context, network, addr string) (net.Conn, error) + netDialTLSContext func(ctx context.Context, network, addr string) (net.Conn, error) + tlsClientConfig *tls.Config + }{ + + { + name: "HTTP server, all NetDial* defined, shall use NetDialContext", + server: server, + netDial: func(network, addr string) (net.Conn, error) { + return nil, errors.New("NetDial should not be called") + }, + netDialContext: func(_ context.Context, network, addr string) (net.Conn, error) { + return net.Dial(network, addr) + }, + netDialTLSContext: func(_ context.Context, network, addr string) (net.Conn, error) { + return nil, errors.New("NetDialTLSContext should not be called") + }, + tlsClientConfig: nil, + }, + { + name: "HTTP server, all NetDial* undefined", + server: server, + netDial: nil, + netDialContext: nil, + netDialTLSContext: nil, + tlsClientConfig: nil, + }, + { + name: "HTTP server, NetDialContext undefined, shall fallback to NetDial", + server: server, + netDial: func(network, addr string) (net.Conn, error) { + return net.Dial(network, addr) + }, + netDialContext: nil, + netDialTLSContext: func(ctx context.Context, network, addr string) (net.Conn, error) { + return nil, errors.New("NetDialTLSContext should not be called") + }, + tlsClientConfig: nil, + }, + { + name: "HTTPS server, all NetDial* defined, shall use NetDialTLSContext", + server: tlsServer, + netDial: func(network, addr string) (net.Conn, error) { + return nil, errors.New("NetDial should not be called") + }, + netDialContext: func(ctx context.Context, network, addr string) (net.Conn, error) { + return nil, errors.New("NetDialContext should not be called") + }, + netDialTLSContext: func(ctx context.Context, network, addr string) (net.Conn, error) { + netConn, err := net.Dial(network, addr) + if err != nil { + return nil, err + } + tlsConn := tls.Client(netConn, tlsConfig) + err = tlsConn.Handshake() + if err != nil { + return nil, err + } + return tlsConn, nil + }, + tlsClientConfig: nil, + }, + { + name: "HTTPS server, NetDialTLSContext undefined, shall fallback to NetDialContext and do handshake", + server: tlsServer, + netDial: func(network, addr string) (net.Conn, error) { + return nil, errors.New("NetDial should not be called") + }, + netDialContext: func(ctx context.Context, network, addr string) (net.Conn, error) { + return net.Dial(network, addr) + }, + netDialTLSContext: nil, + tlsClientConfig: tlsConfig, + }, + { + name: "HTTPS server, NetDialTLSContext and NetDialContext undefined, shall fallback to NetDial and do handshake", + server: tlsServer, + netDial: func(network, addr string) (net.Conn, error) { + return net.Dial(network, addr) + }, + netDialContext: nil, + netDialTLSContext: nil, + tlsClientConfig: tlsConfig, + }, + { + name: "HTTPS server, all NetDial* undefined", + server: tlsServer, + netDial: nil, + netDialContext: nil, + netDialTLSContext: nil, + tlsClientConfig: tlsConfig, + }, + { + name: "HTTPS server, all NetDialTLSContext defined, dummy TlsClientConfig defined, shall not do handshake", + server: tlsServer, + netDial: func(network, addr string) (net.Conn, error) { + return nil, errors.New("NetDial should not be called") + }, + netDialContext: func(ctx context.Context, network, addr string) (net.Conn, error) { + return nil, errors.New("NetDialContext should not be called") + }, + netDialTLSContext: func(ctx context.Context, network, addr string) (net.Conn, error) { + netConn, err := net.Dial(network, addr) + if err != nil { + return nil, err + } + tlsConn := tls.Client(netConn, tlsConfig) + err = tlsConn.Handshake() + if err != nil { + return nil, err + } + return tlsConn, nil + }, + tlsClientConfig: &tls.Config{ + RootCAs: nil, + ServerName: "badserver.com", + InsecureSkipVerify: false, + }, + }, + } + + for _, tc := range tests { + dialer := Dialer{ + NetDial: tc.netDial, + NetDialContext: tc.netDialContext, + NetDialTLSContext: tc.netDialTLSContext, + TLSClientConfig: tc.tlsClientConfig, + } + + // Test websocket dial + c, _, err := dialer.Dial(testUrls[tc.server], nil) + if err != nil { + t.Errorf("FAILED %s, err: %s", tc.name, err.Error()) + } else { + c.Close() + } + } +} From 69d0eb9187b6dead8fe84b2423518475e5cc535c Mon Sep 17 00:00:00 2001 From: Yuki Hirasawa <48427044+hirasawayuki@users.noreply.github.com> Date: Wed, 16 Feb 2022 10:15:20 +0900 Subject: [PATCH 15/21] Add check for Sec-WebSocket-Key header (#752) * add Sec-WebSocket-Key header verification * add testcase to Sec-WebSocket-Key header verification --- server.go | 4 ++-- util.go | 15 +++++++++++++++ util_test.go | 19 +++++++++++++++++++ 3 files changed, 36 insertions(+), 2 deletions(-) diff --git a/server.go b/server.go index 24d53b3..bb33597 100644 --- a/server.go +++ b/server.go @@ -154,8 +154,8 @@ func (u *Upgrader) Upgrade(w http.ResponseWriter, r *http.Request, responseHeade } challengeKey := r.Header.Get("Sec-Websocket-Key") - if challengeKey == "" { - return u.returnError(w, r, http.StatusBadRequest, "websocket: not a websocket handshake: 'Sec-WebSocket-Key' header is missing or blank") + if !isValidChallengeKey(challengeKey) { + return u.returnError(w, r, http.StatusBadRequest, "websocket: not a websocket handshake: 'Sec-WebSocket-Key' header must be Base64 encoded value of 16-byte in length") } subprotocol := u.selectSubprotocol(r, responseHeader) diff --git a/util.go b/util.go index 7bf2f66..31a5dee 100644 --- a/util.go +++ b/util.go @@ -281,3 +281,18 @@ headers: } return result } + +// isValidChallengeKey checks if the argument meets RFC6455 specification. +func isValidChallengeKey(s string) bool { + // From RFC6455: + // + // A |Sec-WebSocket-Key| header field with a base64-encoded (see + // Section 4 of [RFC4648]) value that, when decoded, is 16 bytes in + // length. + + if s == "" { + return false + } + decoded, err := base64.StdEncoding.DecodeString(s) + return err == nil && len(decoded) == 16 +} diff --git a/util_test.go b/util_test.go index af710ba..f14d69a 100644 --- a/util_test.go +++ b/util_test.go @@ -53,6 +53,25 @@ func TestTokenListContainsValue(t *testing.T) { } } +var isValidChallengeKeyTests = []struct { + key string + ok bool +}{ + {"dGhlIHNhbXBsZSBub25jZQ==", true}, + {"", false}, + {"InvalidKey", false}, + {"WHQ4eXhscUtKYjBvOGN3WEdtOEQ=", false}, +} + +func TestIsValidChallengeKey(t *testing.T) { + for _, tt := range isValidChallengeKeyTests { + ok := isValidChallengeKey(tt.key) + if ok != tt.ok { + t.Errorf("isValidChallengeKey returns %v, want %v", ok, tt.ok) + } + } +} + var parseExtensionTests = []struct { value string extensions []map[string]string From 78cf1bc733a927f673fd1988a25256b425552a8a Mon Sep 17 00:00:00 2001 From: JWSong Date: Sun, 17 Apr 2022 22:01:17 +0900 Subject: [PATCH 16/21] Changed the method name UnderlyingConn to NetConn to align the methods names with Go 1.18 standard library (#773) --- conn.go | 8 ++++++++ conn_test.go | 12 +++++++++++- server_test.go | 2 +- 3 files changed, 20 insertions(+), 2 deletions(-) diff --git a/conn.go b/conn.go index 331eebc..5161ef8 100644 --- a/conn.go +++ b/conn.go @@ -1189,8 +1189,16 @@ func (c *Conn) SetPongHandler(h func(appData string) error) { c.handlePong = h } +// NetConn returns the underlying connection that is wrapped by c. +// Note that writing to or reading from this connection directly will corrupt the +// WebSocket connection. +func (c *Conn) NetConn() net.Conn { + return c.conn +} + // UnderlyingConn returns the internal net.Conn. This can be used to further // modifications to connection specific flags. +// Deprecated: Use the NetConn method. func (c *Conn) UnderlyingConn() net.Conn { return c.conn } diff --git a/conn_test.go b/conn_test.go index bd96e0a..06e5184 100644 --- a/conn_test.go +++ b/conn_test.go @@ -562,7 +562,7 @@ func TestAddrs(t *testing.T) { } } -func TestUnderlyingConn(t *testing.T) { +func TestDeprecatedUnderlyingConn(t *testing.T) { var b1, b2 bytes.Buffer fc := fakeNetConn{Reader: &b1, Writer: &b2} c := newConn(fc, true, 1024, 1024, nil, nil, nil) @@ -572,6 +572,16 @@ func TestUnderlyingConn(t *testing.T) { } } +func TestNetConn(t *testing.T) { + var b1, b2 bytes.Buffer + fc := fakeNetConn{Reader: &b1, Writer: &b2} + c := newConn(fc, true, 1024, 1024, nil, nil, nil) + ul := c.NetConn() + if ul != fc { + t.Fatalf("Underlying conn is not what it should be.") + } +} + func TestBufioReadBytes(t *testing.T) { // Test calling bufio.ReadBytes for value longer than read buffer size. diff --git a/server_test.go b/server_test.go index 3029aa8..5804be1 100644 --- a/server_test.go +++ b/server_test.go @@ -111,7 +111,7 @@ func TestBufioReuse(t *testing.T) { if reuse := c.br == br; reuse != tt.reuse { t.Errorf("%d: buffered reader reuse=%v, want %v", i, reuse, tt.reuse) } - writeBuf := bufioWriterBuffer(c.UnderlyingConn(), bw) + writeBuf := bufioWriterBuffer(c.NetConn(), bw) if reuse := &c.writeBuf[0] == &writeBuf[0]; reuse != tt.reuse { t.Errorf("%d: write buffer reuse=%v, want %v", i, reuse, tt.reuse) } From 27d91a9be56520b2ebae7ea508b8afdf1d191f41 Mon Sep 17 00:00:00 2001 From: Chan Kang Date: Sun, 19 Jun 2022 20:27:09 -0400 Subject: [PATCH 17/21] drop the versions of go that are no longer supported + add 1.18 to ci --- .circleci/config.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index a0eb0ed..ecb33f6 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -16,8 +16,8 @@ jobs: type: string default: "" docker: - - image: "circleci/golang:<< parameters.version >>" - working_directory: /go/src/github.com/gorilla/websocket + - image: "cimg/go:<< parameters.version >>" + working_directory: /home/circleci/project/go/src/github.com/gorilla/websocket environment: GO111MODULE: "on" GOPROXY: "<< parameters.goproxy >>" @@ -67,4 +67,4 @@ workflows: - test: matrix: parameters: - version: ["latest", "1.17", "1.16", "1.15", "1.14", "1.13", "1.12", "1.11"] + version: ["1.18", "1.17", "1.16"] From bc7ce893c36595e095de550a211feb5993e6ef92 Mon Sep 17 00:00:00 2001 From: Chan Kang Date: Tue, 21 Jun 2022 13:54:14 -0400 Subject: [PATCH 18/21] Check for and report bad protocol in TLSClientConfig.NextProtos (#788) * return an error when Dialer.TLSClientConfig.NextProtos contains a protocol that is not http/1.1 * include the likely cause of the error in the error message * check for nil-ness of Dialer.TLSClientConfig before attempting to run the check * addressing the review * move the NextProtos test into a separate file so that it can be run conditionally on go versions >= 1.14 * moving the new error check into existing http response error block to reduce the possibility of false positives * wrapping the error in %w * using %v instead of %w for compatibility with older versions of go * Revert "using %v instead of %w for compatibility with older versions of go" This reverts commit d34dd940eeb29b6f4d21d3ab9148893b4019afd1. * move the unit test back into the existing test code since golang build constraint is no longer necessary Co-authored-by: Chan Kang --- client.go | 12 ++++++++++++ client_server_test.go | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/client.go b/client.go index 2efd835..f79ac98 100644 --- a/client.go +++ b/client.go @@ -9,6 +9,7 @@ import ( "context" "crypto/tls" "errors" + "fmt" "io" "io/ioutil" "net" @@ -370,6 +371,17 @@ func (d *Dialer) DialContext(ctx context.Context, urlStr string, requestHeader h resp, err := http.ReadResponse(conn.br, req) if err != nil { + if d.TLSClientConfig != nil { + for _, proto := range d.TLSClientConfig.NextProtos { + if proto != "http/1.1" { + return nil, nil, fmt.Errorf( + "websocket: protocol %q was given but is not supported;"+ + "sharing tls.Config with net/http Transport can cause this error: %w", + proto, err, + ) + } + } + } return nil, nil, err } diff --git a/client_server_test.go b/client_server_test.go index e975e51..a47df48 100644 --- a/client_server_test.go +++ b/client_server_test.go @@ -1098,3 +1098,38 @@ func TestNetDialConnect(t *testing.T) { } } } +func TestNextProtos(t *testing.T) { + ts := httptest.NewUnstartedServer( + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}), + ) + ts.EnableHTTP2 = true + ts.StartTLS() + defer ts.Close() + + d := Dialer{ + TLSClientConfig: ts.Client().Transport.(*http.Transport).TLSClientConfig, + } + + r, err := ts.Client().Get(ts.URL) + if err != nil { + t.Fatalf("Get: %v", err) + } + r.Body.Close() + + // Asserts that Dialer.TLSClientConfig.NextProtos contains "h2" + // after the Client.Get call from net/http above. + var containsHTTP2 bool = false + for _, proto := range d.TLSClientConfig.NextProtos { + if proto == "h2" { + containsHTTP2 = true + } + } + if !containsHTTP2 { + t.Fatalf("Dialer.TLSClientConfig.NextProtos does not contain \"h2\"") + } + + _, _, err = d.Dial(makeWsProto(ts.URL), nil) + if err == nil { + t.Fatalf("Dial succeeded, expect fail ") + } +} From af47554f343b4675b30172ac301638d350db34a5 Mon Sep 17 00:00:00 2001 From: Ye Sijun Date: Wed, 6 Jul 2022 16:19:30 +0800 Subject: [PATCH 19/21] check error before GotConn for trace Signed-off-by: Ye Sijun --- client.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/client.go b/client.go index f79ac98..04fdafe 100644 --- a/client.go +++ b/client.go @@ -319,14 +319,14 @@ func (d *Dialer) DialContext(ctx context.Context, urlStr string, requestHeader h } netConn, err := netDial("tcp", hostPort) + if err != nil { + return nil, nil, err + } if trace != nil && trace.GotConn != nil { trace.GotConn(httptrace.GotConnInfo{ Conn: netConn, }) } - if err != nil { - return nil, nil, err - } defer func() { if netConn != nil { From 76ecc29eff79f0cedf70c530605e486fc32131d1 Mon Sep 17 00:00:00 2001 From: Matt Silverlock Date: Fri, 9 Dec 2022 11:03:16 -0500 Subject: [PATCH 20/21] archive mode --- README.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/README.md b/README.md index 2517a28..4484ac1 100644 --- a/README.md +++ b/README.md @@ -6,10 +6,9 @@ Gorilla WebSocket is a [Go](http://golang.org/) implementation of the [WebSocket](http://www.rfc-editor.org/rfc/rfc6455.txt) protocol. - --- -⚠️ **[The Gorilla WebSocket Package is looking for a new maintainer](https://github.com/gorilla/websocket/issues/370)** +**The Gorilla project has been archived, and is no longer under active maintainenance. You can read more here: https://github.com/gorilla#gorilla-toolkit** --- From 931041c5ee6de24fe9cba1aa16f1a0b910284d6d Mon Sep 17 00:00:00 2001 From: Corey Daley Date: Sat, 15 Jul 2023 10:54:14 -0400 Subject: [PATCH 21/21] Update README.md Signed-off-by: Corey Daley --- README.md | 5 ----- 1 file changed, 5 deletions(-) diff --git a/README.md b/README.md index 4484ac1..d33ed7f 100644 --- a/README.md +++ b/README.md @@ -6,11 +6,6 @@ Gorilla WebSocket is a [Go](http://golang.org/) implementation of the [WebSocket](http://www.rfc-editor.org/rfc/rfc6455.txt) protocol. ---- - -**The Gorilla project has been archived, and is no longer under active maintainenance. You can read more here: https://github.com/gorilla#gorilla-toolkit** - ---- ### Documentation