]> Cypherpunks repositories - gostls13.git/commit
net/http: fix handling of HTTP/2 upgrade failures
authorRuss Cox <rsc@golang.org>
Tue, 25 Feb 2020 18:01:59 +0000 (13:01 -0500)
committerRuss Cox <rsc@golang.org>
Tue, 3 Mar 2020 16:04:34 +0000 (16:04 +0000)
commitea1437a8cdf6bb3c2d2447833a5d06dbd75f7ae4
treea94aa983f21cbf3bc0c07053b6a7f6b36f2f6569
parent96acb74655531794d6f563242736d25f4e716b11
net/http: fix handling of HTTP/2 upgrade failures

If an error occurs during the HTTP/2 upgrade phase, originally this
resulted in a pconn with pconn.alt set to an http2erringRoundTripper,
which always fails. This is not wanted - we want to retry in this case.

CL 202078 added a check for the http2erringRoundTripper to treat it
as a failed pconn, but the handling of the failure was wrong in the case
where the pconn is not in the idle list at all (common in HTTP/2).
This made the added test TestDontCacheBrokenHTTP2Conn flaky.

CL 218097 (unsubmitted) proposed to expand the handling of the
http2erringRoundTripper after the new check, to dispose of the pconn
more thoroughly. Bryan Mills pointed out in that review that we probably
shouldn't make the never-going-to-work pconn in the first place.

This CL changes the upgrade phase look for the http2erringRoundTripper
and return the underlying error instead of claiming to have a working
connection. Having done that, the CL undoes the change in CL 202078
and with it the need for CL 218097, but it keeps the new test added
by CL 202078.

On my laptop, before this commit, TestDontCacheBrokenHTTP2Conn
failed 66 times out of 20,000. With this commit, I see 0 out of 20,000.

Fixes #34978.
Fixes #35113.

Change-Id: Ibd908b63c2ae96e159e8e604213d8373afb350e3
Reviewed-on: https://go-review.googlesource.com/c/go/+/220905
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
src/net/http/omithttp2.go
src/net/http/transport.go