removing error handling while closing connections

Author: apoorvajagtap

client.go

@@ -293,7 +293,8 @@ func (d *Dialer) DialContext(ctx context.Context, urlStr string, requestHeader h
 			}
 			err = c.SetDeadline(deadline)
 			if err != nil {
-				return nil, errors.Join(err, c.Close())
+				c.Close() //#nosec G104 (CWE-703): Errors unhandled
+				return nil, err
 			}
 			return c, nil
 		}
@@ -332,7 +333,9 @@ func (d *Dialer) DialContext(ctx context.Context, urlStr string, requestHeader h
 
 	defer func() {
 		if netConn != nil {
-			netConn.Close() //#nosec:G104 (CWE-703)
+			// 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
 		}
 	}()
 
@@ -423,11 +426,9 @@ 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.
-	return conn, resp, err
+	netConn.SetDeadline(time.Time{}) //#nosec G104 (CWE-703): Errors unhandled
+	netConn = nil                    // to avoid close in defer.
+	return conn, resp, nil
 }
 
 func cloneTLSConfig(cfg *tls.Config) *tls.Config {

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
 	}
 }
@@ -430,7 +435,7 @@ func TestDialBadOrigin(t *testing.T) {
 		ws.Close()
 		t.Fatalf("Dial: nil")
 	}
-	if resp == nil { // nolint:staticcheck
+	if resp == nil {
 		t.Fatalf("resp=nil, err=%v", err)
 	}
 	if resp.StatusCode != http.StatusForbidden { // nolint:staticcheck
@@ -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()
 
@@ -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
 	}()
 

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
 }

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()
 }

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)
 }
 

proxy.go

@@ -57,20 +57,25 @@ func (hpd *httpProxyDialer) Dial(network string, addr string) (net.Conn, error)
 	}
 
 	if err := connectReq.Write(conn); err != nil {
-		return nil, errors.Join(err, conn.Close())
+		// 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
 	}
 
 	// Read response. It's OK to use and discard buffered reader here becaue
 	// the remote server does not speak until spoken to.
 	br := bufio.NewReader(conn)
 	resp, err := http.ReadResponse(br, connectReq)
 	if err != nil {
-		return nil, errors.Join(err, conn.Close())
+		conn.Close() //#nosec G104 (CWE-703): Errors unhandled
+		return nil, err
 	}
 
 	if resp.StatusCode != http.StatusOK {
+		conn.Close() //#nosec G104 (CWE-703): Errors unhandled
 		f := strings.SplitN(resp.Status, " ", 2)
-		return nil, errors.Join(errors.New(f[1]), conn.Close())
+		return nil, errors.New(f[1])
 	}
 	return conn, nil
 }

server.go

@@ -8,6 +8,7 @@ import (
 	"bufio"
 	"errors"
 	"io"
+	"log"
 	"net/http"
 	"net/url"
 	"strings"
@@ -179,10 +180,10 @@ func (u *Upgrader) Upgrade(w http.ResponseWriter, r *http.Request, responseHeade
 	}
 
 	if brw.Reader.Buffered() > 0 {
-		return nil, errors.Join(
-			errors.New("websocket: client sent data before handshake is complete"),
-			netConn.Close(),
-		)
+		// 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")
 	}
 
 	var br *bufio.Reader
@@ -247,20 +248,24 @@ 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 {
-		return nil, errors.Join(err, netConn.Close())
+		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 {
-			return nil, errors.Join(err, netConn.Close())
+			netConn.Close() //#nosec G104 (CWE-703): Errors unhandled
+			return nil, err
 		}
 	}
 	if _, err = netConn.Write(p); err != nil {
-		return nil, errors.Join(err, netConn.Close())
+		netConn.Close() //#nosec G104 (CWE-703): Errors unhandled
+		return nil, err
 	}
 	if u.HandshakeTimeout > 0 {
 		if err := netConn.SetWriteDeadline(time.Time{}); err != nil {
-			return nil, errors.Join(err, netConn.Close())
+			netConn.Close() //#nosec G104 (CWE-703): Errors unhandled
+			return nil, err
 		}
 	}
 
@@ -363,7 +368,7 @@ func bufioWriterBuffer(originalWriter io.Writer, bw *bufio.Writer) []byte {
 		panic(err)
 	}
 	if err := bw.Flush(); err != nil {
-		panic(err)
+		log.Printf("websocket: bufioWriterBuffer: Flush: %v", err)
 	}
 
 	bw.Reset(originalWriter)