net/http: update bundled http2, add tests reading response Body after Close

Updates to golang.org/x/net/http2 git rev 28273ec9 for
https://golang.org/cl/17937

Fixes #13648

Change-Id: I27c77524b2e4a172c5f8be08f6fbb0f2e2e4b200
Reviewed-on: https://go-review.googlesource.com/17938
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>

diff --git a/src/net/http/clientserver_test.go b/src/net/http/clientserver_test.go
index bb94e5ffea0f7593dea19bbea249b81bac69b64a..ac25a04c0da270eff8224299d7a3f52567b6976c 100644 (file)
--- a/src/net/http/clientserver_test.go
+++ b/src/net/http/clientserver_test.go
@@ -597,3 +597,25 @@ func testTrailersServerToClient(t *testing.T, h2, flush bool) {
                t.Errorf("Trailer after body read = %v; want %v", got, want)
        }
 }
+
+// Don't allow a Body.Read after Body.Close. Issue 13648.
+func TestResponseBodyReadAfterClose_h1(t *testing.T) { testResponseBodyReadAfterClose(t, h1Mode) }
+func TestResponseBodyReadAfterClose_h2(t *testing.T) { testResponseBodyReadAfterClose(t, h2Mode) }
+
+func testResponseBodyReadAfterClose(t *testing.T, h2 bool) {
+       defer afterTest(t)
+       const body = "Some body"
+       cst := newClientServerTest(t, h2, HandlerFunc(func(w ResponseWriter, r *Request) {
+               io.WriteString(w, body)
+       }))
+       defer cst.close()
+       res, err := cst.c.Get(cst.ts.URL)
+       if err != nil {
+               t.Fatal(err)
+       }
+       res.Body.Close()
+       data, err := ioutil.ReadAll(res.Body)
+       if len(data) != 0 || err == nil {
+               t.Fatalf("ReadAll returned %q, %v; want error", data, err)
+       }
+}

diff --git a/src/net/http/h2_bundle.go b/src/net/http/h2_bundle.go
index 5e4b9c0141e1fb500bb5d9fc2ccd92a81f948032..020307374b188ad3367a4a94ece155b8edf28df5 100644 (file)
--- a/src/net/http/h2_bundle.go
+++ b/src/net/http/h2_bundle.go
@@ -1940,12 +1940,13 @@ func http2bodyAllowedForStatus(status int) bool {
 // io.Pipe except there are no PipeReader/PipeWriter halves, and the
 // underlying buffer is an interface. (io.Pipe is always unbuffered)
 type http2pipe struct {
-       mu     sync.Mutex
-       c      sync.Cond // c.L must point to
-       b      http2pipeBuffer
-       err    error         // read error once empty. non-nil means closed.
-       donec  chan struct{} // closed on error
-       readFn func()        // optional code to run in Read before error
+       mu       sync.Mutex
+       c        sync.Cond // c.L lazily initialized to &p.mu
+       b        http2pipeBuffer
+       err      error         // read error once empty. non-nil means closed.
+       breakErr error         // immediate read error (caller doesn't see rest of b)
+       donec    chan struct{} // closed on error
+       readFn   func()        // optional code to run in Read before error
 }
 
 type http2pipeBuffer interface {
@@ -1963,6 +1964,9 @@ func (p *http2pipe) Read(d []byte) (n int, err error) {
                p.c.L = &p.mu
        }
        for {
+               if p.breakErr != nil {
+                       return 0, p.breakErr
+               }
                if p.b.Len() > 0 {
                        return p.b.Read(d)
                }
@@ -1999,13 +2003,20 @@ func (p *http2pipe) Write(d []byte) (n int, err error) {
 // read.
 //
 // The error must be non-nil.
-func (p *http2pipe) CloseWithError(err error) { p.closeWithErrorAndCode(err, nil) }
+func (p *http2pipe) CloseWithError(err error) { p.closeWithError(&p.err, err, nil) }
+
+// BreakWithError causes the next Read (waking up a current blocked
+// Read if needed) to return the provided err immediately, without
+// waiting for unread data.
+func (p *http2pipe) BreakWithError(err error) { p.closeWithError(&p.breakErr, err, nil) }
 
 // closeWithErrorAndCode is like CloseWithError but also sets some code to run
 // in the caller's goroutine before returning the error.
-func (p *http2pipe) closeWithErrorAndCode(err error, fn func()) {
+func (p *http2pipe) closeWithErrorAndCode(err error, fn func()) { p.closeWithError(&p.err, err, fn) }
+
+func (p *http2pipe) closeWithError(dst *error, err error, fn func()) {
        if err == nil {
-               panic("CloseWithError err must be non-nil")
+               panic("err must be non-nil")
        }
        p.mu.Lock()
        defer p.mu.Unlock()
@@ -2013,22 +2024,35 @@ func (p *http2pipe) closeWithErrorAndCode(err error, fn func()) {
                p.c.L = &p.mu
        }
        defer p.c.Signal()
-       if p.err != nil {
+       if *dst != nil {
 
                return
        }
        p.readFn = fn
-       p.err = err
-       if p.donec != nil {
+       *dst = err
+       p.closeDoneLocked()
+}
+
+// requires p.mu be held.
+func (p *http2pipe) closeDoneLocked() {
+       if p.donec == nil {
+               return
+       }
+
+       select {
+       case <-p.donec:
+       default:
                close(p.donec)
        }
 }
 
-// Err returns the error (if any) first set with CloseWithError.
-// This is the error which will be returned after the reader is exhausted.
+// Err returns the error (if any) first set by BreakWithError or CloseWithError.
 func (p *http2pipe) Err() error {
        p.mu.Lock()
        defer p.mu.Unlock()
+       if p.breakErr != nil {
+               return p.breakErr
+       }
        return p.err
 }
 
@@ -2039,9 +2063,9 @@ func (p *http2pipe) Done() <-chan struct{} {
        defer p.mu.Unlock()
        if p.donec == nil {
                p.donec = make(chan struct{})
-               if p.err != nil {
+               if p.err != nil || p.breakErr != nil {
 
-                       close(p.donec)
+                       p.closeDoneLocked()
                }
        }
        return p.donec
@@ -5024,11 +5048,15 @@ func (b http2transportResponseBody) Read(p []byte) (n int, err error) {
        return
 }
 
+var http2errClosedResponseBody = errors.New("http2: response body closed")
+
 func (b http2transportResponseBody) Close() error {
-       if b.cs.bufPipe.Err() != io.EOF {
+       cs := b.cs
+       if cs.bufPipe.Err() != io.EOF {
 
-               b.cs.cc.writeStreamReset(b.cs.ID, http2ErrCodeCancel, nil)
+               cs.cc.writeStreamReset(cs.ID, http2ErrCodeCancel, nil)
        }
+       cs.bufPipe.BreakWithError(http2errClosedResponseBody)
        return nil
 }