removing error handling while closing connections

apoorvajagtap · apoorvajagtap · commit c8a6785b304a · 2024-03-19T23:43:22.000+05:30
diff --git a/client.go b/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 {
diff --git 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
 	}
 }
@@ -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
 	}()
 
diff --git 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
@@ -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
@@ -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
@@ -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
 }
diff --git a/server.go b/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)

Original file line number	Diff line number	Diff line change
`@@ -33,9 +33,7 @@ func decompressNoContextTakeover(r io.Reader) io.ReadCloser {`
`33`	`33`	`"\x01\x00\x00\xff\xff"`
`34`	`34`
`35`	`35`	`fr, _ := flateReaderPool.Get().(io.ReadCloser)`
`36`		`- if err := fr.(flate.Resetter).Reset(io.MultiReader(r, strings.NewReader(tail)), nil); err != nil {`
`37`		`- panic(err)`
`38`		`- }`
	`36`	`+ fr.(flate.Resetter).Reset(io.MultiReader(r, strings.NewReader(tail)), nil) //#nosec G104 (CWE-703): Errors unhandled`
`39`	`37`	`return &flateReadWrapper{fr}`
`40`	`38`	`}`
`41`	`39`
`@@ -134,7 +132,7 @@ func (r *flateReadWrapper) Read(p []byte) (int, error) {`
`134`	`132`	`// Preemptively place the reader back in the pool. This helps with`
`135`	`133`	`// scenarios where the application does not call NextReader() soon after`
`136`	`134`	`// this final read.`
`137`		`- _ = r.Close()`
	`135`	`+ r.Close() //#nosec G104 (CWE-703): Errors unhandled`
`138`	`136`	`}`
`139`	`137`	`return n, err`
`140`	`138`	`}`
Original file line number	Diff line number	Diff line change
`@@ -23,9 +23,7 @@ func TestTruncWriter(t *testing.T) {`
`23`	`23`	`if m > n {`
`24`	`24`	`m = n`
`25`	`25`	`}`
`26`		`- if _, err := w.Write(p[:m]); err != nil {`
`27`		`- t.Fatal(err)`
`28`		`- }`
	`26`	`+ w.Write(p[:m]) // nolint:errcheck`
`29`	`27`	`p = p[m:]`
`30`	`28`	`}`
`31`	`29`	`if b.String() != data[:len(data)-len(w.p)] {`
`@@ -49,9 +47,7 @@ func BenchmarkWriteNoCompression(b *testing.B) {`
`49`	`47`	`messages := textMessages(100)`
`50`	`48`	`b.ResetTimer()`
`51`	`49`	`for i := 0; i < b.N; i++ {`
`52`		`- if err := c.WriteMessage(TextMessage, messages[i%len(messages)]); err != nil {`
`53`		`- b.Fatal(err)`
`54`		`- }`
	`50`	`+ c.WriteMessage(TextMessage, messages[i%len(messages)]) // nolint:errcheck`
`55`	`51`	`}`
`56`	`52`	`b.ReportAllocs()`
`57`	`53`	`}`
`@@ -64,9 +60,7 @@ func BenchmarkWriteWithCompression(b *testing.B) {`
`64`	`60`	`c.newCompressionWriter = compressNoContextTakeover`
`65`	`61`	`b.ResetTimer()`
`66`	`62`	`for i := 0; i < b.N; i++ {`
`67`		`- if err := c.WriteMessage(TextMessage, messages[i%len(messages)]); err != nil {`
`68`		`- b.Fatal(err)`
`69`		`- }`
	`63`	`+ c.WriteMessage(TextMessage, messages[i%len(messages)]) // nolint:errcheck`
`70`	`64`	`}`
`71`	`65`	`b.ReportAllocs()`
`72`	`66`	`}`
Original file line number	Diff line number	Diff line change
`@@ -934,9 +934,7 @@ func (c *Conn) advanceFrame() (int, error) {`
`934`	`934`	`}`
`935`	`935`
`936`	`936`	`if c.readLimit > 0 && c.readLength > c.readLimit {`
`937`		`- if err := c.WriteControl(CloseMessage, FormatCloseMessage(CloseMessageTooBig, ""), time.Now().Add(writeWait)); err != nil {`
`938`		`- return noFrame, err`
`939`		`- }`
	`937`	`+ c.WriteControl(CloseMessage, FormatCloseMessage(CloseMessageTooBig, ""), time.Now().Add(writeWait)) //#nosec G104 (CWE-703): Errors unhandled`
`940`	`938`	`return noFrame, ErrReadLimit`
`941`	`939`	`}`
`942`	`940`
`@@ -997,9 +995,7 @@ func (c *Conn) handleProtocolError(message string) error {`
`997`	`995`	`if len(data) > maxControlFramePayloadSize {`
`998`	`996`	`data = data[:maxControlFramePayloadSize]`
`999`	`997`	`}`
`1000`		`- if err := c.WriteControl(CloseMessage, data, time.Now().Add(writeWait)); err != nil {`
`1001`		`- return err`
`1002`		`- }`
	`998`	`+ c.WriteControl(CloseMessage, data, time.Now().Add(writeWait)) //#nosec G104 (CWE-703): Errors unhandled`
`1003`	`999`	`return errors.New("websocket: " + message)`
`1004`	`1000`	`}`
`1005`	`1001`
Original file line number	Diff line number	Diff line change
`@@ -57,20 +57,25 @@ func (hpd *httpProxyDialer) Dial(network string, addr string) (net.Conn, error)`
`57`	`57`	`}`
`58`	`58`
`59`	`59`	`if err := connectReq.Write(conn); err != nil {`
`60`		`- return nil, errors.Join(err, conn.Close())`
	`60`	`+ // As mentioned in https://github.com/gorilla/websocket/pull/897#issuecomment-1947108098:`
	`61`	`+ // It's safe to ignore the errors for conn.Close()`
	`62`	`+ conn.Close() //#nosec G104 (CWE-703): Errors unhandled`
	`63`	`+ return nil, err`
`61`	`64`	`}`
`62`	`65`
`63`	`66`	`// Read response. It's OK to use and discard buffered reader here becaue`
`64`	`67`	`// the remote server does not speak until spoken to.`
`65`	`68`	`br := bufio.NewReader(conn)`
`66`	`69`	`resp, err := http.ReadResponse(br, connectReq)`
`67`	`70`	`if err != nil {`
`68`		`- return nil, errors.Join(err, conn.Close())`
	`71`	`+ conn.Close() //#nosec G104 (CWE-703): Errors unhandled`
	`72`	`+ return nil, err`
`69`	`73`	`}`
`70`	`74`
`71`	`75`	`if resp.StatusCode != http.StatusOK {`
	`76`	`+ conn.Close() //#nosec G104 (CWE-703): Errors unhandled`
`72`	`77`	`f := strings.SplitN(resp.Status, " ", 2)`
`73`		`- return nil, errors.Join(errors.New(f[1]), conn.Close())`
	`78`	`+ return nil, errors.New(f[1])`
`74`	`79`	`}`
`75`	`80`	`return conn, nil`
`76`	`81`	`}`
Original file line number	Diff line number	Diff line change
`@@ -8,6 +8,7 @@ import (`
`8`	`8`	`"bufio"`
`9`	`9`	`"errors"`
`10`	`10`	`"io"`
	`11`	`+ "log"`
`11`	`12`	`"net/http"`
`12`	`13`	`"net/url"`
`13`	`14`	`"strings"`
`@@ -179,10 +180,10 @@ func (u Upgrader) Upgrade(w http.ResponseWriter, r http.Request, responseHeade`
`179`	`180`	`}`
`180`	`181`
`181`	`182`	`if brw.Reader.Buffered() > 0 {`
`182`		`- return nil, errors.Join(`
`183`		`- errors.New("websocket: client sent data before handshake is complete"),`
`184`		`- netConn.Close(),`
`185`		`- )`
	`183`	`+ // As mentioned in https://github.com/gorilla/websocket/pull/897#issuecomment-1947108098:`
	`184`	`+ // It's safe to ignore the errors for netconn.Close()`
	`185`	`+ netConn.Close() //#nosec G104 (CWE-703): Errors unhandled`
	`186`	`+ return nil, errors.New("websocket: client sent data before handshake is complete")`
`186`	`187`	`}`
`187`	`188`
`188`	`189`	`var br *bufio.Reader`
`@@ -247,20 +248,24 @@ func (u Upgrader) Upgrade(w http.ResponseWriter, r http.Request, responseHeade`
`247`	`248`
`248`	`249`	`// Clear deadlines set by HTTP server.`
`249`	`250`	`if err := netConn.SetDeadline(time.Time{}); err != nil {`
`250`		`- return nil, errors.Join(err, netConn.Close())`
	`251`	`+ netConn.Close() //#nosec G104 (CWE-703): Errors unhandled`
	`252`	`+ return nil, err`
`251`	`253`	`}`
`252`	`254`
`253`	`255`	`if u.HandshakeTimeout > 0 {`
`254`	`256`	`if err := netConn.SetWriteDeadline(time.Now().Add(u.HandshakeTimeout)); err != nil {`
`255`		`- return nil, errors.Join(err, netConn.Close())`
	`257`	`+ netConn.Close() //#nosec G104 (CWE-703): Errors unhandled`
	`258`	`+ return nil, err`
`256`	`259`	`}`
`257`	`260`	`}`
`258`	`261`	`if _, err = netConn.Write(p); err != nil {`
`259`		`- return nil, errors.Join(err, netConn.Close())`
	`262`	`+ netConn.Close() //#nosec G104 (CWE-703): Errors unhandled`
	`263`	`+ return nil, err`
`260`	`264`	`}`
`261`	`265`	`if u.HandshakeTimeout > 0 {`
`262`	`266`	`if err := netConn.SetWriteDeadline(time.Time{}); err != nil {`
`263`		`- return nil, errors.Join(err, netConn.Close())`
	`267`	`+ netConn.Close() //#nosec G104 (CWE-703): Errors unhandled`
	`268`	`+ return nil, err`
`264`	`269`	`}`
`265`	`270`	`}`
`266`	`271`
`@@ -363,7 +368,7 @@ func bufioWriterBuffer(originalWriter io.Writer, bw *bufio.Writer) []byte {`
`363`	`368`	`panic(err)`
`364`	`369`	`}`
`365`	`370`	`if err := bw.Flush(); err != nil {`
`366`		`- panic(err)`
	`371`	`+ log.Printf("websocket: bufioWriterBuffer: Flush: %v", err)`
`367`	`372`	`}`
`368`	`373`
`369`	`374`	`bw.Reset(originalWriter)`