http2: fix Transport.RoundTrip hang on stream error before headers
If the Transport got a stream error on the response headers, it was
never unblocking the client. Previously, Response.Body reads would be
aborted with the stream error, but RoundTrip itself would never
unblock.
The Transport now also sends a RST_STREAM to the server when we
encounter a stream error.
Also, add a "Cause" field to StreamError with additional detail. The
old code was just returning the detail, without the stream error
header.
Fixes golang/go#16572
Change-Id: Ibecedb5779f17bf98c32787b68eb8a9b850833b3
Reviewed-on: https://go-review.googlesource.com/25402
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Gerrand <adg@golang.org>
diff --git a/http2/server.go b/http2/server.go
index 679bda4..8206fa7 100644
--- a/http2/server.go
+++ b/http2/server.go
@@ -922,7 +922,7 @@
// state here anyway, after telling the peer
// we're hanging up on them.
st.state = stateHalfClosedLocal // won't last long, but necessary for closeStream via resetStream
- errCancel := StreamError{st.id, ErrCodeCancel}
+ errCancel := streamError(st.id, ErrCodeCancel)
sc.resetStream(errCancel)
case stateHalfClosedRemote:
sc.closeStream(st, errHandlerComplete)
@@ -1133,7 +1133,7 @@
return nil
}
if !st.flow.add(int32(f.Increment)) {
- return StreamError{f.StreamID, ErrCodeFlowControl}
+ return streamError(f.StreamID, ErrCodeFlowControl)
}
default: // connection-level flow control
if !sc.flow.add(int32(f.Increment)) {
@@ -1159,7 +1159,7 @@
if st != nil {
st.gotReset = true
st.cancelCtx()
- sc.closeStream(st, StreamError{f.StreamID, f.ErrCode})
+ sc.closeStream(st, streamError(f.StreamID, f.ErrCode))
}
return nil
}
@@ -1299,7 +1299,7 @@
// and return any flow control bytes since we're not going
// to consume them.
if sc.inflow.available() < int32(f.Length) {
- return StreamError{id, ErrCodeFlowControl}
+ return streamError(id, ErrCodeFlowControl)
}
// Deduct the flow control from inflow, since we're
// going to immediately add it back in
@@ -1308,7 +1308,7 @@
sc.inflow.take(int32(f.Length))
sc.sendWindowUpdate(nil, int(f.Length)) // conn-level
- return StreamError{id, ErrCodeStreamClosed}
+ return streamError(id, ErrCodeStreamClosed)
}
if st.body == nil {
panic("internal error: should have a body in this state")
@@ -1317,19 +1317,19 @@
// Sender sending more than they'd declared?
if st.declBodyBytes != -1 && st.bodyBytes+int64(len(data)) > st.declBodyBytes {
st.body.CloseWithError(fmt.Errorf("sender tried to send more than declared Content-Length of %d bytes", st.declBodyBytes))
- return StreamError{id, ErrCodeStreamClosed}
+ return streamError(id, ErrCodeStreamClosed)
}
if f.Length > 0 {
// Check whether the client has flow control quota.
if st.inflow.available() < int32(f.Length) {
- return StreamError{id, ErrCodeFlowControl}
+ return streamError(id, ErrCodeFlowControl)
}
st.inflow.take(int32(f.Length))
if len(data) > 0 {
wrote, err := st.body.Write(data)
if err != nil {
- return StreamError{id, ErrCodeStreamClosed}
+ return streamError(id, ErrCodeStreamClosed)
}
if wrote != len(data) {
panic("internal error: bad Writer")
@@ -1446,14 +1446,14 @@
// REFUSED_STREAM."
if sc.unackedSettings == 0 {
// They should know better.
- return StreamError{st.id, ErrCodeProtocol}
+ return streamError(st.id, ErrCodeProtocol)
}
// Assume it's a network race, where they just haven't
// received our last SETTINGS update. But actually
// this can't happen yet, because we don't yet provide
// a way for users to adjust server parameters at
// runtime.
- return StreamError{st.id, ErrCodeRefusedStream}
+ return streamError(st.id, ErrCodeRefusedStream)
}
rw, req, err := sc.newWriterAndRequest(st, f)
@@ -1487,11 +1487,11 @@
}
st.gotTrailerHeader = true
if !f.StreamEnded() {
- return StreamError{st.id, ErrCodeProtocol}
+ return streamError(st.id, ErrCodeProtocol)
}
if len(f.PseudoFields()) > 0 {
- return StreamError{st.id, ErrCodeProtocol}
+ return streamError(st.id, ErrCodeProtocol)
}
if st.trailer != nil {
for _, hf := range f.RegularFields() {
@@ -1500,7 +1500,7 @@
// TODO: send more details to the peer somehow. But http2 has
// no way to send debug data at a stream level. Discuss with
// HTTP folk.
- return StreamError{st.id, ErrCodeProtocol}
+ return streamError(st.id, ErrCodeProtocol)
}
st.trailer[key] = append(st.trailer[key], hf.Value)
}
@@ -1561,7 +1561,7 @@
isConnect := method == "CONNECT"
if isConnect {
if path != "" || scheme != "" || authority == "" {
- return nil, nil, StreamError{f.StreamID, ErrCodeProtocol}
+ return nil, nil, streamError(f.StreamID, ErrCodeProtocol)
}
} else if method == "" || path == "" ||
(scheme != "https" && scheme != "http") {
@@ -1575,13 +1575,13 @@
// "All HTTP/2 requests MUST include exactly one valid
// value for the :method, :scheme, and :path
// pseudo-header fields"
- return nil, nil, StreamError{f.StreamID, ErrCodeProtocol}
+ return nil, nil, streamError(f.StreamID, ErrCodeProtocol)
}
bodyOpen := !f.StreamEnded()
if method == "HEAD" && bodyOpen {
// HEAD requests can't have bodies
- return nil, nil, StreamError{f.StreamID, ErrCodeProtocol}
+ return nil, nil, streamError(f.StreamID, ErrCodeProtocol)
}
var tlsState *tls.ConnectionState // nil if not scheme https
@@ -1639,7 +1639,7 @@
var err error
url_, err = url.ParseRequestURI(path)
if err != nil {
- return nil, nil, StreamError{f.StreamID, ErrCodeProtocol}
+ return nil, nil, streamError(f.StreamID, ErrCodeProtocol)
}
requestURI = path
}
