aboutsummaryrefslogtreecommitdiff
path: root/src/net/http/transport_test.go
AgeCommit message (Collapse)Author
2021-08-04net/http: speed up and deflake TestCancelRequestWhenSharingConnectionDamien Neil
This test made many requests over the same connection for 10 seconds, trusting that this will exercise the request cancelation race from #41600. Change the test to exhibit the specific race in a targeted fashion with only two requests. Updates #41600. Updates #47016. Change-Id: If99c9b9331ff645f6bb67fe9fb79b8aab8784710 Reviewed-on: https://go-review.googlesource.com/c/go/+/339594 Trust: Damien Neil <dneil@google.com> Run-TryBot: Damien Neil <dneil@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Heschi Kreinick <heschi@google.com>
2021-06-10net/http: remove test-only private key from production binariesDamien Neil
The net/http/internal package contains a PEM-encoded private key used in tests. This key is initialized at init time, which prevents it from being stripped by the linker in non-test binaries. Move the certificate and key to a new net/http/internal/testcert package to ensure it is only included in binaries that reference it. Fixes #46677. Change-Id: Ie98bda529169314cc791063e7ce4d99ef99113c8 Reviewed-on: https://go-review.googlesource.com/c/go/+/326771 Trust: Damien Neil <dneil@google.com> Run-TryBot: Damien Neil <dneil@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
2021-05-14net/http: prevent infinite wait during TestMissingStatusNoPanicMichael Fraenkel
If the client request never makes it to the server, the outstanding accept is never broken. Change the test to always close the listening socket when the client request completes. Updates #45358 Change-Id: I744a91dfa11704e7e528163d7669c394e90456dc Reviewed-on: https://go-review.googlesource.com/c/go/+/319275 Trust: Heschi Kreinick <heschi@google.com> Reviewed-by: Bryan C. Mills <bcmills@google.com>
2021-04-15net/http: allow multiple dials in TestTransportMaxConnsPerHostMichael Fraenkel
If there is more than the expected single dial, the channel will block. Allow at least one connection per client, and do the expected cleanup. Updates #45570 Change-Id: Iaecd45298a7d7c591b7d7b1be13cea6e4a1e2e85 Reviewed-on: https://go-review.googlesource.com/c/go/+/310213 Reviewed-by: Bryan C. Mills <bcmills@google.com> Reviewed-by: Damien Neil <dneil@google.com> Trust: Damien Neil <dneil@google.com> Trust: Emmanuel Odeke <emmanuel@orijtech.com>
2021-03-16crypto/tls: add HandshakeContext method to ConnJohan Brandhorst
Adds the (*tls.Conn).HandshakeContext method. This allows us to pass the context provided down the call stack to eventually reach the tls.ClientHelloInfo and tls.CertificateRequestInfo structs. These contexts are exposed to the user as read-only via Context() methods. This allows users of (*tls.Config).GetCertificate and (*tls.Config).GetClientCertificate to use the context for request scoped parameters and cancellation. Replace uses of (*tls.Conn).Handshake with (*tls.Conn).HandshakeContext where appropriate, to propagate existing contexts. Fixes #32406 Change-Id: I259939c744bdc9b805bf51a845a8bc462c042483 Reviewed-on: https://go-review.googlesource.com/c/go/+/295370 Run-TryBot: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com> TryBot-Result: Go Bot <gobot@golang.org> Trust: Katie Hockman <katie@golang.org> Reviewed-by: Filippo Valsorda <filippo@golang.org>
2020-12-17crypto/tls: revert "add HandshakeContext method to Conn"Johan Brandhorst
This reverts CL 246338. Reason for revert: waiting for 1.17 release cycle Updates #32406 Change-Id: I074379039041e086c62271d689b4b7f442281663 Reviewed-on: https://go-review.googlesource.com/c/go/+/269697 Run-TryBot: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com> Run-TryBot: Katie Hockman <katie@golang.org> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Katie Hockman <katie@golang.org> Trust: Katie Hockman <katie@golang.org> Trust: Roland Shoemaker <roland@golang.org>
2020-12-09all: update to use os.ReadFile, os.WriteFile, os.CreateTemp, os.MkdirTempRuss Cox
As part of #42026, these helpers from io/ioutil were moved to os. (ioutil.TempFile and TempDir became os.CreateTemp and MkdirTemp.) Update the Go tree to use the preferred names. As usual, code compiled with the Go 1.4 bootstrap toolchain and code vendored from other sources is excluded. ReadDir changes are in a separate CL, because they are not a simple search and replace. For #42026. Change-Id: If318df0216d57e95ea0c4093b89f65e5b0ababb3 Reviewed-on: https://go-review.googlesource.com/c/go/+/266365 Trust: Russ Cox <rsc@golang.org> Run-TryBot: Russ Cox <rsc@golang.org> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
2020-12-01net/http: ignore connection closes once done with the connectionMichael Fraenkel
Once the connection is put back into the idle pool, the request should not take any action if the connection is closed. Fixes #41600 Change-Id: I5e4ddcdc03cd44f5197ecfbe324638604961de84 Reviewed-on: https://go-review.googlesource.com/c/go/+/257818 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Trust: Damien Neil <dneil@google.com>
2020-11-09crypto/tls: add HandshakeContext method to ConnJohan Brandhorst
Adds the (*tls.Conn).HandshakeContext method. This allows us to pass the context provided down the call stack to eventually reach the tls.ClientHelloInfo and tls.CertificateRequestInfo structs. These contexts are exposed to the user as read-only via Context() methods. This allows users of (*tls.Config).GetCertificate and (*tls.Config).GetClientCertificate to use the context for request scoped parameters and cancellation. Replace uses of (*tls.Conn).Handshake with (*tls.Conn).HandshakeContext where appropriate, to propagate existing contexts. Fixes #32406 Change-Id: I33c228904fe82dcf57683b63627497d3eb841ff2 Reviewed-on: https://go-review.googlesource.com/c/go/+/246338 Run-TryBot: Filippo Valsorda <filippo@golang.org> TryBot-Result: Go Bot <gobot@golang.org> Trust: Roland Shoemaker <roland@golang.org> Reviewed-by: Filippo Valsorda <filippo@golang.org>
2020-10-24net/http: fix typo in TestTransportReadToEndReusesConnKeiichi Hirobe
The test sets a Content-Type where it looks like it wants a Content-Length. The test passes because the Content-Length header is automatically added anyway, but fix the typo and set Content-Length as intended. Change-Id: Ic2af778f82c3e9d58e164892f6ac6ef5745f884f Reviewed-on: https://go-review.googlesource.com/c/go/+/246977 Reviewed-by: Damien Neil <dneil@google.com> Trust: Alberto Donizetti <alb.donizetti@gmail.com> Trust: Damien Neil <dneil@google.com> Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com> TryBot-Result: Go Bot <gobot@golang.org>
2020-10-20all: update references to symbols moved from io/ioutil to ioRuss Cox
The old ioutil references are still valid, but update our code to reflect best practices and get used to the new locations. Code compiled with the bootstrap toolchain (cmd/asm, cmd/dist, cmd/compile, debug/elf) must remain Go 1.4-compatible and is excluded. Also excluded vendored code. For #41190. Change-Id: I6d86f2bf7bc37a9d904b6cee3fe0c7af6d94d5b1 Reviewed-on: https://go-review.googlesource.com/c/go/+/263142 Trust: Russ Cox <rsc@golang.org> Run-TryBot: Russ Cox <rsc@golang.org> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2020-10-12net/http: deflake TestTransportPersistConnLeak on macOSRuss Cox
On a loaded system, sometimes connections don't work out. Ignore those in TestTransportPersistConnLeak to avoid flakes. For #33585. Change-Id: Ic07057532dc0ea5115d6ec49c3c29099a9382295 Reviewed-on: https://go-review.googlesource.com/c/go/+/261538 Run-TryBot: Russ Cox <rsc@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org> TryBot-Result: Go Bot <gobot@golang.org> Trust: Russ Cox <rsc@golang.org>
2020-10-06net/http: add Transport.GetProxyConnectHeaderBrad Fitzpatrick
Fixes golang/go#41048 Change-Id: I38e01605bffb6f85100c098051b0c416dd77f261 Reviewed-on: https://go-review.googlesource.com/c/go/+/259917 Trust: Brad Fitzpatrick <bradfitz@golang.org> Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Damien Neil <dneil@google.com>
2020-09-09net/http: fix data race due to writeLoop goroutine left runningSteven Hartland
Fix a data race for clients that mutate requests after receiving a response error which is caused by the writeLoop goroutine left running, this can be seen on cancelled requests. Fixes #37669 Change-Id: Ia4743c6b8abde3a7503de362cc6a3782e19e7f60 Reviewed-on: https://go-review.googlesource.com/c/go/+/251858 Reviewed-by: Bryan C. Mills <bcmills@google.com> Run-TryBot: Bryan C. Mills <bcmills@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-08-25Revert "net/http: fix data race due to writeLoop goroutine left running"Bryan C. Mills
This reverts CL 232799. Reason for revert: net/http test is failing on all longtest builders. Change-Id: I4694e34f35419bab2d0b45fa6d8c3ac2aa1f51a0 Reviewed-on: https://go-review.googlesource.com/c/go/+/250597 Run-TryBot: Bryan C. Mills <bcmills@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Katie Hockman <katie@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-08-25net/http: fix data race due to writeLoop goroutine left runningSteven Hartland
Fix a data race for clients that mutate requests after receiving a response error which is caused by the writeLoop goroutine left running, this can be seen on canceled requests. Fixes #37669 Change-Id: I0e0e4fd63266326b32587d8596456760bf848b13 Reviewed-on: https://go-review.googlesource.com/c/go/+/232799 Reviewed-by: Bryan C. Mills <bcmills@google.com> Run-TryBot: Bryan C. Mills <bcmills@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-08-21net/http: use iotest.ErrReader in testsCarlos Alexandro Becker
Updates #38781 Change-Id: I16a66904167ca4c0e916619b4da1dd23795b3ab2 GitHub-Last-Rev: 45054235a009cf776030bc951ba9a2a2a02c13e9 GitHub-Pull-Request: golang/go#40864 Reviewed-on: https://go-review.googlesource.com/c/go/+/249037 Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2020-08-04net/http: fix cancelation of requests with a readTrackingBody wrapperDamien Neil
Use the original *Request in the reqCanceler map, not the transient wrapper created to handle body rewinding. Change the key of reqCanceler to a struct{*Request}, to make it more difficult to accidentally use the wrong request as the key. Fixes #40453. Change-Id: I4e61ee9ff2c794fb4c920a3a66c9a0458693d757 Reviewed-on: https://go-review.googlesource.com/c/go/+/245357 Run-TryBot: Damien Neil <dneil@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Russ Cox <rsc@golang.org>
2020-06-17net/http: make Transport.RoundTrip preserve RequestsDamien Neil
Ensure that the exact Request passed to Transport.RoundTrip is returned in the Response. Do not replace the Request with a copy when resetting the request body. Fixes #39533 Change-Id: Ie6fb080c24b0f6625b0761b7aa542af3d2411817 Reviewed-on: https://go-review.googlesource.com/c/go/+/237560 Run-TryBot: Ian Lance Taylor <iant@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Russ Cox <rsc@golang.org>
2020-05-31net/http: reject HTTP/1.1 Content-Length with sign in responsePaschalis Tsilias
Enforces section 14.13 of RFC 2616 so that Content-Length header values with a sign such as "+5" will be rejected. Updates #39017 Change-Id: Icce9f00d03c8475fe704b33f9bed9089ff8802f0 Reviewed-on: https://go-review.googlesource.com/c/go/+/234817 Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2020-05-27net/http: handle body rewind in HTTP/2 connection loss betterRuss Cox
In certain cases the HTTP/2 stack needs to resend a request. It obtains a fresh body to send by calling req.GetBody. This call was missing from the path where the HTTP/2 round tripper returns ErrSkipAltProtocol, meaning fall back to HTTP/1.1. The result was that the HTTP/1.1 fallback request was sent with no body at all. This CL changes that code path to rewind the body before falling back to HTTP/1.1. But rewinding the body is easier said than done. Some requests have no GetBody function, meaning the body can't be rewound. If we need to rewind and can't, that's an error. But if we didn't read anything, we don't need to rewind. So we have to track whether we read anything, with a new ReadCloser wrapper. That in turn requires adding to the couple places that unwrap Body values to look at the underlying implementation. This CL adds the new rewinding code in the main retry loop as well. The new rewindBody function also takes care of closing the old body before abandoning it. That was missing in the old rewind code. Thanks to Aleksandr Razumov for CL 210123 and to Jun Chen for CL 234358, both of which informed this CL. Fixes #32441. Change-Id: Id183758526c087c6b179ab73cf3b61ed23a2a46a Reviewed-on: https://go-review.googlesource.com/c/go/+/234894 Run-TryBot: Russ Cox <rsc@golang.org> Reviewed-by: Damien Neil <dneil@google.com> Reviewed-by: Bryan C. Mills <bcmills@google.com>
2020-02-27all: fix incorrect channel and API usage in some unit testsZiheng Liu
This CL changes some unit test functions, making sure that these tests (and goroutines spawned during test) won't block. Since they are just test functions, I use one CL to fix them all. I hope this won't cause trouble to reviewers and can save time for us. There are three main categories of incorrect logic fixed by this CL: 1. Use testing.Fatal()/Fatalf() in spawned goroutines, which is forbidden by Go's document. 2. Channels are used in such a way that, when errors or timeout happen, the test will be blocked and never return. 3. Channels are used in such a way that, when errors or timeout happen, the test can return but some spawned goroutines will be leaked, occupying resource until all other tests return and the process is killed. Change-Id: I3df931ec380794a0cf1404e632c1dd57c65d63e8 Reviewed-on: https://go-review.googlesource.com/c/go/+/219380 Run-TryBot: Ian Lance Taylor <iant@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
2020-01-29net/http: don't treat an alternate protocol as a known round tripperIan Lance Taylor
As of CL 175857, the client code checks for known round tripper implementations, and uses simpler cancellation code when it finds one. However, this code was not considering the case of a request that uses a user-defined protocol, where the user-defined protocol was registered with the transport to use a different round tripper. The effect was that round trippers that worked with earlier releases would not see the expected cancellation semantics with tip. Fixes #36820 Change-Id: I60e75b5d0badcfb9fde9d73a966ba1d3f7aa42b1 Reviewed-on: https://go-review.googlesource.com/c/go/+/216618 Run-TryBot: Ian Lance Taylor <iant@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2020-01-07net/http: avoid writing to Transport.ProxyConnectHeaderBryan C. Mills
Previously, we accidentally wrote the Proxy-Authorization header for the initial CONNECT request to the shared ProxyConnectHeader map when it was non-nil. Fixes #36431 Change-Id: I5cb414f391dddf8c23d85427eb6973f14c949025 Reviewed-on: https://go-review.googlesource.com/c/go/+/213638 Run-TryBot: Bryan C. Mills <bcmills@google.com> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-12-11net/http: use cancellation instead of a timeout in ↵Bryan C. Mills
TestTransportProxyHTTPSConnectTimeout The use of a timeout in this test caused it to be flaky: if the timeout occurred before the connection was attempted, then the Accept call on the Listener could hang indefinitely, and its goroutine would not exit until that Listener was closed. That caused the test to fail. A longer timeout would make the test less flaky, but it would become even slower and would still be sensitive to timing. Instead, replace the timeout with an explicit Context cancellation after the CONNECT request has been read. That not only ensures that the cancellation occurs at the appropriate point, but also makes the test much faster: a test run with -count=1000 now executes in less than 2s on my machine, whereas before it took upwards of 50s. Fixes #36082 Updates #28012 Change-Id: I00c20d87365fd3d257774422f39d2acc8791febd Reviewed-on: https://go-review.googlesource.com/c/go/+/210857 Run-TryBot: Bryan C. Mills <bcmills@google.com> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-12-09net/http: don't wait indefinitely in Transport for proxy CONNECT responseBrad Fitzpatrick
Fixes #28012 Change-Id: I711ebaabf63194e3d2c608d829da49c51a294d74 Reviewed-on: https://go-review.googlesource.com/c/go/+/210286 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
2019-11-15all: fix a bunch of misspellingsVille Skyttä
Change-Id: I5b909df0fd048cd66c5a27fca1b06466d3bcaac7 GitHub-Last-Rev: 778c5d21311abee09a5fbda2e4005a5fd4cc3f9f GitHub-Pull-Request: golang/go#35624 Reviewed-on: https://go-review.googlesource.com/c/go/+/207421 Run-TryBot: Ian Lance Taylor <iant@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
2019-11-13net/http: add some debugging to TestDontCacheBrokenHTTP2ConnBrad Fitzpatrick
Not a fix, but will give us more info when it flakes again. Updates #35113 Change-Id: I2f90c24530c1bea81dd9d8c7a59f4b0640dfa4c2 Reviewed-on: https://go-review.googlesource.com/c/go/+/206819 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
2019-11-11net/http: add DialTLSContext hook to TransportGabriel Rosenhouse
Fixes #21526 Change-Id: I2f8215cd671641cddfa8499f8a8c0130db93dbc6 Reviewed-on: https://go-review.googlesource.com/c/go/+/61291 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
2019-11-04net/http: support disabling built-in HTTP/2 with a new build tagBrad Fitzpatrick
Fixes #35082 Updates #6853 Change-Id: I4eeb0e15f534cff57fefb6039cd33fadf15b946e Reviewed-on: https://go-review.googlesource.com/c/go/+/205139 Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com> Reviewed-by: David Crawshaw <crawshaw@golang.org>
2019-11-02net/http: deflake TestCancelRequestWithChannelBeforeDo_CancelConstantin Konstantinidis
Goroutines clean up takes longer when using deprecated CloseNotifier. Fixes #35122 Change-Id: Id820a3012b5c781ddfb294b38ee3b009624e398c Reviewed-on: https://go-review.googlesource.com/c/go/+/204661 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2019-10-29net/http: only decrement connection count if we removed a connectionMichael Fraenkel
The connection count must only be decremented if the persistent connection was also removed. Fixes #34941 Change-Id: I5070717d5d9effec78016005fa4910593500c8cf Reviewed-on: https://go-review.googlesource.com/c/go/+/202087 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-10-25net/http: skip failing test on windows-amd64-longtest builderBryan C. Mills
bradfitz is actively thinking about a proper fix. In the meantime, skip the test to suss out any other failures in the builder. Updates #35122 Change-Id: I9bf0640222e3d385c1a3e2be5ab52b80d3e8c21a Reviewed-on: https://go-review.googlesource.com/c/go/+/203500 Run-TryBot: Bryan C. Mills <bcmills@google.com> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2019-10-22net/http: don't cache http2.erringRoundTripper connectionsBrad Fitzpatrick
Fixes #34978 Change-Id: I3baf1392ba7366ae6628889c47c343ef702ec438 Reviewed-on: https://go-review.googlesource.com/c/go/+/202078 Reviewed-by: Bryan C. Mills <bcmills@google.com> Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-10-21net/http: make Transport.RoundTrip close body on any invalid requestEmmanuel T Odeke
Fixes #35015 Change-Id: I7a1ed9cfa219ad88014aad033e3a01f9dffc3eb3 Reviewed-on: https://go-review.googlesource.com/c/go/+/202239 Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2019-10-20net/http: make Transport.roundTrip close body on invalid methodLuke Young
Updates #35015 Change-Id: Ibfe8f72ed3887ca88ce9c1d8a29dacda72f3fe17 GitHub-Last-Rev: 4bfc56e71660ad9624ac5eb594b3afd0d221c99d GitHub-Pull-Request: golang/go#35014 Reviewed-on: https://go-review.googlesource.com/c/go/+/202237 Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com> Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-09-26net/textproto: don't normalize headers with spaces before the colonFilippo Valsorda
RFC 7230 is clear about headers with a space before the colon, like X-Answer : 42 being invalid, but we've been accepting and normalizing them for compatibility purposes since CL 5690059 in 2012. On the client side, this is harmless and indeed most browsers behave the same to this day. On the server side, this becomes a security issue when the behavior doesn't match that of a reverse proxy sitting in front of the server. For example, if a WAF accepts them without normalizing them, it might be possible to bypass its filters, because the Go server would interpret the header differently. Worse, if the reverse proxy coalesces requests onto a single HTTP/1.1 connection to a Go server, the understanding of the request boundaries can get out of sync between them, allowing an attacker to tack an arbitrary method and path onto a request by other clients, including authentication headers unknown to the attacker. This was recently presented at multiple security conferences: https://portswigger.net/blog/http-desync-attacks-request-smuggling-reborn net/http servers already reject header keys with invalid characters. Simply stop normalizing extra spaces in net/textproto, let it return them unchanged like it does for other invalid headers, and let net/http enforce RFC 7230, which is HTTP specific. This loses us normalization on the client side, but there's no right answer on the client side anyway, and hiding the issue sounds worse than letting the application decide. Fixes CVE-2019-16276 Fixes #34540 Change-Id: I6d272de827e0870da85d93df770d6a0e161bbcf1 Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/549719 Reviewed-by: Brad Fitzpatrick <bradfitz@google.com> Reviewed-on: https://go-review.googlesource.com/c/go/+/197503 Run-TryBot: Filippo Valsorda <filippo@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
2019-09-24net/http: remove http2 connections when no longer cachedMichael Fraenkel
When the http2 transport returns a NoCachedConnError, the connection must be removed from the idle list as well as the connections per host. Fixes #34387 Change-Id: I7875c9c95e694a37a339bb04385243b49f9b20d3 Reviewed-on: https://go-review.googlesource.com/c/go/+/196665 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-09-18net/http: fix HTTP/2 idle pool tracingTom Thorogood
CL 140357 caused HTTP/2 connections to be put in the idle pool, but failed to properly guard the trace.GotConn call in getConn. dialConn returns a minimal persistConn with conn == nil for HTTP/2 connections. This persistConn was then returned from queueForIdleConn and caused the httptrace.GotConnInfo passed into GotConn to have a nil Conn field. HTTP/2 connections call GotConn themselves so leave it for HTTP/2 to call GotConn as is done directly below. Fixes #34282 Change-Id: If54bfaf6edb14f5391463f908efbef5bb8a5d78e GitHub-Last-Rev: 2b7d66a1ce66b4424c4d0fca2b8e8b547d874136 GitHub-Pull-Request: golang/go#34283 Reviewed-on: https://go-review.googlesource.com/c/go/+/195237 Reviewed-by: Michael Fraenkel <michael.fraenkel@gmail.com> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-08-28net/http: fix a long test after CL 185117Daniel Martí
The net/url error in question now quotes the URL, so update the expected output string. While at it, also update a comment in httputil, though that doesn't affect any test. Fixes #33910. Change-Id: I0981f528b24337c2952ef60c0db3b7ff72d72110 Reviewed-on: https://go-review.googlesource.com/c/go/+/192078 Run-TryBot: Daniel Martí <mvdan@mvdan.cc> Reviewed-by: Bryan C. Mills <bcmills@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-08-27net/http: fix wantConnQueue memory leaks in TransportBryan C. Mills
I'm trying to keep the code changes minimal for backporting to Go 1.13, so it is still possible for a handful of entries to leak, but the leaks are now O(1) instead of O(N) in the steady state. Longer-term, I think it would be a good idea to coalesce idleMu with connsPerHostMu and clear entries out of both queues as soon as their goroutines are done waiting. Fixes #33849 Fixes #33850 Change-Id: Ia66bc64671eb1014369f2d3a01debfc023b44281 Reviewed-on: https://go-review.googlesource.com/c/go/+/191964 Run-TryBot: Bryan C. Mills <bcmills@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2019-08-12net/url: make Hostname and Port predictable for invalid Host valuesFilippo Valsorda
When Host is not valid per RFC 3986, the behavior of Hostname and Port was wildly unpredictable, to the point that Host could have a suffix that didn't appear in neither Hostname nor Port. This is a security issue when applications are applying checks to Host and expecting them to be meaningful for the contents of Hostname. To reduce disruption, this change only aims to guarantee the following two security-relevant invariants. * Host is either Hostname or [Hostname] with Port empty, or Hostname:Port or [Hostname]:Port. * Port is only decimals. The second invariant is the one that's most likely to cause disruption, but I believe it's important, as it's conceivable an application might do a suffix check on Host and expect it to be meaningful for the contents of Hostname (if the suffix is not a valid port). There are three ways to ensure it. 1) Reject invalid ports in Parse. Note that non-numeric ports are already rejected if and only if the host starts with "[". 2) Consider non-numeric ports as part of Hostname, not Port. 3) Allow non-numeric ports, and hope they only flow down to net/http, which will reject them (#14353). This change adopts both 1 and 2. We could do only the latter, but then these invalid hosts would flow past port checks, like in http_test.TestTransportRejectsAlphaPort. Non-numeric ports weren't fully supported anyway, because they were rejected after IPv6 literals, so this restores consistency. We could do only the former, but at this point 2) is free and might help with manually constructed Host values (or if we get something wrong in Parse). Note that net.SplitHostPort and net.Dial explicitly accept service names in place of port numbers, but this is an URL package, and RFC 3986, Section 3.2.3, clearly specifies ports as a number in decimal. net/http uses a mix of net.SplitHostPort and url.Parse that would deserve looking into, but in general it seems that it will still accept service names in Addr fields as they are passed to net.Listen, while rejecting them in URLs, which feels correct. This leaves a number of invalid URLs to reject, which however are not security relevant once the two invariants above hold, so can be done in Go 1.14: IPv6 literals without brackets (#31024), invalid IPv6 literals, hostnames with invalid characters, and more. Tested with 200M executions of go-fuzz and the following Fuzz function. u, err := url.Parse(string(data)) if err != nil { return 0 } h := u.Hostname() p := u.Port() switch u.Host { case h + ":" + p: return 1 case "[" + h + "]:" + p: return 1 case h: fallthrough case "[" + h + "]": if p != "" { panic("unexpected Port()") } return 1 } panic("Host is not a variant of [Hostname]:Port") Fixes CVE-2019-14809 Updates #29098 Change-Id: I7ef40823dab28f29511329fa2d5a7fb10c3ec895 Reviewed-on: https://go-review.googlesource.com/c/go/+/189258 Reviewed-by: Ian Lance Taylor <iant@golang.org>
2019-07-08net/http: fix Transport.MaxConnsPerHost limits & idle pool racesRuss Cox
There were at least three races in the implementation of the pool of idle HTTP connections before this CL. The first race is that HTTP/2 connections can be shared for many requests, but each requesting goroutine would take the connection out of the pool and then immediately return it before using it; this created unnecessary, tiny little race windows during which another goroutine might dial a second connection instead of reusing the first. This CL changes the idle pool to just leave the HTTP/2 connection in the pool permanently (until there is reason to close it), instead of doing the take-it-out-put-it-back dance race. The second race is that “is there an idle connection?” and “register to wait for an idle connection” were implemented as two separate steps, in different critical sections. So a client could end up registered to wait for an idle connection and be waiting or perhaps dialing, not having noticed the idle connection sitting in the pool that arrived between the two steps. The third race is that t.getIdleConnCh assumes that the inability to send on the channel means the client doesn't need the result, when it could mean that the client has not yet entered the select. That is, the main dial does: idleConnCh := t.getIdleConnCh(cm) select { case v := <-dialc: ... case pc := <-idleConnCh ... ... } But then tryPutIdleConn does: waitingDialer := t.idleConnCh[key] // what getIdleConnCh(cm) returned select { case waitingDialer <- pconn: // We're done ... return nil default: if waitingDialer != nil { // They had populated this, but their dial won // first, so we can clean up this map entry. delete(t.idleConnCh, key) } } If the client has returned from getIdleConnCh but not yet reached the select, tryPutIdleConn will be unable to do the send, incorrectly conclude that the client does not care anymore, and put the connection in the idle pool instead, again leaving the client dialing unnecessarily while a connection sits in the idle pool. (It's also odd that the success case does not clean up the map entry, and also that the map has room for only a single waiting goroutine for a given host.) None of these races mattered too much before Go 1.11: at most they meant that connections were not reused quite as promptly as possible, or a few more than necessary would be created. But Go 1.11 added Transport.MaxConnsPerHost, which limited the number of connections created for a given host. The default is 0 (unlimited), but if a user did explicitly impose a low limit (2 is common), all these misplaced conns could easily add up to the entire limit, causing a deadlock. This was causing intermittent timeouts in TestTransportMaxConnsPerHost. The addition of the MaxConnsPerHost support added its own races. For example, here t.incHostConnCount could increment the count and return a channel ready for receiving, and then the client would not receive from it nor ever issue the decrement, because the select need not evaluate these two cases in order: select { case <-t.incHostConnCount(cmKey): // count below conn per host limit; proceed case pc := <-t.getIdleConnCh(cm): if trace != nil && trace.GotConn != nil { trace.GotConn(httptrace.GotConnInfo{Conn: pc.conn, Reused: pc.isReused()}) } return pc, nil ... } Obviously, unmatched increments are another way to get to a deadlock. TestTransportMaxConnsPerHost deadlocked approximately 100% of the time with a small random sleep added between incHostConnCount and the select: ch := t.incHostConnCount(cmKey): time.Sleep(time.Duration(rand.Intn(10))*time.Millisecond) select { case <-ch // count below conn per host limit; proceed case pc := <-t.getIdleConnCh(cm): ... } The limit also did not properly apply to HTTP/2, because of the decrement being attached to the underlying net.Conn.Close and net/http not having access to the underlying HTTP/2 conn. The alternate decrements for HTTP/2 may also have introduced spurious decrements (discussion in #29889). Perhaps those spurious decrements or other races caused the other intermittent non-deadlock failures in TestTransportMaxConnsPerHost, in which the HTTP/2 phase created too many connections (#31982). This CL replaces the buggy, racy code with new code that is hopefully neither buggy nor racy. Fixes #29889. Fixes #31982. Fixes #32336. Change-Id: I0dfac3a6fe8a6cdf5f0853722781fe2ec071ac97 Reviewed-on: https://go-review.googlesource.com/c/go/+/184262 Run-TryBot: Russ Cox <rsc@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
2019-05-30net/http: prevent Transport from spamming stderr on server 408 replyBrad Fitzpatrick
HTTP 408 responses now exist and are seen in the wild (e.g. from Google's GFE), so make Go's HTTP client not spam about them when seen. They're normal (now). Fixes #32310 Change-Id: I558eb4654960c74cf20db1902ccaae13d03310f6 Reviewed-on: https://go-review.googlesource.com/c/go/+/179457 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
2019-05-28net/http: fix TestTransportServerClosingUnexpectedly flakeBrad Fitzpatrick
Fixes #32119 Change-Id: I8cf2e2e69737e2485568af91ab75149f3cf66781 Reviewed-on: https://go-review.googlesource.com/c/go/+/178918 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
2019-05-03net/http: fix TestTransportMaxConnsPerHost flakesMichael Fraenkel
The testcase created a race between the close of the current connection and the client grabbing a connection for the next request. The client may receive the current connection which may be closed during its use. We can have the trasnport close all idle connections thereby forcing the client to receive a new connection. Closing idle connections did not handle cleaning up host connection counts for http/2. We will now decrement the host connection count for http/2 connections. Fixes #31784 Change-Id: Iefc0d0d7ed9fa3acd8b4f42004f1579fc1de63fd Reviewed-on: https://go-review.googlesource.com/c/go/+/174950 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-05-03net/http: add Transport.CloneBrad Fitzpatrick
Fixes #26013 Change-Id: I2c82bd90ea7ce6f7a8e5b6c460d3982dca681a93 Reviewed-on: https://go-review.googlesource.com/c/go/+/174597 Reviewed-by: Andrew Bonventre <andybons@golang.org>
2019-05-02net/http: skip flaky TestTransportMaxConnsPerHost for nowBrad Fitzpatrick
Updates #31784 Change-Id: Iee056c850c03939606b227a12715c76b0339d268 Reviewed-on: https://go-review.googlesource.com/c/go/+/175097 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
2019-04-30net/http: make Transport.MaxConnsPerHost work for HTTP/2Michael Fraenkel
Treat HTTP/2 connections as an ongoing persistent connection. When we are told there is no cached connections, cleanup the associated connection and host connection count. Fixes #27753 Change-Id: I6b7bd915fc7819617cb5d3b35e46e225c75eda29 Reviewed-on: https://go-review.googlesource.com/c/go/+/140357 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2019-04-16all: s/cancelation/cancellation/Josh Bleecher Snyder
Though there is variation in the spelling of canceled, cancellation is always spelled with a double l. Reference: https://www.grammarly.com/blog/canceled-vs-cancelled/ Change-Id: I240f1a297776c8e27e74f3eca566d2bc4c856f2f Reviewed-on: https://go-review.googlesource.com/c/go/+/170060 Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
7apu2bq3rhoylwmucxizk54afw5jyda7pho4j5zdcqpwkufke7zdzwad.onion