x/net/http2: PROTOCOL_ERROR after GOAWAY #32112
Comments
|
Per §8.1.2.2:
I don't see anything in the I don't know whether there is a documented, supported way to provoke a Go HTTP/2 server to gracefully close the connection after serving a request, but |
bcmills
added
the
NeedsInvestigation
|
@bcmills, the http2 server already maps a Handler's The // "Connection" headers aren't allowed in HTTP/2 (RFC 7540, 8.1.2.2),
// but respect "Connection" == "close" to mean sending a GOAWAY and tearing
// down the TCP connection when idle, like we do for HTTP/1.
// TODO: remove more Connection-specific header fields here, in addition
// to "Connection".
if _, ok := rws.snapHeader["Connection"]; ok {
v := rws.snapHeader.Get("Connection")
delete(rws.snapHeader, "Connection")
if v == "close" {
rws.conn.startGracefulShutdown()
}
} |
|
This looks like the server & client were both speaking at the ~same time and had messages that crossed paths in flight: The server was sending: GOAWAY The client was sending: HEADERS+DATA (a POST request with a body) The server got the HEADERS and ignored it, since it was already func (sc *serverConn) processHeaders(f *MetaHeadersFrame) error {
sc.serveG.check()
id := f.StreamID
if sc.inGoAway {
// Ignore.
return nil
}But it seems like that safety measure was only added for GET requests, because when we get the POST requests body in te DATA frame: func (sc *serverConn) processData(f *DataFrame) error {
sc.serveG.check()
if sc.inGoAway && sc.goAwayCode != ErrCodeNo {
return nil
}
data := f.Data()
// "If a DATA frame is received whose stream is not in "open"
// or "half closed (local)" state, the recipient MUST respond
// with a stream error (Section 5.4.2) of type STREAM_CLOSED."
id := f.Header().StreamID
state, st := sc.state(id)
if id == 0 || state == stateIdle {
// Section 5.1: "Receiving any frame other than HEADERS
// or PRIORITY on a stream in this state MUST be
// treated as a connection error (Section 5.4.1) of
// type PROTOCOL_ERROR."
return ConnectionError(ErrCodeProtocol)
}I don't remember why the |
|
Looks like it was added in golang/net@569280f |
|
Not sure if it's related, but in the server handler, I also occasionally saw errors like |
|
Change https://golang.org/cl/188360 mentions this issue: |
|
@bradfitz @tombergan @bcmills this tiny CL has been open for almost a month now. It's mostly documentation updates with a tiny bug fix. Would one of you mind reviewing? |
|
I've been on parental leave for 3 months. I'm just back at work and catching up. |
|
@bradfitz got time to give a final stamp? Andrew already gave a +1. It's a super tiny change that I'd really love to close out. Should only take a moment of your time. |
|
@mightyguava, it needs a test. |
|
Change https://golang.org/cl/237957 mentions this issue: |
|
Change https://golang.org/cl/296789 mentions this issue: |
Updates src/ and src/cmd/* dependencies, using
go mod vendor
as well as
updatestd -branch=master -goroot=$GOROOT
This change was ran in anticipation of bringing in x/net/http2 CL 237957.
For #32112.
For #36905.
Change-Id: If8cefc348463b6d82d85020b57db411213720ef8
Reviewed-on: https://go-review.googlesource.com/c/go/+/296789
Trust: Emmanuel Odeke <emmanuel@orijtech.com>
Trust: Dmitri Shuralyov <dmitshur@golang.org>
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Emmanuel Odeke <emmanuel@orijtech.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Alexander Rakoczy <alex@golang.org>
…tdown If the server sends a GOAWAY at the same time that a client sends HEADERS+DATA, the server will discard the HEADERS, but error on the DATA frame. Luckily, the server doesn't turn this into a connection error because it's already in a GOAWAY state. It just logs the PROTOCOL_ERROR, but that produces a confusing log message. This change effectively suppresses the log message by discarding the DATA frame rather than erroring on it. Also, we now discard any frames for streams with identifiers higher than the identified last stream. This is done as per section 6.8 of the HTTP2 spec. I also updated some stale documentation while I was trying to understand the logic. This is CL 188360 with a test Fixes golang/go#32112 Co-authored-by: Yunchi Luo <mightyguava@gmail.com> Co-authored-by: Michael Fraenkel <michael.fraenkel@gmail.com> Change-Id: I612c2bd82e37551e813af0961b16a98d14e77c38 Reviewed-on: https://go-review.googlesource.com/c/net/+/237957 Run-TryBot: Agniva De Sarker <agniva.quicksilver@gmail.com> Run-TryBot: Emmanuel Odeke <emmanuel@orijtech.com> Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com> TryBot-Result: Go Bot <gobot@golang.org> Trust: Damien Neil <dneil@google.com>
What version of Go are you using (
go version)?Does this issue reproduce with the latest release?
Yes
What operating system and processor architecture are you using (
go env)?This reproduces on Linux as well. I've been grabbing debugging data from my mac though.
go envOutputWhat did you do?
We are running http2 servers behind a TCP ELB loadbalancer (we need to terminate SSL at the server). This is causing unbalanced traffic since clients try to multiplex a single http2 connection. We had the idea of randomly sending GOAWAY to get clients to close their connections.
I then ran fake traffic using Vegeta to my server. So Go client/server.
What did you expect to see?
I expect that when a GOAWAY frame is sent, the server will complete processing of any open requests, and the client will open a new connection for new requests. This should all be transparent to user code. (I expected maybe I'd see
http.Clientreturn some errors about GOAWAY, but haven't seen that)What did you see instead?
The connection close chance was set at 0.5. At low qps, <10, everything is fine, no errors. Ramping up qps to 50, I start seeing
PROTOCOL_ERRORcoming from the server. This is reproducible locally with no load balancer in between client/server.Here's server and client logs for such occurrence:
Server logs
Client logs:
The text was updated successfully, but these errors were encountered: