Skip to content

Commit 3810b23

Browse files
Canelo Hilljaitaiwan
authored andcommitted
Handle errcheck warnings
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.
1 parent a62d9d2 commit 3810b23

File tree

10 files changed

+99
-51
lines changed

10 files changed

+99
-51
lines changed

client.go

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -305,9 +305,15 @@ func (d *Dialer) DialContext(ctx context.Context, urlStr string, requestHeader h
305305
})
306306
}
307307

308+
// Close the network connection when returning an error. The variable
309+
// netConn is set to nil before the success return at the end of the
310+
// function.
308311
defer func() {
309312
if netConn != nil {
310-
netConn.Close()
313+
// It's safe to ignore the error from Close() because this code is
314+
// only executed when returning a more important error to the
315+
// application.
316+
_ = netConn.Close()
311317
}
312318
}()
313319

@@ -398,8 +404,14 @@ func (d *Dialer) DialContext(ctx context.Context, urlStr string, requestHeader h
398404
resp.Body = io.NopCloser(bytes.NewReader([]byte{}))
399405
conn.subprotocol = resp.Header.Get("Sec-Websocket-Protocol")
400406

401-
netConn.SetDeadline(time.Time{})
402-
netConn = nil // to avoid close in defer.
407+
if err := netConn.SetDeadline(time.Time{}); err != nil {
408+
return nil, resp, err
409+
}
410+
411+
// Success! Set netConn to nil to stop the deferred function above from
412+
// closing the network connection.
413+
netConn = nil
414+
403415
return conn, resp, nil
404416
}
405417

client_server_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -578,7 +578,7 @@ func TestRespOnBadHandshake(t *testing.T) {
578578

579579
s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
580580
w.WriteHeader(expectedStatus)
581-
io.WriteString(w, expectedBody)
581+
_, _ = io.WriteString(w, expectedBody)
582582
}))
583583
defer s.Close()
584584

@@ -828,7 +828,7 @@ func TestSocksProxyDial(t *testing.T) {
828828
}
829829
defer c1.Close()
830830

831-
c1.SetDeadline(time.Now().Add(30 * time.Second))
831+
_ = c1.SetDeadline(time.Now().Add(30 * time.Second))
832832

833833
buf := make([]byte, 32)
834834
if _, err := io.ReadFull(c1, buf[:3]); err != nil {
@@ -867,10 +867,10 @@ func TestSocksProxyDial(t *testing.T) {
867867
defer c2.Close()
868868
done := make(chan struct{})
869869
go func() {
870-
io.Copy(c1, c2)
870+
_, _ = io.Copy(c1, c2)
871871
close(done)
872872
}()
873-
io.Copy(c2, c1)
873+
_, _ = io.Copy(c2, c1)
874874
<-done
875875
}()
876876

compression.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,11 @@ func decompressNoContextTakeover(r io.Reader) io.ReadCloser {
3333
"\x01\x00\x00\xff\xff"
3434

3535
fr, _ := flateReaderPool.Get().(io.ReadCloser)
36-
fr.(flate.Resetter).Reset(io.MultiReader(r, strings.NewReader(tail)), nil)
36+
mr := io.MultiReader(r, strings.NewReader(tail))
37+
if err := fr.(flate.Resetter).Reset(mr, nil); err != nil {
38+
// Reset never fails, but handle error in case that changes.
39+
fr = flate.NewReader(mr)
40+
}
3741
return &flateReadWrapper{fr}
3842
}
3943

compression_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ func TestTruncWriter(t *testing.T) {
2222
if m > n {
2323
m = n
2424
}
25-
w.Write(p[:m])
25+
_, _ = w.Write(p[:m])
2626
p = p[m:]
2727
}
2828
if b.String() != data[:len(data)-len(w.p)] {
@@ -46,7 +46,7 @@ func BenchmarkWriteNoCompression(b *testing.B) {
4646
messages := textMessages(100)
4747
b.ResetTimer()
4848
for i := 0; i < b.N; i++ {
49-
c.WriteMessage(TextMessage, messages[i%len(messages)])
49+
_ = c.WriteMessage(TextMessage, messages[i%len(messages)])
5050
}
5151
b.ReportAllocs()
5252
}
@@ -59,7 +59,7 @@ func BenchmarkWriteWithCompression(b *testing.B) {
5959
c.newCompressionWriter = compressNoContextTakeover
6060
b.ResetTimer()
6161
for i := 0; i < b.N; i++ {
62-
c.WriteMessage(TextMessage, messages[i%len(messages)])
62+
_ = c.WriteMessage(TextMessage, messages[i%len(messages)])
6363
}
6464
b.ReportAllocs()
6565
}

conn.go

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -370,7 +370,9 @@ func (c *Conn) read(n int) ([]byte, error) {
370370
if err == io.EOF {
371371
err = errUnexpectedEOF
372372
}
373-
c.br.Discard(len(p))
373+
// Discard is guaranteed to succeed because the number of bytes to discard
374+
// is less than or equal to the number of bytes buffered.
375+
_, _ = c.br.Discard(len(p))
374376
return p, err
375377
}
376378

@@ -385,7 +387,9 @@ func (c *Conn) write(frameType int, deadline time.Time, buf0, buf1 []byte) error
385387
return err
386388
}
387389

388-
c.conn.SetWriteDeadline(deadline)
390+
if err := c.conn.SetWriteDeadline(deadline); err != nil {
391+
return c.writeFatal(err)
392+
}
389393
if len(buf1) == 0 {
390394
_, err = c.conn.Write(buf0)
391395
} else {
@@ -395,7 +399,7 @@ func (c *Conn) write(frameType int, deadline time.Time, buf0, buf1 []byte) error
395399
return c.writeFatal(err)
396400
}
397401
if frameType == CloseMessage {
398-
c.writeFatal(ErrCloseSent)
402+
_ = c.writeFatal(ErrCloseSent)
399403
}
400404
return nil
401405
}
@@ -458,13 +462,14 @@ func (c *Conn) WriteControl(messageType int, data []byte, deadline time.Time) er
458462
return err
459463
}
460464

461-
c.conn.SetWriteDeadline(deadline)
462-
_, err = c.conn.Write(buf)
463-
if err != nil {
465+
if err := c.conn.SetWriteDeadline(deadline); err != nil {
466+
return c.writeFatal(err)
467+
}
468+
if _, err = c.conn.Write(buf); err != nil {
464469
return c.writeFatal(err)
465470
}
466471
if messageType == CloseMessage {
467-
c.writeFatal(ErrCloseSent)
472+
_ = c.writeFatal(ErrCloseSent)
468473
}
469474
return err
470475
}
@@ -628,7 +633,7 @@ func (w *messageWriter) flushFrame(final bool, extra []byte) error {
628633
}
629634

630635
if final {
631-
w.endMessage(errWriteClosed)
636+
_ = w.endMessage(errWriteClosed)
632637
return nil
633638
}
634639

@@ -815,7 +820,7 @@ func (c *Conn) advanceFrame() (int, error) {
815820
rsv2 := p[0]&rsv2Bit != 0
816821
rsv3 := p[0]&rsv3Bit != 0
817822
mask := p[1]&maskBit != 0
818-
c.setReadRemaining(int64(p[1] & 0x7f))
823+
_ = c.setReadRemaining(int64(p[1] & 0x7f)) // will not fail because argument is >= 0
819824

820825
c.readDecompress = false
821826
if rsv1 {
@@ -920,7 +925,8 @@ func (c *Conn) advanceFrame() (int, error) {
920925
}
921926

922927
if c.readLimit > 0 && c.readLength > c.readLimit {
923-
c.WriteControl(CloseMessage, FormatCloseMessage(CloseMessageTooBig, ""), time.Now().Add(writeWait))
928+
// Make a best effort to send a close message describing the problem.
929+
_ = c.WriteControl(CloseMessage, FormatCloseMessage(CloseMessageTooBig, ""), time.Now().Add(writeWait))
924930
return noFrame, ErrReadLimit
925931
}
926932

@@ -932,7 +938,7 @@ func (c *Conn) advanceFrame() (int, error) {
932938
var payload []byte
933939
if c.readRemaining > 0 {
934940
payload, err = c.read(int(c.readRemaining))
935-
c.setReadRemaining(0)
941+
_ = c.setReadRemaining(0) // will not fail because argument is >= 0
936942
if err != nil {
937943
return noFrame, err
938944
}
@@ -979,7 +985,8 @@ func (c *Conn) handleProtocolError(message string) error {
979985
if len(data) > maxControlFramePayloadSize {
980986
data = data[:maxControlFramePayloadSize]
981987
}
982-
c.WriteControl(CloseMessage, data, time.Now().Add(writeWait))
988+
// Make a best effor to send a close message describing the problem.
989+
_ = c.WriteControl(CloseMessage, data, time.Now().Add(writeWait))
983990
return errors.New("websocket: " + message)
984991
}
985992

@@ -1052,7 +1059,7 @@ func (r *messageReader) Read(b []byte) (int, error) {
10521059
}
10531060
rem := c.readRemaining
10541061
rem -= int64(n)
1055-
c.setReadRemaining(rem)
1062+
_ = c.setReadRemaining(rem) // rem is guaranteed to be >= 0
10561063
if c.readRemaining > 0 && c.readErr == io.EOF {
10571064
c.readErr = errUnexpectedEOF
10581065
}
@@ -1134,7 +1141,8 @@ func (c *Conn) SetCloseHandler(h func(code int, text string) error) {
11341141
if h == nil {
11351142
h = func(code int, text string) error {
11361143
message := FormatCloseMessage(code, "")
1137-
c.WriteControl(CloseMessage, message, time.Now().Add(writeWait))
1144+
// Make a best effor to send the close message.
1145+
_ = c.WriteControl(CloseMessage, message, time.Now().Add(writeWait))
11381146
return nil
11391147
}
11401148
}

conn_broadcast_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,9 +69,9 @@ func (b *broadcastBench) makeConns(numConns int) {
6969
select {
7070
case msg := <-c.msgCh:
7171
if msg.prepared != nil {
72-
c.conn.WritePreparedMessage(msg.prepared)
72+
_ = c.conn.WritePreparedMessage(msg.prepared)
7373
} else {
74-
c.conn.WriteMessage(TextMessage, msg.payload)
74+
_ = c.conn.WriteMessage(TextMessage, msg.payload)
7575
}
7676
val := atomic.AddInt32(&b.count, 1)
7777
if val%int32(numConns) == 0 {

conn_test.go

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ func TestControl(t *testing.T) {
157157
wc := newTestConn(nil, &connBuf, isServer)
158158
rc := newTestConn(&connBuf, nil, !isServer)
159159
if isWriteControl {
160-
wc.WriteControl(PongMessage, []byte(message), time.Now().Add(time.Second))
160+
_ = wc.WriteControl(PongMessage, []byte(message), time.Now().Add(time.Second))
161161
} else {
162162
w, err := wc.NextWriter(PongMessage)
163163
if err != nil {
@@ -174,7 +174,7 @@ func TestControl(t *testing.T) {
174174
}
175175
var actualMessage string
176176
rc.SetPongHandler(func(s string) error { actualMessage = s; return nil })
177-
rc.NextReader()
177+
_, _, _ = rc.NextReader()
178178
if actualMessage != message {
179179
t.Errorf("%s: pong=%q, want %q", name, actualMessage, message)
180180
continue
@@ -358,8 +358,8 @@ func TestCloseFrameBeforeFinalMessageFrame(t *testing.T) {
358358
rc := newTestConn(&b1, &b2, true)
359359

360360
w, _ := wc.NextWriter(BinaryMessage)
361-
w.Write(make([]byte, bufSize+bufSize/2))
362-
wc.WriteControl(CloseMessage, FormatCloseMessage(expectedErr.Code, expectedErr.Text), time.Now().Add(10*time.Second))
361+
_, _ = w.Write(make([]byte, bufSize+bufSize/2))
362+
_ = wc.WriteControl(CloseMessage, FormatCloseMessage(expectedErr.Code, expectedErr.Text), time.Now().Add(10*time.Second))
363363
w.Close()
364364

365365
op, r, err := rc.NextReader()
@@ -385,7 +385,7 @@ func TestEOFWithinFrame(t *testing.T) {
385385
rc := newTestConn(&b, nil, true)
386386

387387
w, _ := wc.NextWriter(BinaryMessage)
388-
w.Write(make([]byte, bufSize))
388+
_, _ = w.Write(make([]byte, bufSize))
389389
w.Close()
390390

391391
if n >= b.Len() {
@@ -419,7 +419,7 @@ func TestEOFBeforeFinalFrame(t *testing.T) {
419419
rc := newTestConn(&b1, &b2, true)
420420

421421
w, _ := wc.NextWriter(BinaryMessage)
422-
w.Write(make([]byte, bufSize+bufSize/2))
422+
_, _ = w.Write(make([]byte, bufSize+bufSize/2))
423423

424424
op, r, err := rc.NextReader()
425425
if op != BinaryMessage || err != nil {
@@ -438,7 +438,7 @@ func TestEOFBeforeFinalFrame(t *testing.T) {
438438
func TestWriteAfterMessageWriterClose(t *testing.T) {
439439
wc := newTestConn(nil, &bytes.Buffer{}, false)
440440
w, _ := wc.NextWriter(BinaryMessage)
441-
io.WriteString(w, "hello")
441+
_, _ = io.WriteString(w, "hello")
442442
if err := w.Close(); err != nil {
443443
t.Fatalf("unxpected error closing message writer, %v", err)
444444
}
@@ -448,7 +448,7 @@ func TestWriteAfterMessageWriterClose(t *testing.T) {
448448
}
449449

450450
w, _ = wc.NextWriter(BinaryMessage)
451-
io.WriteString(w, "hello")
451+
_, _ = io.WriteString(w, "hello")
452452

453453
// close w by getting next writer
454454
_, err := wc.NextWriter(BinaryMessage)
@@ -473,13 +473,13 @@ func TestReadLimit(t *testing.T) {
473473

474474
// Send message at the limit with interleaved pong.
475475
w, _ := wc.NextWriter(BinaryMessage)
476-
w.Write(message[:readLimit-1])
477-
wc.WriteControl(PongMessage, []byte("this is a pong"), time.Now().Add(10*time.Second))
478-
w.Write(message[:1])
476+
_, _ = w.Write(message[:readLimit-1])
477+
_ = wc.WriteControl(PongMessage, []byte("this is a pong"), time.Now().Add(10*time.Second))
478+
_, _ = w.Write(message[:1])
479479
w.Close()
480480

481481
// Send message larger than the limit.
482-
wc.WriteMessage(BinaryMessage, message[:readLimit+1])
482+
_ = wc.WriteMessage(BinaryMessage, message[:readLimit+1])
483483

484484
op, _, err := rc.NextReader()
485485
if op != BinaryMessage || err != nil {
@@ -592,7 +592,7 @@ func TestBufioReadBytes(t *testing.T) {
592592
rc := newConn(fakeNetConn{Reader: &b1, Writer: &b2}, true, len(m)-64, len(m)-64, nil, nil, nil)
593593

594594
w, _ := wc.NextWriter(BinaryMessage)
595-
w.Write(m)
595+
_, _ = w.Write(m)
596596
w.Close()
597597

598598
op, r, err := rc.NextReader()
@@ -666,7 +666,7 @@ func TestConcurrentWritePanic(t *testing.T) {
666666
w := blockingWriter{make(chan struct{}), make(chan struct{})}
667667
c := newTestConn(nil, w, false)
668668
go func() {
669-
c.WriteMessage(TextMessage, []byte{})
669+
_ = c.WriteMessage(TextMessage, []byte{})
670670
}()
671671

672672
// wait for goroutine to block in write.
@@ -679,7 +679,7 @@ func TestConcurrentWritePanic(t *testing.T) {
679679
}
680680
}()
681681

682-
c.WriteMessage(TextMessage, []byte{})
682+
_ = c.WriteMessage(TextMessage, []byte{})
683683
t.Fatal("should not get here")
684684
}
685685

@@ -699,7 +699,7 @@ func TestFailedConnectionReadPanic(t *testing.T) {
699699
}()
700700

701701
for i := 0; i < 20000; i++ {
702-
c.ReadMessage()
702+
_, _, _ = c.ReadMessage()
703703
}
704704
t.Fatal("should not get here")
705705
}

join_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ func TestJoinMessages(t *testing.T) {
1919
wc := newTestConn(nil, &connBuf, true)
2020
rc := newTestConn(&connBuf, nil, false)
2121
for _, m := range messages {
22-
wc.WriteMessage(BinaryMessage, []byte(m))
22+
_ = wc.WriteMessage(BinaryMessage, []byte(m))
2323
}
2424

2525
var result bytes.Buffer

prepared_test.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,9 @@ func TestPreparedMessage(t *testing.T) {
4545
if tt.enableWriteCompression {
4646
c.newCompressionWriter = compressNoContextTakeover
4747
}
48-
c.SetCompressionLevel(tt.compressionLevel)
48+
if err := c.SetCompressionLevel(tt.compressionLevel); err != nil {
49+
t.Fatal(err)
50+
}
4951

5052
// Seed random number generator for consistent frame mask.
5153
testRand.Seed(1234)

0 commit comments

Comments
 (0)