From 62685c2d7ca23c807425dca88b11a3e2323dab41 Mon Sep 17 00:00:00 2001
From: Brad Fitzpatrick <bradfitz@golang.org>
Date: Sat, 13 Feb 2016 01:55:36 +0000
Subject: [PATCH] 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
---
 http2/transport.go      |  7 ++++++-
 http2/transport_test.go | 17 +++++++++++++++++
 2 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/http2/transport.go b/http2/transport.go
index 0aaa067f..c3a1bdb8 100644
--- a/http2/transport.go
+++ b/http2/transport.go
@@ -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
 		}
 	}
diff --git a/http2/transport_test.go b/http2/transport_test.go
index e9b89f75..39428735 100644
--- a/http2/transport_test.go
+++ b/http2/transport_test.go
@@ -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)
+	}
+}