Skip to content

Commit 3470a06

Browse files
tomberganbradfitz
authored andcommitted
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 <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Brad Fitzpatrick <[email protected]>
1 parent 06b2bf2 commit 3470a06

File tree

2 files changed

+11
-4
lines changed

2 files changed

+11
-4
lines changed

http2/pipe.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ func (p *pipe) Read(d []byte) (n int, err error) {
5050
if p.breakErr != nil {
5151
return 0, p.breakErr
5252
}
53-
if p.b.Len() > 0 {
53+
if p.b != nil && p.b.Len() > 0 {
5454
return p.b.Read(d)
5555
}
5656
if p.err != nil {

http2/pipe_test.go

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -92,9 +92,12 @@ func TestPipeCloseWithError(t *testing.T) {
9292
if err != a {
9393
t.Logf("read error = %v, %v", err, a)
9494
}
95-
// Write should fail.
95+
// Read and Write should fail.
9696
if n, err := p.Write([]byte("abc")); err != errClosedPipeWrite || n != 0 {
97-
t.Errorf("Write(abc) after close\ngot =%v, %v\nwant 0, %v", n, err, errClosedPipeWrite)
97+
t.Errorf("Write(abc) after close\ngot %v, %v\nwant 0, %v", n, err, errClosedPipeWrite)
98+
}
99+
if n, err := p.Read(make([]byte, 1)); err == nil || n != 0 {
100+
t.Errorf("Read() after close\ngot %v, nil\nwant 0, %v", n, errClosedPipeWrite)
98101
}
99102
}
100103

@@ -115,9 +118,13 @@ func TestPipeBreakWithError(t *testing.T) {
115118
}
116119
// Write should succeed silently.
117120
if n, err := p.Write([]byte("abc")); err != nil || n != 3 {
118-
t.Errorf("Write(abc) after break\ngot =%v, %v\nwant 0, nil", n, err)
121+
t.Errorf("Write(abc) after break\ngot %v, %v\nwant 0, nil", n, err)
119122
}
120123
if p.b != nil {
121124
t.Errorf("buffer should be nil after Write")
122125
}
126+
// Read should fail.
127+
if n, err := p.Read(make([]byte, 1)); err == nil || n != 0 {
128+
t.Errorf("Read() after close\ngot %v, nil\nwant 0, not nil", n)
129+
}
123130
}

0 commit comments

Comments
 (0)