New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
net/http: "missing status pseudo header" when reusing HTTP/2 connection #22880
Comments
No, they don't. Which URL are you actually requesting? I'm wondering which server you're hitting. Can you attach a trace with GODEBUG=http2debug=2 set? /cc @tombergan |
No unfortunately not. The crash only occurred once and never again since.
I initially preferred to not disclose this information, maybe it might still help though: |
Good news, it crashed again 🎉
EDIT: |
That's a bogus response from the Admittedly we should handle it better (not crash, whoops), but we also need to contact the author of that HTTP/2 server and get it fixed. What do Chrome & Firefox do when /cc @tombergan @mcmanus @mnot |
Did a quick tcpdump; that server doesn't appear to be negotiating H2, either with ALPN or Alt-Svc. |
@mnot both h2i and Chrome do negotiate HTTP/2 on my side: $ h2i api.cryptowat.ch
Connecting to api.cryptowat.ch:443 ...
Connected to 96.126.127.137:443
Negotiated protocol "h2"
Sending: []
[FrameHeader SETTINGS len=24]
[MAX_FRAME_SIZE = 1048576]
[MAX_CONCURRENT_STREAMS = 250]
[MAX_HEADER_LIST_SIZE = 1048896]
[INITIAL_WINDOW_SIZE = 1048576]
[FrameHeader WINDOW_UPDATE len=4]
Window-Increment = 983041
[FrameHeader SETTINGS flags=ACK len=0] |
It's possible it's only doing NPN and not ALPN. I can check later. |
a response missing :status is a PROTOCOL_ERROR in firefox |
Change https://golang.org/cl/80056 mentions this issue: |
api.cryptowat.ch seems to do both NPN and ALPN. Still not sure which HTTP/2 server it is. https://cryptowat.ch/about says it was created by @artursapek. Artur, can you comment on which HTTP/2 server you're using for Cryptowatch? The HTTP/2 implementation seems buggy. |
hi @bradfitz. it's a the binary on production was built with:
|
Well that's embarrassing. @artursapek, thanks. I'll try to figure out how and where we're dropping |
@bradfitz no worries, lmk if I can provide more specific details. and thanks for all your work on Go! we have a ton of gophers working hard for us over here. |
@artursapek, I actually do see one way for this to happen, but it's weird: your http.Handler code would have to call Is it possible you ever call |
I could imagine code like: var status int
if someCondition {
status = http.BadRequest
} else if otherCondition {
status = 500
}
rw.WriteHeader(status) ... that would forget to set status. |
@bradfitz you're right, there was a case where we were failing to overwrite a default zero-value and passing that into |
Thanks for confirming. I'll fix the http2 server and make sure we're consistent in behavior from Go's API point of view between http1 and http2. |
In golang/go#22880, our http2 server was sending a HEADERS response without a :status header. Our client code correctly returned an error from RoundTrip, but we forgot to clean up properly, and then subsequently crashed on a DATA frame. This fixes the Transport crash. A fix for the server bug will come separately. Change-Id: Iea3bcf4a8c95963c8b5e2b6dd722177607bd1bc1 Reviewed-on: https://go-review.googlesource.com/80056 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Tom Bergan <tombergan@google.com>
Change https://golang.org/cl/80076 mentions this issue: |
Tests are in net/http. (upcoming CL) Updates golang/go#22880 Change-Id: Ie94693ad4e14f0c07926a0b6c7827caace94a0aa Reviewed-on: https://go-review.googlesource.com/80076 Reviewed-by: Tom Bergan <tombergan@google.com> Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>
Also https://go-review.googlesource.com/c/go/+/80077/3 (in which I typoed this issue number). |
Change https://golang.org/cl/80078 mentions this issue: |
In golang/go#22880, our http2 server was sending a HEADERS response without a :status header. Our client code correctly returned an error from RoundTrip, but we forgot to clean up properly, and then subsequently crashed on a DATA frame. This fixes the Transport crash. A fix for the server bug will come separately. Change-Id: Iea3bcf4a8c95963c8b5e2b6dd722177607bd1bc1 Reviewed-on: https://go-review.googlesource.com/80056 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Tom Bergan <tombergan@google.com>
Tests are in net/http. (upcoming CL) Updates golang/go#22880 Change-Id: Ie94693ad4e14f0c07926a0b6c7827caace94a0aa Reviewed-on: https://go-review.googlesource.com/80076 Reviewed-by: Tom Bergan <tombergan@google.com> Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>
Update http2 to x/net git rev db473f6b23. (And un-skip TestWriteHeader0_h2 added in CL 80077, now fixed.) Includes: http2: remove afterReqBodyWriteError wrapper https://golang.org/cl/75252 http2: fix transport data race on reused *http.Request objects https://golang.org/cl/75530 http2: require either ECDSA or RSA ciphersuite https://golang.org/cl/30721 http2: don't log about timeouts reading client preface on new connections https://golang.org/cl/79498 http2: don't crash in Transport on server's DATA following bogus HEADERS https://golang.org/cl/80056 http2: panic on invalid WriteHeader status code https://golang.org/cl/80076 http2: fix race on ClientConn.maxFrameSize https://golang.org/cl/79238 http2: don't autodetect Content-Type when the response has an empty body https://golang.org/cl/80135 Fixes golang/go#18776 Updates golang/go#20784 Fixes golang/go#21316 Fixes golang/go#22721 Fixes golang/go#22880 Change-Id: Ie86e24e0ee2582a5a82afe5de3c7c801528be069 Reviewed-on: https://go-review.googlesource.com/80078 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Tom Bergan <tombergan@google.com>
What version of Go are you using (
go version
)?1.9.2
Does this issue reproduce with the latest release?
Yes
What operating system and processor architecture are you using (
go env
)?What did you do?
Reusing an HTTP client resulted in a panic, example code: https://play.golang.org/p/DGUqwSMygZ
Unfortunately, I was unable to reproduce this issue again, still find it worth reporting it here.
Panic trace:
The first request succeeded, (
Got response!
), the second one failed (failed to GET: missing status pseudo header
) and most likely the third one caused the panic.Is this a bug? Do Go HTTP clients need to be reinstantiated upon request failure?
The text was updated successfully, but these errors were encountered: