git » go-net » commit 8e573f4

http2: merge multiple GOAWAY frames' contents into error message

author Brad Fitzpatrick
2016-06-29 18:55:36 UTC
committer Brad Fitzpatrick
2016-06-29 22:54:44 UTC
parent ef2e00e88c5e0a3569f0bb9df697e9cbc6215fea

http2: merge multiple GOAWAY frames' contents into error message

The http2 spec permits multiple GOAWAY frames if the conditions
change.  Merge their error messages together, preferring the first
non-empty debug data, and first non-ErrCodeNo error code.

Updates golang/go#14627

Change-Id: I073b9234d71f128ed0713f09a24c728b56d7c1ae
Reviewed-on: https://go-review.googlesource.com/24600
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Andrew Gerrand <adg@golang.org>

http2/transport.go +10 -1
http2/transport_test.go +5 -2

diff --git a/http2/transport.go b/http2/transport.go
index 52bc9a3..ad65ab6 100644
--- a/http2/transport.go
+++ b/http2/transport.go
@@ -495,8 +495,17 @@ func (t *Transport) NewClientConn(c net.Conn) (*ClientConn, error) {
 func (cc *ClientConn) setGoAway(f *GoAwayFrame) {
 	cc.mu.Lock()
 	defer cc.mu.Unlock()
+
+	old := cc.goAway
 	cc.goAway = f
-	cc.goAwayDebug = string(f.DebugData())
+
+	// Merge the previous and current GoAway error frames.
+	if cc.goAwayDebug == "" {
+		cc.goAwayDebug = string(f.DebugData())
+	}
+	if old != nil && old.ErrCode != ErrCodeNo {
+		cc.goAway.ErrCode = old.ErrCode
+	}
 }
 
 func (cc *ClientConn) CanTakeNewRequest() bool {
diff --git a/http2/transport_test.go b/http2/transport_test.go
index e1274b0..39abde2 100644
--- a/http2/transport_test.go
+++ b/http2/transport_test.go
@@ -2044,7 +2044,7 @@ func testTransportUsesGoAwayDebugError(t *testing.T, failMidBody bool) {
 			res.Body.Close()
 		}
 		want := GoAwayError{
-			LastStreamID: 0,
+			LastStreamID: 5,
 			ErrCode:      goAwayErrCode,
 			DebugData:    goAwayDebugData,
 		}
@@ -2077,7 +2077,10 @@ func testTransportUsesGoAwayDebugError(t *testing.T, failMidBody bool) {
 					BlockFragment: buf.Bytes(),
 				})
 			}
-			ct.fr.WriteGoAway(0, goAwayErrCode, []byte(goAwayDebugData))
+			// Write two GOAWAY frames, to test that the Transport takes
+			// the interesting parts of both.
+			ct.fr.WriteGoAway(5, ErrCodeNo, []byte(goAwayDebugData))
+			ct.fr.WriteGoAway(5, goAwayErrCode, nil)
 			ct.sc.Close()
 			<-clientDone
 			return nil