Skip to content

Commit

Permalink
http2: fix nil dereference after Read completes with an error
Browse files Browse the repository at this point in the history
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 <tombergan@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
  • Loading branch information
tombergan authored and bradfitz committed May 26, 2017
1 parent 06b2bf2 commit 3470a06
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 4 deletions.
2 changes: 1 addition & 1 deletion http2/pipe.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
13 changes: 10 additions & 3 deletions http2/pipe_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand All @@ -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)
}
}

0 comments on commit 3470a06

Please sign in to comment.