Merge eb890c85fd into 1d5465562b

The package ignored errors from net.Conn Set*Deadline in a few places.
Update the package to return these errors to the caller.

Ignore all other errors reported by errcheck. These errors are safe to
ignore because
- The function is making a best effort to cleanup while handling another
  error.
- The function call is guaranteed to succeed.
- The error is ignored in a test.

client.go

@@ -305,9 +305,15 @@ func (d *Dialer) DialContext(ctx context.Context, urlStr string, requestHeader h
 		})
 	}
 
+	// Close the network connection when returning an error. The variable
+	// netConn is set to nil before the success return at the end of the
+	// function.
 	defer func() {
 		if netConn != nil {
-			netConn.Close()
+			// It's safe to ignore the error from Close() because this code is
+			// only executed when returning a more important error to the
+			// application.
+			_ = netConn.Close()
 		}
 	}()
 
@@ -399,11 +405,11 @@ func (d *Dialer) DialContext(ctx context.Context, urlStr string, requestHeader h
 	conn.subprotocol = resp.Header.Get("Sec-Websocket-Protocol")
 
 	if err := netConn.SetDeadline(time.Time{}); err != nil {
-		return nil, nil, err
+		return nil, resp, err
 	}
 
-	// Set netConn to nil to avoid call to netConn.Close() in
-	// deferred function call.
+	// Success! Set netConn to nil to stop the deferred function above from
+	// closing the network connection.
 	netConn = nil
 
 	return conn, resp, nil

conn.go

@@ -371,7 +371,9 @@ func (c *Conn) read(n int) ([]byte, error) {
 	if err == io.EOF {
 		err = errUnexpectedEOF
 	}
-	_, _ = c.br.Discard(len(p)) // guaranteed to succeed
+	// Discard is guaranteed to succeed because the number of bytes to discard
+	// is less than or equal to the number of bytes buffered.
+	_, _ = c.br.Discard(len(p))
 	return p, err
 }
 
@@ -819,7 +821,7 @@ func (c *Conn) advanceFrame() (int, error) {
 	rsv2 := p[0]&rsv2Bit != 0
 	rsv3 := p[0]&rsv3Bit != 0
 	mask := p[1]&maskBit != 0
-	_ = c.setReadRemaining(int64(p[1] & 0x7f)) // will not overflow
+	_ = c.setReadRemaining(int64(p[1] & 0x7f)) // will not fail because argument is >= 0
 
 	c.readDecompress = false
 	if rsv1 {
@@ -937,7 +939,7 @@ func (c *Conn) advanceFrame() (int, error) {
 	var payload []byte
 	if c.readRemaining > 0 {
 		payload, err = c.read(int(c.readRemaining))
-		_ = c.setReadRemaining(0) // will not overflow
+		_ = c.setReadRemaining(0) // will not fail because argument is >= 0
 		if err != nil {
 			return noFrame, err
 		}
@@ -1058,7 +1060,7 @@ func (r *messageReader) Read(b []byte) (int, error) {
 			}
 			rem := c.readRemaining
 			rem -= int64(n)
-			_ = c.setReadRemaining(rem) // will not overflow
+			_ = c.setReadRemaining(rem) // rem is guaranteed to be >= 0
 			if c.readRemaining > 0 && c.readErr == io.EOF {
 				c.readErr = errUnexpectedEOF
 			}

server.go

@@ -178,10 +178,13 @@ func (u *Upgrader) Upgrade(w http.ResponseWriter, r *http.Request, responseHeade
 			"websocket: hijack: "+err.Error())
 	}
 
+	// Close the network connection when returning an error. The variable
+	// netConn is set to nil before the success return at the end of the
+	// function.
 	defer func() {
 		if netConn != nil {
 			// It's safe to ignore the error from Close() because this code is
-			// only executed when returning a more important to the
+			// only executed when returning a more important error to the
 			// application.
 			_ = netConn.Close()
 		}
@@ -271,8 +274,8 @@ func (u *Upgrader) Upgrade(w http.ResponseWriter, r *http.Request, responseHeade
 		}
 	}
 
-	// Set netConn to nil to avoid call to netConn.Close() in
-	// deferred function call.
+	// Success! Set netConn to nil to stop the deferred function above from
+	// closing the network connection.
 	netConn = nil
 
 	return c, nil