From 3470a06c1357df533e251f14d3a181f67396fe35 Mon Sep 17 00:00:00 2001 From: Tom Bergan Date: Fri, 26 May 2017 09:42:14 -0700 Subject: [PATCH] http2: fix nil dereference after Read completes with an error Case happens if Read is called after it has already returned an error previously. Verified that the new TestPipeCloseWithError test fails before this change but passes after. Updates golang/go#20501 Change-Id: I636fbb194f2d0019b0722556cc25a88da2d18e13 Reviewed-on: https://go-review.googlesource.com/44330 Run-TryBot: Tom Bergan TryBot-Result: Gobot Gobot Reviewed-by: Brad Fitzpatrick --- http2/pipe.go | 2 +- http2/pipe_test.go | 13 ++++++++++--- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/http2/pipe.go b/http2/pipe.go index 0b9848be..a6140099 100644 --- a/http2/pipe.go +++ b/http2/pipe.go @@ -50,7 +50,7 @@ func (p *pipe) Read(d []byte) (n int, err error) { if p.breakErr != nil { return 0, p.breakErr } - if p.b.Len() > 0 { + if p.b != nil && p.b.Len() > 0 { return p.b.Read(d) } if p.err != nil { diff --git a/http2/pipe_test.go b/http2/pipe_test.go index 82bf9a34..1bf351ff 100644 --- a/http2/pipe_test.go +++ b/http2/pipe_test.go @@ -92,9 +92,12 @@ func TestPipeCloseWithError(t *testing.T) { if err != a { t.Logf("read error = %v, %v", err, a) } - // Write should fail. + // Read and Write should fail. if n, err := p.Write([]byte("abc")); err != errClosedPipeWrite || n != 0 { - t.Errorf("Write(abc) after close\ngot =%v, %v\nwant 0, %v", n, err, errClosedPipeWrite) + t.Errorf("Write(abc) after close\ngot %v, %v\nwant 0, %v", n, err, errClosedPipeWrite) + } + if n, err := p.Read(make([]byte, 1)); err == nil || n != 0 { + t.Errorf("Read() after close\ngot %v, nil\nwant 0, %v", n, errClosedPipeWrite) } } @@ -115,9 +118,13 @@ func TestPipeBreakWithError(t *testing.T) { } // Write should succeed silently. if n, err := p.Write([]byte("abc")); err != nil || n != 3 { - t.Errorf("Write(abc) after break\ngot =%v, %v\nwant 0, nil", n, err) + t.Errorf("Write(abc) after break\ngot %v, %v\nwant 0, nil", n, err) } if p.b != nil { t.Errorf("buffer should be nil after Write") } + // Read should fail. + if n, err := p.Read(make([]byte, 1)); err == nil || n != 0 { + t.Errorf("Read() after close\ngot %v, nil\nwant 0, not nil", n) + } }