Skip to content

Commit

Permalink
removing error handling while closing connections
Browse files Browse the repository at this point in the history
  • Loading branch information
apoorvajagtap committed Mar 11, 2024
1 parent ff3115a commit 471f923
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 15 deletions.
9 changes: 6 additions & 3 deletions client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Check warning on line 296 in client.go

View check run for this annotation

Codecov / codecov/patch

client.go#L296

Added line #L296 was not covered by tests
return nil, err
}
return c, nil
}
Expand Down Expand Up @@ -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
}
}()

Expand Down Expand Up @@ -427,7 +430,7 @@ func (d *Dialer) DialContext(ctx context.Context, urlStr string, requestHeader h
return nil, nil, err
}
netConn = nil // to avoid close in defer.
return conn, resp, err
return conn, resp, nil
}

func cloneTLSConfig(cfg *tls.Config) *tls.Config {
Expand Down
11 changes: 8 additions & 3 deletions proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Check warning on line 62 in proxy.go

View check run for this annotation

Codecov / codecov/patch

proxy.go#L60-L62

Added lines #L60 - L62 were not covered by tests
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

Check warning on line 71 in proxy.go

View check run for this annotation

Codecov / codecov/patch

proxy.go#L71

Added line #L71 was not covered by tests
return nil, err
}

if resp.StatusCode != http.StatusOK {
conn.Close() //#nosec G104 (CWE-703): Errors unhandled

Check warning on line 76 in proxy.go

View check run for this annotation

Codecov / codecov/patch

proxy.go#L76

Added line #L76 was not covered by tests
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
}
23 changes: 14 additions & 9 deletions server.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"bufio"
"errors"
"io"
"log"
"net/http"
"net/url"
"strings"
Expand Down Expand Up @@ -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

Check warning on line 185 in server.go

View check run for this annotation

Codecov / codecov/patch

server.go#L183-L185

Added lines #L183 - L185 were not covered by tests
return nil, errors.New("websocket: client sent data before handshake is complete")
}

var br *bufio.Reader
Expand Down Expand Up @@ -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

Check warning on line 251 in server.go

View check run for this annotation

Codecov / codecov/patch

server.go#L251

Added line #L251 was not covered by tests
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

Check warning on line 257 in server.go

View check run for this annotation

Codecov / codecov/patch

server.go#L257

Added line #L257 was not covered by tests
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

Check warning on line 262 in server.go

View check run for this annotation

Codecov / codecov/patch

server.go#L262

Added line #L262 was not covered by tests
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

Check warning on line 267 in server.go

View check run for this annotation

Codecov / codecov/patch

server.go#L267

Added line #L267 was not covered by tests
return nil, err
}
}

Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit 471f923

Please sign in to comment.