From 0704177f3be733130adf7deeb70a4c0e4799dba3 Mon Sep 17 00:00:00 2001
From: Michael Fraenkel <michael.fraenkel@gmail.com>
Date: Sun, 28 Jun 2020 13:05:20 -0600
Subject: [PATCH] [release-branch.go1.15-bundle] http2: close Transport
 connection on write errors

When a new connection is created, and a write error occurs during the
initial exchange, the connection must be closed. There is no guarantee
that the caller will close the connection.

When a connection with an existing write error is used or being used, it
will stay in use until its read loop completes. Requests will continue
to use this connection and fail when writing its header. These
connections should be closed to force the cleanup in its readLoop.

Updates golang/go#39337.
For golang/go#42113.

Change-Id: I45e1293309e40629531f4cbb69387864f4f71bc2
Reviewed-on: https://go-review.googlesource.com/c/net/+/240337
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Brad Fitzpatrick <bradfitz@golang.org>
Trust: Damien Neil <dneil@google.com>
(cherry picked from commit f5854403a9740e74b2e9e725e6cd7c8a57711905)
Reviewed-on: https://go-review.googlesource.com/c/net/+/266158
Trust: Dmitri Shuralyov <dmitshur@golang.org>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com>
---
 http2/transport.go      | 10 +++++++
 http2/transport_test.go | 62 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 72 insertions(+)

diff --git a/http2/transport.go b/http2/transport.go
index 54acc1e36..7a07ad591 100644
--- a/http2/transport.go
+++ b/http2/transport.go
@@ -668,6 +668,7 @@ func (t *Transport) newClientConn(c net.Conn, singleUse bool) (*ClientConn, erro
 	cc.inflow.add(transportDefaultConnFlow + initialWindowSize)
 	cc.bw.Flush()
 	if cc.werr != nil {
+		cc.Close()
 		return nil, cc.werr
 	}
 
@@ -1033,6 +1034,15 @@ func (cc *ClientConn) roundTrip(req *http.Request) (res *http.Response, gotErrAf
 	bodyWriter := cc.t.getBodyWriterState(cs, body)
 	cs.on100 = bodyWriter.on100
 
+	defer func() {
+		cc.wmu.Lock()
+		werr := cc.werr
+		cc.wmu.Unlock()
+		if werr != nil {
+			cc.Close()
+		}
+	}()
+
 	cc.wmu.Lock()
 	endStream := !hasBody && !hasTrailers
 	werr := cc.writeHeaders(cs.ID, endStream, int(cc.maxFrameSize), hdrs)
diff --git a/http2/transport_test.go b/http2/transport_test.go
index 1424f818b..599525418 100644
--- a/http2/transport_test.go
+++ b/http2/transport_test.go
@@ -4568,3 +4568,65 @@ func TestClientConnTooIdle(t *testing.T) {
 		}
 	}
 }
+
+type fakeConnErr struct {
+	net.Conn
+	writeErr error
+	closed   bool
+}
+
+func (fce *fakeConnErr) Write(b []byte) (n int, err error) {
+	return 0, fce.writeErr
+}
+
+func (fce *fakeConnErr) Close() error {
+	fce.closed = true
+	return nil
+}
+
+// issue 39337: close the connection on a failed write
+func TestTransportNewClientConnCloseOnWriteError(t *testing.T) {
+	tr := &Transport{}
+	writeErr := errors.New("write error")
+	fakeConn := &fakeConnErr{writeErr: writeErr}
+	_, err := tr.NewClientConn(fakeConn)
+	if err != writeErr {
+		t.Fatalf("expected %v, got %v", writeErr, err)
+	}
+	if !fakeConn.closed {
+		t.Error("expected closed conn")
+	}
+}
+
+func TestTransportRoundtripCloseOnWriteError(t *testing.T) {
+	req, err := http.NewRequest("GET", "https://dummy.tld/", nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	st := newServerTester(t, func(w http.ResponseWriter, r *http.Request) {}, optOnlyServer)
+	defer st.Close()
+
+	tr := &Transport{TLSClientConfig: tlsConfigInsecure}
+	defer tr.CloseIdleConnections()
+	cc, err := tr.dialClientConn(st.ts.Listener.Addr().String(), false)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	writeErr := errors.New("write error")
+	cc.wmu.Lock()
+	cc.werr = writeErr
+	cc.wmu.Unlock()
+
+	_, err = cc.RoundTrip(req)
+	if err != writeErr {
+		t.Fatalf("expected %v, got %v", writeErr, err)
+	}
+
+	cc.mu.Lock()
+	closed := cc.closed
+	cc.mu.Unlock()
+	if !closed {
+		t.Fatal("expected closed")
+	}
+}