net/http/httputil: Proxy terminates HTTP/2 stream before reading response body. #16788
Comments
|
(I did run into caddyserver/caddy#782 + #15425 - but not yet convinced these are the same issue) |
|
Updated issue with additional tests. |
|
Do you have a stand-alone repro which doesn't import external packages, and doesn't wrap the ResponseWriter with your own version? Also, which side of Go's HTTP/2 support are you reporting problems against? Go's HTTP/2 server or Go's HTTP/2 client? Draw me a little picture of the three entities involved and which protocols are on the two links. |
bradfitz
added
the
WaitingForInfo
|
Issue is against Go's HTTP/2 client - e.g. the upstream connection from the reverse proxy to the target. (Terrible) ASCII diagram: The issue (termination of the HTTP/2 stream before the response can be written) occurs at this point:
Caddy nginx All of those endpoints are live. Hammer as much as you want. The backend application just echoes the request; the same HTTP 502 and premature stream termination occurs when hitting CloudFlare's API via the same proxy (you would require API keys to test that). |
|
Oh, this might be a dup of #16450 Can you remove your ReadTimeout and WriteTimeout and see if the problem goes away? Or make them larger and see if it takes correspondingly long for the problem to repro? |
|
Oh, but you said it's a client problem, not a server problem. Still, if you're connecting to the server with HTTP/2 also and the conn dies, that would cancel the context to the proxy and abort the request there, which might be what you're seeing. Maybe. Worth trying anyway. |
|
Yep, client problem! Still, removed the Write/Read timeouts from the server-side of the proxy: issue persists, noting that the server-side (listening on localhost) of the proxy is just plaintext HTTP/1.1. Note: to make this somewhat worse, it appears to be broken ~95% of the time, with ~5% of responses being written. |
|
Can you paste the output of the proxy running with env |
It looks like the |
|
Whoa, this is messed up: Two END_STREAM bits for the same stream ID (13). No wonder the server on the other side is telling us RST_STREAM. I'm surprised it's not just hanging up on us for a protocol error violation. (To be clear, because your comment looks backwards, @elithrar: the Go HTTP/2 client appears to be writing bogus frames, and then the server is writing RST_STREAM stream=13 len=4 ErrCode=STREAM_CLOSED, which the proxy's HTTP client then reads.) |
bradfitz
added
NeedsInvestigation
|
@elithrar, this is untested, but can you try out this patch: diff --git a/src/net/http/h2_bundle.go b/src/net/http/h2_bundle.go
index ffe15f0..00718f6 100644
--- a/src/net/http/h2_bundle.go
+++ b/src/net/http/h2_bundle.go
@@ -5714,8 +5714,12 @@ func (cs *http2clientStream) writeRequestBody(body io.Reader, bodyCloser io.Clos
}
}
+ if sentEnd {
+ return nil
+ }
+
var trls []byte
- if !sentEnd && hasTrailers {
+ if hasTrailers {
cc.mu.Lock()
defer cc.mu.Unlock()
trls = cc.encodeTrailers(req) |
|
@bradfitz Works for me. Ran a few thousand curls through it with differing data (to detect any transient issues) - no longer seeing the client write bad frames: (I'm also curious as to why others haven't hit this yet, and it doesn't seem unique to my env) |
|
@elithrar, I am also curious about that. I will investigate more. I also don't know how this regressed (did it?) or how our end-to-end tests didn't catch it. Very confused. |
|
Simple package main
import (
"fmt"
"io/ioutil"
"log"
"net/http"
"net/url"
)
func main() {
resp, err := http.PostForm("https://post.workwithgo.com/post/data",
url.Values{"key": {"Value"}, "id": {"123"}})
if err != nil {
log.Fatal(err)
}
defer resp.Body.Close()
fmt.Println(resp.StatusCode)
body, err := ioutil.ReadAll(resp.Body)
if err != nil {
log.Fatal(err)
}
fmt.Println(string(body))
} |
|
The empty write isn't the problem. That second DATA frame is conveying useful information: END_STREAM is the "EOF" bit. The problem above is the duplicate END_STREAM. |
|
CL https://golang.org/cl/27406 mentions this issue. |
|
Thanks for the patch + very fast CL @bradfitz—hugely appreciated. |
… bodies The mod_h2 workaround CL (git rev 28d1bd4, https://golang.org/cl/25362) introduced a regression where the Transport could write two DATA frames, both with END_STREAM, if the Request.Body returned (non-0, io.EOF). strings.Reader, bytes.Reader are the most common Reader types used with HTTP requests and they only return (0, io.EOF) so we got generally lucky and things seemed to work, but other Readers do return (non-0, io.EOF), like the HTTP transport/server Readers. This is why we broke the HTTP proxy code, when proxying to HTTP/2. Updates golang/go#16788 (fixes after it's bundled into std) Change-Id: I42684017039eacfc27ee53e9c11431f713fdaca4 Reviewed-on: https://go-review.googlesource.com/27406 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Chris Broadfoot <cbro@golang.org>
|
CL https://golang.org/cl/27451 mentions this issue. |
…ouble STREAM_ENDED error Updates bundled http2 to x/net/http2 git rev 7394c11 for: http2: fix protocol violation regression when writing certain request bodies https://golang.org/cl/27406 Fixes #16788 Change-Id: I0efcd36e2b4b34a1df79f763d35bf7a3a1858506 Reviewed-on: https://go-review.googlesource.com/27451 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Andrew Gerrand <adg@golang.org> Reviewed-on: https://go-review.googlesource.com/28636 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
|
CL https://golang.org/cl/29080 mentions this issue. |
…writing certain request bodies The mod_h2 workaround CL (git rev 28d1bd4, https://golang.org/cl/25362) introduced a regression where the Transport could write two DATA frames, both with END_STREAM, if the Request.Body returned (non-0, io.EOF). strings.Reader, bytes.Reader are the most common Reader types used with HTTP requests and they only return (0, io.EOF) so we got generally lucky and things seemed to work, but other Readers do return (non-0, io.EOF), like the HTTP transport/server Readers. This is why we broke the HTTP proxy code, when proxying to HTTP/2. Updates golang/go#16788 (fixes after it's bundled into std) Change-Id: I42684017039eacfc27ee53e9c11431f713fdaca4 Reviewed-on: https://go-review.googlesource.com/27406 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Chris Broadfoot <cbro@golang.org> Reviewed-on: https://go-review.googlesource.com/29080
Please answer these questions before submitting your issue. Thanks!
1. What version of Go are you using (
go version)?go version go1.7 darwin/amd642. What operating system and processor architecture are you using (
go env)?3. What did you do?
-proxyflag.4. What did you expect to see?
5. What did you see instead?
binaryname -proxy=https://post.workwithgo.com/post/data(Caddy, HTTP/2, proxying the same origin application) and running the same curl does not cause the same error.Note: Disabling HTTP/2 via a custom transport on the upstream leg avoids the issue (as you would expect, given the error). I have yet to test on another HTTP/2 origin (I'll spin up a raw Caddy instance shortly).
The text was updated successfully, but these errors were encountered: