removing error handling while closing connections

This commit is contained in:
apoorvajagtap 2024-03-11 12:36:55 +05:30 committed by Alex Vulaj
parent 58af150309
commit 09a6bab466
7 changed files with 48 additions and 53 deletions

View File

@ -293,7 +293,8 @@ func (d *Dialer) DialContext(ctx context.Context, urlStr string, requestHeader h
} }
err = c.SetDeadline(deadline) err = c.SetDeadline(deadline)
if err != nil { if err != nil {
return nil, errors.Join(err, c.Close()) c.Close() //#nosec G104 (CWE-703): Errors unhandled
return nil, err
} }
return c, nil return c, nil
} }
@ -332,7 +333,9 @@ func (d *Dialer) DialContext(ctx context.Context, urlStr string, requestHeader h
defer func() { defer func() {
if netConn != nil { 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{})) resp.Body = io.NopCloser(bytes.NewReader([]byte{}))
conn.subprotocol = resp.Header.Get("Sec-Websocket-Protocol") conn.subprotocol = resp.Header.Get("Sec-Websocket-Protocol")
if err := netConn.SetDeadline(time.Time{}); err != nil { netConn.SetDeadline(time.Time{}) //#nosec G104 (CWE-703): Errors unhandled
return nil, nil, err netConn = nil // to avoid close in defer.
} return conn, resp, nil
netConn = nil // to avoid close in defer.
return conn, resp, err
} }
func cloneTLSConfig(cfg *tls.Config) *tls.Config { func cloneTLSConfig(cfg *tls.Config) *tls.Config {

View File

@ -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"}}) ws, err := cstUpgrader.Upgrade(w, r, http.Header{"Set-Cookie": {"sessionID=1234"}})
if err != nil { if err != nil {
t.Logf("Upgrade: %v", err)
return return
} }
defer ws.Close() defer ws.Close()
@ -103,16 +104,20 @@ func (t cstHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
} }
op, rd, err := ws.NextReader() op, rd, err := ws.NextReader()
if err != nil { if err != nil {
t.Logf("NextReader: %v", err)
return return
} }
wr, err := ws.NextWriter(op) wr, err := ws.NextWriter(op)
if err != nil { if err != nil {
t.Logf("NextWriter: %v", err)
return return
} }
if _, err = io.Copy(wr, rd); err != nil { if _, err = io.Copy(wr, rd); err != nil {
t.Logf("Copy: %v", err)
return return
} }
if err := wr.Close(); err != nil { if err := wr.Close(); err != nil {
t.Logf("Close: %v", err)
return return
} }
} }
@ -430,7 +435,7 @@ func TestDialBadOrigin(t *testing.T) {
ws.Close() ws.Close()
t.Fatalf("Dial: nil") t.Fatalf("Dial: nil")
} }
if resp == nil { // nolint:staticcheck if resp == nil {
t.Fatalf("resp=nil, err=%v", err) t.Fatalf("resp=nil, err=%v", err)
} }
if resp.StatusCode != http.StatusForbidden { // nolint:staticcheck 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) { s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(expectedStatus) w.WriteHeader(expectedStatus)
if _, err := io.WriteString(w, expectedBody); err != nil { io.WriteString(w, expectedBody) // nolint:errcheck
t.Fatalf("WriteString: %v", err)
}
})) }))
defer s.Close() defer s.Close()
@ -574,6 +577,7 @@ type testLogWriter struct {
} }
func (w testLogWriter) Write(p []byte) (int, error) { func (w testLogWriter) Write(p []byte) (int, error) {
w.t.Logf("%s", p)
return len(p), nil return len(p), nil
} }
@ -793,10 +797,7 @@ func TestSocksProxyDial(t *testing.T) {
} }
defer c1.Close() defer c1.Close()
if err := c1.SetDeadline(time.Now().Add(30 * time.Second)); err != nil { c1.SetDeadline(time.Now().Add(30 * time.Second)) // nolint:errcheck
t.Errorf("set deadline failed: %v", err)
return
}
buf := make([]byte, 32) buf := make([]byte, 32)
if _, err := io.ReadFull(c1, buf[:3]); err != nil { if _, err := io.ReadFull(c1, buf[:3]); err != nil {
@ -835,15 +836,10 @@ func TestSocksProxyDial(t *testing.T) {
defer c2.Close() defer c2.Close()
done := make(chan struct{}) done := make(chan struct{})
go func() { go func() {
if _, err := io.Copy(c1, c2); err != nil { io.Copy(c1, c2) // nolint:errcheck
t.Errorf("copy failed: %v", err)
}
close(done) close(done)
}() }()
if _, err := io.Copy(c2, c1); err != nil { io.Copy(c2, c1) // nolint:errcheck
t.Errorf("copy failed: %v", err)
return
}
<-done <-done
}() }()

View File

@ -33,9 +33,7 @@ func decompressNoContextTakeover(r io.Reader) io.ReadCloser {
"\x01\x00\x00\xff\xff" "\x01\x00\x00\xff\xff"
fr, _ := flateReaderPool.Get().(io.ReadCloser) fr, _ := flateReaderPool.Get().(io.ReadCloser)
if err := fr.(flate.Resetter).Reset(io.MultiReader(r, strings.NewReader(tail)), nil); err != nil { fr.(flate.Resetter).Reset(io.MultiReader(r, strings.NewReader(tail)), nil) //#nosec G104 (CWE-703): Errors unhandled
panic(err)
}
return &flateReadWrapper{fr} 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 // Preemptively place the reader back in the pool. This helps with
// scenarios where the application does not call NextReader() soon after // scenarios where the application does not call NextReader() soon after
// this final read. // this final read.
_ = r.Close() r.Close() //#nosec G104 (CWE-703): Errors unhandled
} }
return n, err return n, err
} }

View File

@ -23,9 +23,7 @@ func TestTruncWriter(t *testing.T) {
if m > n { if m > n {
m = n m = n
} }
if _, err := w.Write(p[:m]); err != nil { w.Write(p[:m]) // nolint:errcheck
t.Fatal(err)
}
p = p[m:] p = p[m:]
} }
if b.String() != data[:len(data)-len(w.p)] { if b.String() != data[:len(data)-len(w.p)] {
@ -49,9 +47,7 @@ func BenchmarkWriteNoCompression(b *testing.B) {
messages := textMessages(100) messages := textMessages(100)
b.ResetTimer() b.ResetTimer()
for i := 0; i < b.N; i++ { for i := 0; i < b.N; i++ {
if err := c.WriteMessage(TextMessage, messages[i%len(messages)]); err != nil { c.WriteMessage(TextMessage, messages[i%len(messages)]) // nolint:errcheck
b.Fatal(err)
}
} }
b.ReportAllocs() b.ReportAllocs()
} }
@ -64,9 +60,7 @@ func BenchmarkWriteWithCompression(b *testing.B) {
c.newCompressionWriter = compressNoContextTakeover c.newCompressionWriter = compressNoContextTakeover
b.ResetTimer() b.ResetTimer()
for i := 0; i < b.N; i++ { for i := 0; i < b.N; i++ {
if err := c.WriteMessage(TextMessage, messages[i%len(messages)]); err != nil { c.WriteMessage(TextMessage, messages[i%len(messages)]) // nolint:errcheck
b.Fatal(err)
}
} }
b.ReportAllocs() b.ReportAllocs()
} }

View File

@ -934,9 +934,7 @@ func (c *Conn) advanceFrame() (int, error) {
} }
if c.readLimit > 0 && c.readLength > c.readLimit { if c.readLimit > 0 && c.readLength > c.readLimit {
if err := c.WriteControl(CloseMessage, FormatCloseMessage(CloseMessageTooBig, ""), time.Now().Add(writeWait)); err != nil { c.WriteControl(CloseMessage, FormatCloseMessage(CloseMessageTooBig, ""), time.Now().Add(writeWait)) //#nosec G104 (CWE-703): Errors unhandled
return noFrame, err
}
return noFrame, ErrReadLimit return noFrame, ErrReadLimit
} }
@ -997,9 +995,7 @@ func (c *Conn) handleProtocolError(message string) error {
if len(data) > maxControlFramePayloadSize { if len(data) > maxControlFramePayloadSize {
data = data[:maxControlFramePayloadSize] data = data[:maxControlFramePayloadSize]
} }
if err := c.WriteControl(CloseMessage, data, time.Now().Add(writeWait)); err != nil { c.WriteControl(CloseMessage, data, time.Now().Add(writeWait)) //#nosec G104 (CWE-703): Errors unhandled
return err
}
return errors.New("websocket: " + message) return errors.New("websocket: " + message)
} }

View File

@ -57,7 +57,10 @@ func (hpd *httpProxyDialer) Dial(network string, addr string) (net.Conn, error)
} }
if err := connectReq.Write(conn); err != nil { 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 // Read response. It's OK to use and discard buffered reader here becaue
@ -65,12 +68,14 @@ func (hpd *httpProxyDialer) Dial(network string, addr string) (net.Conn, error)
br := bufio.NewReader(conn) br := bufio.NewReader(conn)
resp, err := http.ReadResponse(br, connectReq) resp, err := http.ReadResponse(br, connectReq)
if err != nil { 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 { if resp.StatusCode != http.StatusOK {
conn.Close() //#nosec G104 (CWE-703): Errors unhandled
f := strings.SplitN(resp.Status, " ", 2) 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 return conn, nil
} }

View File

@ -8,6 +8,7 @@ import (
"bufio" "bufio"
"errors" "errors"
"io" "io"
"log"
"net/http" "net/http"
"net/url" "net/url"
"strings" "strings"
@ -179,10 +180,10 @@ func (u *Upgrader) Upgrade(w http.ResponseWriter, r *http.Request, responseHeade
} }
if brw.Reader.Buffered() > 0 { if brw.Reader.Buffered() > 0 {
return nil, errors.Join( // As mentioned in https://github.com/gorilla/websocket/pull/897#issuecomment-1947108098:
errors.New("websocket: client sent data before handshake is complete"), // It's safe to ignore the errors for netconn.Close()
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 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. // Clear deadlines set by HTTP server.
if err := netConn.SetDeadline(time.Time{}); err != nil { 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 u.HandshakeTimeout > 0 {
if err := netConn.SetWriteDeadline(time.Now().Add(u.HandshakeTimeout)); err != nil { 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 { 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 u.HandshakeTimeout > 0 {
if err := netConn.SetWriteDeadline(time.Time{}); err != nil { 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) panic(err)
} }
if err := bw.Flush(); err != nil { if err := bw.Flush(); err != nil {
panic(err) log.Printf("websocket: bufioWriterBuffer: Flush: %v", err)
} }
bw.Reset(originalWriter) bw.Reset(originalWriter)