From 227456c3cc00808ec7480f02d8f7cc794b3529d5 Mon Sep 17 00:00:00 2001 From: Daniel Holmes Date: Wed, 19 Jun 2024 04:30:39 +0000 Subject: [PATCH 1/7] chore: Retract v1.5.2 from go.mod Maintainers accidentally changed the reference commit for v1.5.2. This change retracts v1.5.2 which also includes a number of avoidable issues. Fixes #927 --- go.mod | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/go.mod b/go.mod index 1a7afd5..22d2668 100644 --- a/go.mod +++ b/go.mod @@ -1,3 +1,7 @@ module github.com/gorilla/websocket go 1.12 + +retract ( + v1.5.2 // tag accidentally overwritten +) \ No newline at end of file From ac1b326ac0ae2f53411189133a884ade0649c05c Mon Sep 17 00:00:00 2001 From: Canelo Hill <172609632+canelohill@users.noreply.github.com> Date: Tue, 18 Jun 2024 22:40:57 -0600 Subject: [PATCH 2/7] Set min Go version to 1.20 (#930) Update go.mod and CI to Go version 1.20. --- .circleci/config.yml | 2 +- go.mod | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index ecb33f6..ebd12c1 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -67,4 +67,4 @@ workflows: - test: matrix: parameters: - version: ["1.18", "1.17", "1.16"] + version: ["1.22", "1.21", "1.20"] diff --git a/go.mod b/go.mod index 22d2668..dba1e22 100644 --- a/go.mod +++ b/go.mod @@ -1,7 +1,7 @@ module github.com/gorilla/websocket -go 1.12 +go 1.20 retract ( v1.5.2 // tag accidentally overwritten -) \ No newline at end of file +) From a70cea529a3a07f6bf467a2129225a44fb44162f Mon Sep 17 00:00:00 2001 From: Canelo Hill <172609632+canelohill@users.noreply.github.com> Date: Tue, 18 Jun 2024 22:44:41 -0600 Subject: [PATCH 3/7] Update for deprecated ioutil package (#931) --- client.go | 5 ++--- client_server_test.go | 3 +-- compression_test.go | 5 ++--- conn.go | 5 ++--- conn_broadcast_test.go | 3 +-- conn_test.go | 11 +++++------ examples/autobahn/server.go | 2 +- examples/filewatch/main.go | 3 +-- 8 files changed, 15 insertions(+), 22 deletions(-) diff --git a/client.go b/client.go index 04fdafe..170301d 100644 --- a/client.go +++ b/client.go @@ -11,7 +11,6 @@ import ( "errors" "fmt" "io" - "io/ioutil" "net" "net/http" "net/http/httptrace" @@ -400,7 +399,7 @@ func (d *Dialer) DialContext(ctx context.Context, urlStr string, requestHeader h // debugging. buf := make([]byte, 1024) n, _ := io.ReadFull(resp.Body, buf) - resp.Body = ioutil.NopCloser(bytes.NewReader(buf[:n])) + resp.Body = io.NopCloser(bytes.NewReader(buf[:n])) return nil, resp, ErrBadHandshake } @@ -418,7 +417,7 @@ func (d *Dialer) DialContext(ctx context.Context, urlStr string, requestHeader h break } - resp.Body = ioutil.NopCloser(bytes.NewReader([]byte{})) + resp.Body = io.NopCloser(bytes.NewReader([]byte{})) conn.subprotocol = resp.Header.Get("Sec-Websocket-Protocol") netConn.SetDeadline(time.Time{}) diff --git a/client_server_test.go b/client_server_test.go index a47df48..67dd346 100644 --- a/client_server_test.go +++ b/client_server_test.go @@ -14,7 +14,6 @@ import ( "errors" "fmt" "io" - "io/ioutil" "log" "net" "net/http" @@ -549,7 +548,7 @@ func TestRespOnBadHandshake(t *testing.T) { t.Errorf("resp.StatusCode=%d, want %d", resp.StatusCode, expectedStatus) } - p, err := ioutil.ReadAll(resp.Body) + p, err := io.ReadAll(resp.Body) if err != nil { t.Fatalf("ReadFull(resp.Body) returned error %v", err) } diff --git a/compression_test.go b/compression_test.go index 8a26b30..23591c4 100644 --- a/compression_test.go +++ b/compression_test.go @@ -4,7 +4,6 @@ import ( "bytes" "fmt" "io" - "io/ioutil" "testing" ) @@ -42,7 +41,7 @@ func textMessages(num int) [][]byte { } func BenchmarkWriteNoCompression(b *testing.B) { - w := ioutil.Discard + w := io.Discard c := newTestConn(nil, w, false) messages := textMessages(100) b.ResetTimer() @@ -53,7 +52,7 @@ func BenchmarkWriteNoCompression(b *testing.B) { } func BenchmarkWriteWithCompression(b *testing.B) { - w := ioutil.Discard + w := io.Discard c := newTestConn(nil, w, false) messages := textMessages(100) c.enableWriteCompression = true diff --git a/conn.go b/conn.go index 5161ef8..9353252 100644 --- a/conn.go +++ b/conn.go @@ -9,7 +9,6 @@ import ( "encoding/binary" "errors" "io" - "io/ioutil" "math/rand" "net" "strconv" @@ -795,7 +794,7 @@ func (c *Conn) advanceFrame() (int, error) { // 1. Skip remainder of previous frame. if c.readRemaining > 0 { - if _, err := io.CopyN(ioutil.Discard, c.br, c.readRemaining); err != nil { + if _, err := io.CopyN(io.Discard, c.br, c.readRemaining); err != nil { return noFrame, err } } @@ -1094,7 +1093,7 @@ func (c *Conn) ReadMessage() (messageType int, p []byte, err error) { if err != nil { return messageType, nil, err } - p, err = ioutil.ReadAll(r) + p, err = io.ReadAll(r) return messageType, p, err } diff --git a/conn_broadcast_test.go b/conn_broadcast_test.go index 6e744fc..d8a6492 100644 --- a/conn_broadcast_test.go +++ b/conn_broadcast_test.go @@ -6,7 +6,6 @@ package websocket import ( "io" - "io/ioutil" "sync/atomic" "testing" ) @@ -45,7 +44,7 @@ func newBroadcastConn(c *Conn) *broadcastConn { func newBroadcastBench(usePrepared, compression bool) *broadcastBench { bench := &broadcastBench{ - w: ioutil.Discard, + w: io.Discard, doneCh: make(chan struct{}), closeCh: make(chan struct{}), usePrepared: usePrepared, diff --git a/conn_test.go b/conn_test.go index 06e5184..e9f5441 100644 --- a/conn_test.go +++ b/conn_test.go @@ -10,7 +10,6 @@ import ( "errors" "fmt" "io" - "io/ioutil" "net" "reflect" "sync" @@ -125,7 +124,7 @@ func TestFraming(t *testing.T) { } t.Logf("frame size: %d", n) - rbuf, err := ioutil.ReadAll(r) + rbuf, err := io.ReadAll(r) if err != nil { t.Errorf("%s: ReadFull() returned rbuf, %v", name, err) continue @@ -367,7 +366,7 @@ func TestCloseFrameBeforeFinalMessageFrame(t *testing.T) { if op != BinaryMessage || err != nil { t.Fatalf("NextReader() returned %d, %v", op, err) } - _, err = io.Copy(ioutil.Discard, r) + _, err = io.Copy(io.Discard, r) if !reflect.DeepEqual(err, expectedErr) { t.Fatalf("io.Copy() returned %v, want %v", err, expectedErr) } @@ -401,7 +400,7 @@ func TestEOFWithinFrame(t *testing.T) { if op != BinaryMessage || err != nil { t.Fatalf("%d: NextReader() returned %d, %v", n, op, err) } - _, err = io.Copy(ioutil.Discard, r) + _, err = io.Copy(io.Discard, r) if err != errUnexpectedEOF { t.Fatalf("%d: io.Copy() returned %v, want %v", n, err, errUnexpectedEOF) } @@ -426,7 +425,7 @@ func TestEOFBeforeFinalFrame(t *testing.T) { if op != BinaryMessage || err != nil { t.Fatalf("NextReader() returned %d, %v", op, err) } - _, err = io.Copy(ioutil.Discard, r) + _, err = io.Copy(io.Discard, r) if err != errUnexpectedEOF { t.Fatalf("io.Copy() returned %v, want %v", err, errUnexpectedEOF) } @@ -490,7 +489,7 @@ func TestReadLimit(t *testing.T) { if op != BinaryMessage || err != nil { t.Fatalf("2: NextReader() returned %d, %v", op, err) } - _, err = io.Copy(ioutil.Discard, r) + _, err = io.Copy(io.Discard, r) if err != ErrReadLimit { t.Fatalf("io.Copy() returned %v", err) } diff --git a/examples/autobahn/server.go b/examples/autobahn/server.go index 8b17fe3..2d6d36f 100644 --- a/examples/autobahn/server.go +++ b/examples/autobahn/server.go @@ -84,7 +84,7 @@ func echoCopyFull(w http.ResponseWriter, r *http.Request) { } // echoReadAll echoes messages from the client by reading the entire message -// with ioutil.ReadAll. +// with io.ReadAll. func echoReadAll(w http.ResponseWriter, r *http.Request, writeMessage, writePrepared bool) { conn, err := upgrader.Upgrade(w, r, nil) if err != nil { diff --git a/examples/filewatch/main.go b/examples/filewatch/main.go index d4bf80e..57d5a0b 100644 --- a/examples/filewatch/main.go +++ b/examples/filewatch/main.go @@ -7,7 +7,6 @@ package main import ( "flag" "html/template" - "io/ioutil" "log" "net/http" "os" @@ -49,7 +48,7 @@ func readFileIfModified(lastMod time.Time) ([]byte, time.Time, error) { if !fi.ModTime().After(lastMod) { return nil, lastMod, nil } - p, err := ioutil.ReadFile(filename) + p, err := os.ReadFile(filename) if err != nil { return nil, fi.ModTime(), err } From c7502098b0f8461511d811d27c26075165735b05 Mon Sep 17 00:00:00 2001 From: merlin Date: Wed, 22 Nov 2023 21:47:04 +0200 Subject: [PATCH 4/7] use http.ResposnseController --- server.go | 10 ++++------ server_test.go | 22 ++++++++++++++++++++++ 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/server.go b/server.go index bb33597..69e6f83 100644 --- a/server.go +++ b/server.go @@ -172,13 +172,11 @@ func (u *Upgrader) Upgrade(w http.ResponseWriter, r *http.Request, responseHeade } } - h, ok := w.(http.Hijacker) - if !ok { + netConn, brw, err := http.NewResponseController(w).Hijack() + switch { + case errors.Is(err, errors.ErrUnsupported): return u.returnError(w, r, http.StatusInternalServerError, "websocket: response does not implement http.Hijacker") - } - var brw *bufio.ReadWriter - netConn, brw, err := h.Hijack() - if err != nil { + case err != nil: return u.returnError(w, r, http.StatusInternalServerError, err.Error()) } diff --git a/server_test.go b/server_test.go index 5804be1..2ce6f7f 100644 --- a/server_test.go +++ b/server_test.go @@ -7,8 +7,10 @@ package websocket import ( "bufio" "bytes" + "errors" "net" "net/http" + "net/http/httptest" "reflect" "strings" "testing" @@ -117,3 +119,23 @@ func TestBufioReuse(t *testing.T) { } } } + +func TestHijack_NotSupported(t *testing.T) { + t.Parallel() + + req := httptest.NewRequest(http.MethodGet, "http://example.com", nil) + req.Header.Set("Upgrade", "websocket") + req.Header.Set("Connection", "upgrade") + req.Header.Set("Sec-Websocket-Key", "dGhlIHNhbXBsZSBub25jZQ==") + req.Header.Set("Sec-Websocket-Version", "13") + + recorder := httptest.NewRecorder() + + upgrader := Upgrader{} + _, err := upgrader.Upgrade(recorder, req, nil) + + if want := (HandshakeError{}); !errors.As(err, &want) || recorder.Code != http.StatusInternalServerError { + t.Errorf("want %T and status_code=%d", want, http.StatusInternalServerError) + t.Fatalf("got err=%T and status_code=%d", err, recorder.Code) + } +} From 8890e3e578e96aa953e60bcbcf8a07082f9f9784 Mon Sep 17 00:00:00 2001 From: merlin Date: Wed, 13 Dec 2023 22:28:24 +0200 Subject: [PATCH 5/7] fix: don't use errors.ErrUnsupported, it's available only since go1.21 --- server.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/server.go b/server.go index 69e6f83..3ecb2d9 100644 --- a/server.go +++ b/server.go @@ -173,10 +173,7 @@ func (u *Upgrader) Upgrade(w http.ResponseWriter, r *http.Request, responseHeade } netConn, brw, err := http.NewResponseController(w).Hijack() - switch { - case errors.Is(err, errors.ErrUnsupported): - return u.returnError(w, r, http.StatusInternalServerError, "websocket: response does not implement http.Hijacker") - case err != nil: + if err != nil { return u.returnError(w, r, http.StatusInternalServerError, err.Error()) } From 7e5e9b5a25ec2d8cb251bbc3de64f7d57691da4e Mon Sep 17 00:00:00 2001 From: tebuka <171117698+tebuka@users.noreply.github.com> Date: Wed, 12 Jun 2024 21:15:45 -0700 Subject: [PATCH 6/7] Improve hijack failure error text Include "hijack" in text to indicate where in this package the error occurred. --- server.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/server.go b/server.go index 3ecb2d9..3e3826b 100644 --- a/server.go +++ b/server.go @@ -174,7 +174,8 @@ func (u *Upgrader) Upgrade(w http.ResponseWriter, r *http.Request, responseHeade netConn, brw, err := http.NewResponseController(w).Hijack() if err != nil { - return u.returnError(w, r, http.StatusInternalServerError, err.Error()) + return u.returnError(w, r, http.StatusInternalServerError, + "websocket: hijack: "+err.Error()) } if brw.Reader.Buffered() > 0 { From 688592ebe68e20f1256de8f920e0ade52a182e2d Mon Sep 17 00:00:00 2001 From: Canelo Hill <172609632+canelohill@users.noreply.github.com> Date: Tue, 18 Jun 2024 21:42:48 -0600 Subject: [PATCH 7/7] Improve client/server tests Tests must not call *testing.T methods after the test function returns. Use a sync.WaitGroup to ensure that server handler functions complete before tests return. --- client_server_test.go | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/client_server_test.go b/client_server_test.go index 67dd346..610fbe2 100644 --- a/client_server_test.go +++ b/client_server_test.go @@ -23,6 +23,7 @@ import ( "net/url" "reflect" "strings" + "sync" "testing" "time" ) @@ -44,12 +45,15 @@ var cstDialer = Dialer{ HandshakeTimeout: 30 * time.Second, } -type cstHandler struct{ *testing.T } +type cstHandler struct { + *testing.T + s *cstServer +} type cstServer struct { - *httptest.Server - URL string - t *testing.T + URL string + Server *httptest.Server + wg sync.WaitGroup } const ( @@ -58,9 +62,15 @@ const ( cstRequestURI = cstPath + "?" + cstRawQuery ) +func (s *cstServer) Close() { + s.Server.Close() + // Wait for handler functions to complete. + s.wg.Wait() +} + func newServer(t *testing.T) *cstServer { var s cstServer - s.Server = httptest.NewServer(cstHandler{t}) + s.Server = httptest.NewServer(cstHandler{T: t, s: &s}) s.Server.URL += cstRequestURI s.URL = makeWsProto(s.Server.URL) return &s @@ -68,13 +78,19 @@ func newServer(t *testing.T) *cstServer { func newTLSServer(t *testing.T) *cstServer { var s cstServer - s.Server = httptest.NewTLSServer(cstHandler{t}) + s.Server = httptest.NewTLSServer(cstHandler{T: t, s: &s}) s.Server.URL += cstRequestURI s.URL = makeWsProto(s.Server.URL) return &s } func (t cstHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { + // Because tests wait for a response from a server, we are guaranteed that + // the wait group count is incremented before the test waits on the group + // in the call to (*cstServer).Close(). + t.s.wg.Add(1) + defer t.s.wg.Done() + if r.URL.Path != cstPath { t.Logf("path=%v, want %v", r.URL.Path, cstPath) http.Error(w, "bad path", http.StatusBadRequest)