Commit cbbbe2bc authored by Brad Fitzpatrick's avatar Brad Fitzpatrick

http2: fix crash in Transport on double Read of invalid gzip Response.Body

This old code was buggy:

type gzipReader struct {
        body io.ReadCloser // underlying Response.Body
        zr   io.Reader     // lazily-initialized gzip reader
}

func (gz *gzipReader) Read(p []byte) (n int, err error) {
        if gz.zr == nil {
                gz.zr, err = gzip.NewReader(gz.body)
                if err != nil {
                        return 0, err
                }
        }
        return gz.zr.Read(p)
}

If a Read on a gzipped Response.Body (of type *http2.gzipReader)
resulted in gzip.NewReader returning an error, gzipReader assigned
a *gzip.Reader-typed nil value to the gz.zr interface value.

On a subsequent Read, gz.zr would not be equal to ==, because it was
actually equal to (type *gzip.Reader, nil), and then zr.Read would call
(*gzip.Reader).Read with a nil receiver and explode.

Debugged internally. (http://go/http2gzipbug)

Change-Id: Icba040ace8ffac3536e5e7ade6695c7660838ca1
Reviewed-on: https://go-review.googlesource.com/19483Reviewed-by: 's avatarIan Lance Taylor <iant@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
parent 8968c619
......@@ -1723,13 +1723,18 @@ func (rt erringRoundTripper) RoundTrip(*http.Request) (*http.Response, error) {
// call gzip.NewReader on the first call to Read
type gzipReader struct {
body io.ReadCloser // underlying Response.Body
zr io.Reader // lazily-initialized gzip reader
zr *gzip.Reader // lazily-initialized gzip reader
zerr error // sticky error
}
func (gz *gzipReader) Read(p []byte) (n int, err error) {
if gz.zerr != nil {
return 0, gz.zerr
}
if gz.zr == nil {
gz.zr, err = gzip.NewReader(gz.body)
if err != nil {
gz.zerr = err
return 0, err
}
}
......
......@@ -1660,3 +1660,20 @@ func TestTransportRejectsConnHeaders(t *testing.T) {
}
}
}
// Tests that gzipReader doesn't crash on a second Read call following
// the first Read call's gzip.NewReader returning an error.
func TestGzipReader_DoubleReadCrash(t *testing.T) {
gz := &gzipReader{
body: ioutil.NopCloser(strings.NewReader("0123456789")),
}
var buf [1]byte
n, err1 := gz.Read(buf[:])
if n != 0 || !strings.Contains(fmt.Sprint(err1), "invalid header") {
t.Fatalf("Read = %v, %v; want 0, invalid header", n, err1)
}
n, err2 := gz.Read(buf[:])
if n != 0 || err2 != err1 {
t.Fatalf("second Read = %v, %v; want 0, %v", n, err2, err1)
}
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment