diff --git a/client.go b/client.go index dbca164a..6afa45ae 100644 --- a/client.go +++ b/client.go @@ -11,7 +11,6 @@ import ( "errors" "fmt" "io" - "log" "net" "net/http" @@ -294,9 +293,7 @@ func (d *Dialer) DialContext(ctx context.Context, urlStr string, requestHeader h } err = c.SetDeadline(deadline) if err != nil { - if err := c.Close(); err != nil { - log.Printf("websocket: failed to close network connection: %v", err) - } + c.Close() //#nosec G104 (CWE-703): Errors unhandled return nil, err } return c, nil @@ -336,9 +333,9 @@ func (d *Dialer) DialContext(ctx context.Context, urlStr string, requestHeader h defer func() { if netConn != nil { - if err := netConn.Close(); err != nil { - log.Printf("websocket: failed to close network connection: %v", err) - } + // As mentioned in https://github.com/gorilla/websocket/pull/897#issuecomment-1947108098: + // It's safe to ignore the errors for netconn.Close() + netConn.Close() //#nosec G104 (CWE-703): Errors unhandled } }() @@ -429,10 +426,8 @@ func (d *Dialer) DialContext(ctx context.Context, urlStr string, requestHeader h resp.Body = io.NopCloser(bytes.NewReader([]byte{})) conn.subprotocol = resp.Header.Get("Sec-Websocket-Protocol") - if err := netConn.SetDeadline(time.Time{}); err != nil { - return nil, nil, err - } - netConn = nil // to avoid close in defer. + netConn.SetDeadline(time.Time{}) //#nosec G104 (CWE-703): Errors unhandled + netConn = nil // to avoid close in defer. return conn, resp, nil } diff --git a/client_server_test.go b/client_server_test.go index 2c848a4b..eb5ec90c 100644 --- a/client_server_test.go +++ b/client_server_test.go @@ -92,6 +92,7 @@ func (t cstHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { } ws, err := cstUpgrader.Upgrade(w, r, http.Header{"Set-Cookie": {"sessionID=1234"}}) if err != nil { + t.Logf("Upgrade: %v", err) return } defer ws.Close() @@ -103,16 +104,20 @@ func (t cstHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { } op, rd, err := ws.NextReader() if err != nil { + t.Logf("NextReader: %v", err) return } wr, err := ws.NextWriter(op) if err != nil { + t.Logf("NextWriter: %v", err) return } if _, err = io.Copy(wr, rd); err != nil { + t.Logf("Copy: %v", err) return } if err := wr.Close(); err != nil { + t.Logf("Close: %v", err) return } } @@ -433,7 +438,7 @@ func TestDialBadOrigin(t *testing.T) { if resp == nil { t.Fatalf("resp=nil, err=%v", err) } - if resp.StatusCode != http.StatusForbidden { + if resp.StatusCode != http.StatusForbidden { // nolint:staticcheck t.Fatalf("status=%d, want %d", resp.StatusCode, http.StatusForbidden) } } @@ -539,9 +544,7 @@ func TestRespOnBadHandshake(t *testing.T) { s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(expectedStatus) - if _, err := io.WriteString(w, expectedBody); err != nil { - t.Fatalf("WriteString: %v", err) - } + io.WriteString(w, expectedBody) // nolint:errcheck })) defer s.Close() @@ -551,11 +554,11 @@ func TestRespOnBadHandshake(t *testing.T) { t.Fatalf("Dial: nil") } - if resp == nil { + if resp == nil { // nolint:staticcheck t.Fatalf("resp=nil, err=%v", err) } - if resp.StatusCode != expectedStatus { + if resp.StatusCode != expectedStatus { // nolint:staticcheck t.Errorf("resp.StatusCode=%d, want %d", resp.StatusCode, expectedStatus) } @@ -574,6 +577,7 @@ type testLogWriter struct { } func (w testLogWriter) Write(p []byte) (int, error) { + w.t.Logf("%s", p) return len(p), nil } @@ -793,10 +797,7 @@ func TestSocksProxyDial(t *testing.T) { } defer c1.Close() - if err := c1.SetDeadline(time.Now().Add(30 * time.Second)); err != nil { - t.Errorf("set deadline failed: %v", err) - return - } + c1.SetDeadline(time.Now().Add(30 * time.Second)) // nolint:errcheck buf := make([]byte, 32) if _, err := io.ReadFull(c1, buf[:3]); err != nil { @@ -835,15 +836,10 @@ func TestSocksProxyDial(t *testing.T) { defer c2.Close() done := make(chan struct{}) go func() { - if _, err := io.Copy(c1, c2); err != nil { - t.Errorf("copy failed: %v", err) - } + io.Copy(c1, c2) // nolint:errcheck close(done) }() - if _, err := io.Copy(c2, c1); err != nil { - t.Errorf("copy failed: %v", err) - return - } + io.Copy(c2, c1) // nolint:errcheck <-done }() diff --git a/compression.go b/compression.go index 4630dba9..4a36783c 100644 --- a/compression.go +++ b/compression.go @@ -33,9 +33,7 @@ func decompressNoContextTakeover(r io.Reader) io.ReadCloser { "\x01\x00\x00\xff\xff" fr, _ := flateReaderPool.Get().(io.ReadCloser) - if err := fr.(flate.Resetter).Reset(io.MultiReader(r, strings.NewReader(tail)), nil); err != nil { - panic(err) - } + fr.(flate.Resetter).Reset(io.MultiReader(r, strings.NewReader(tail)), nil) //#nosec G104 (CWE-703): Errors unhandled return &flateReadWrapper{fr} } @@ -134,7 +132,7 @@ func (r *flateReadWrapper) Read(p []byte) (int, error) { // Preemptively place the reader back in the pool. This helps with // scenarios where the application does not call NextReader() soon after // this final read. - _ = r.Close() + r.Close() //#nosec G104 (CWE-703): Errors unhandled } return n, err } diff --git a/compression_test.go b/compression_test.go index 410fb984..309f06a3 100644 --- a/compression_test.go +++ b/compression_test.go @@ -23,9 +23,7 @@ func TestTruncWriter(t *testing.T) { if m > n { m = n } - if _, err := w.Write(p[:m]); err != nil { - t.Fatal(err) - } + w.Write(p[:m]) // nolint:errcheck p = p[m:] } if b.String() != data[:len(data)-len(w.p)] { @@ -49,9 +47,7 @@ func BenchmarkWriteNoCompression(b *testing.B) { messages := textMessages(100) b.ResetTimer() for i := 0; i < b.N; i++ { - if err := c.WriteMessage(TextMessage, messages[i%len(messages)]); err != nil { - b.Fatal(err) - } + c.WriteMessage(TextMessage, messages[i%len(messages)]) // nolint:errcheck } b.ReportAllocs() } @@ -64,9 +60,7 @@ func BenchmarkWriteWithCompression(b *testing.B) { c.newCompressionWriter = compressNoContextTakeover b.ResetTimer() for i := 0; i < b.N; i++ { - if err := c.WriteMessage(TextMessage, messages[i%len(messages)]); err != nil { - b.Fatal(err) - } + c.WriteMessage(TextMessage, messages[i%len(messages)]) // nolint:errcheck } b.ReportAllocs() } diff --git a/conn.go b/conn.go index efe89e4a..d1504eb1 100644 --- a/conn.go +++ b/conn.go @@ -934,9 +934,7 @@ func (c *Conn) advanceFrame() (int, error) { } if c.readLimit > 0 && c.readLength > c.readLimit { - if err := c.WriteControl(CloseMessage, FormatCloseMessage(CloseMessageTooBig, ""), time.Now().Add(writeWait)); err != nil { - return noFrame, err - } + c.WriteControl(CloseMessage, FormatCloseMessage(CloseMessageTooBig, ""), time.Now().Add(writeWait)) //#nosec G104 (CWE-703): Errors unhandled return noFrame, ErrReadLimit } @@ -997,9 +995,7 @@ func (c *Conn) handleProtocolError(message string) error { if len(data) > maxControlFramePayloadSize { data = data[:maxControlFramePayloadSize] } - if err := c.WriteControl(CloseMessage, data, time.Now().Add(writeWait)); err != nil { - return err - } + c.WriteControl(CloseMessage, data, time.Now().Add(writeWait)) //#nosec G104 (CWE-703): Errors unhandled return errors.New("websocket: " + message) } diff --git a/proxy.go b/proxy.go index b6af21f9..0bdb5a0c 100644 --- a/proxy.go +++ b/proxy.go @@ -8,7 +8,6 @@ import ( "bufio" "encoding/base64" "errors" - "log" "net" "net/http" "net/url" @@ -58,9 +57,9 @@ func (hpd *httpProxyDialer) Dial(network string, addr string) (net.Conn, error) } if err := connectReq.Write(conn); err != nil { - if err := conn.Close(); err != nil { - log.Printf("httpProxyDialer: failed to close connection: %v", err) - } + // As mentioned in https://github.com/gorilla/websocket/pull/897#issuecomment-1947108098: + // It's safe to ignore the errors for conn.Close() + conn.Close() //#nosec G104 (CWE-703): Errors unhandled return nil, err } @@ -69,16 +68,12 @@ func (hpd *httpProxyDialer) Dial(network string, addr string) (net.Conn, error) br := bufio.NewReader(conn) resp, err := http.ReadResponse(br, connectReq) if err != nil { - if err := conn.Close(); err != nil { - log.Printf("httpProxyDialer: failed to close connection: %v", err) - } + conn.Close() //#nosec G104 (CWE-703): Errors unhandled return nil, err } if resp.StatusCode != http.StatusOK { - if err := conn.Close(); err != nil { - log.Printf("httpProxyDialer: failed to close connection: %v", err) - } + conn.Close() //#nosec G104 (CWE-703): Errors unhandled f := strings.SplitN(resp.Status, " ", 2) return nil, errors.New(f[1]) } diff --git a/server.go b/server.go index 357de704..94eb4885 100644 --- a/server.go +++ b/server.go @@ -180,9 +180,9 @@ func (u *Upgrader) Upgrade(w http.ResponseWriter, r *http.Request, responseHeade } if brw.Reader.Buffered() > 0 { - if err := netConn.Close(); err != nil { - log.Printf("websocket: failed to close network connection: %v", err) - } + // As mentioned in https://github.com/gorilla/websocket/pull/897#issuecomment-1947108098: + // It's safe to ignore the errors for netconn.Close() + netConn.Close() //#nosec G104 (CWE-703): Errors unhandled return nil, errors.New("websocket: client sent data before handshake is complete") } @@ -248,31 +248,23 @@ func (u *Upgrader) Upgrade(w http.ResponseWriter, r *http.Request, responseHeade // Clear deadlines set by HTTP server. if err := netConn.SetDeadline(time.Time{}); err != nil { - if err := netConn.Close(); err != nil { - log.Printf("websocket: failed to close network connection: %v", err) - } + netConn.Close() //#nosec G104 (CWE-703): Errors unhandled return nil, err } if u.HandshakeTimeout > 0 { if err := netConn.SetWriteDeadline(time.Now().Add(u.HandshakeTimeout)); err != nil { - if err := netConn.Close(); err != nil { - log.Printf("websocket: failed to close network connection: %v", err) - } + netConn.Close() //#nosec G104 (CWE-703): Errors unhandled return nil, err } } if _, err = netConn.Write(p); err != nil { - if err := netConn.Close(); err != nil { - log.Printf("websocket: failed to close network connection: %v", err) - } + netConn.Close() //#nosec G104 (CWE-703): Errors unhandled return nil, err } if u.HandshakeTimeout > 0 { if err := netConn.SetWriteDeadline(time.Time{}); err != nil { - if err := netConn.Close(); err != nil { - log.Printf("websocket: failed to close network connection: %v", err) - } + netConn.Close() //#nosec G104 (CWE-703): Errors unhandled return nil, err } }