aboutsummaryrefslogtreecommitdiff
path: root/src/net/http/export_test.go
AgeCommit message (Collapse)Author
5 daysnet/http: add support for running HTTP tests against HTTP/3Nicholas S. Husin
Add support within clientserver_test.go to bring up a test HTTP/3 server and client when http3Mode testMode option is passed. To be able to reuse net/http/httptest, net/http/httptest.Server.StartTLS (and Start) have been modified so they can be called with a nil Listener. In such cases, both methods will behave identically as usual, but will not actually make its server serve or set its transport dialer, both of which requires having a listener. This should be a no-op for regular users of the package, whose entrypoint via functions such as NewServer will automatically set a local listener. Actually enabling HTTP/3 for our tests will be done in a separate CL. For #70914 Change-Id: Ibc5fc83287b6a04b46e668a54924761a92b620a4 Reviewed-on: https://go-review.googlesource.com/c/go/+/740122 Reviewed-by: Damien Neil <dneil@google.com> Reviewed-by: Nicholas Husin <husin@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
2026-03-12net/http: use net/http/internal/http2 rather than h2_bundle.goDamien Neil
Rework net/http/internal/http2 to use internally-defined types rather than net/http types (to avoid an import cycle). Remove h2_bundle.go, and replace it with calls into net/http/internal/http2 instead. For #67810 Change-Id: I56a1b28dbd0e302ab15a30f819dd46256a6a6964 Reviewed-on: https://go-review.googlesource.com/c/go/+/751304 Reviewed-by: Nicholas Husin <nsh@golang.org> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Auto-Submit: Damien Neil <dneil@google.com> Reviewed-by: Nicholas Husin <husin@google.com>
2026-03-12net/http/internal/http2: remove ExportSetH2GoawayTimeoutDamien Neil
This was used by one test. Rewrite that test to use synctest (faster!) and not care about the timeout. For #67810 Change-Id: I61496e575ae8a16ff778470f3f9d711e6a6a6964 Reviewed-on: https://go-review.googlesource.com/c/go/+/751303 Reviewed-by: Nicholas Husin <husin@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Auto-Submit: Damien Neil <dneil@google.com> Reviewed-by: Nicholas Husin <nsh@golang.org>
2026-02-02net/http: try to drain response body upon closing for better connection re-useNicholas S. Husin
Currently, we have a rather inconsistent behavior in terms of whether a connection can be re-used or not when an HTTP body is not read to completion: - In HTTP/2, not reading bodies to completion is not an issue, since a new HTTP/2 stream can be created on the same TCP connection. - In HTTP/1 server, we discard up to 256 KiB of unconsumed request body, to potentially allow re-use. - In HTTP/1 client, we do not do anything, and fail to re-use a TCP connection if there are any unconsumed response body at all. This has led to some confusion. For example, some users have mistakenly discarded response body for HTTP/2 when doing so is not needed. Manually discarding response body can also be disadvantageous if the body is excessively large or is a never-ending stream. To solve this issue, this CL makes it so that closing a response body will cause any remaining content to be drained, up to a limit of 256 KiB or 50 milliseconds, whichever one is reached first. This allows better connection re-use for HTTP/1, and most users can now avoid having to manually drain their response body. For #77370 Change-Id: I71e1227fc9cf5f901362c8e234320817f6b0be24 Reviewed-on: https://go-review.googlesource.com/c/go/+/737720 Reviewed-by: Nicholas Husin <husin@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Damien Neil <dneil@google.com>
2025-02-10net/http: use standard time formatting methodsTom Thorogood
time.Time has had an AppendFormat method since go1.5 so there's no need to carry around a custom implementation. Change-Id: I8e7e5a9ac34e8bf251f5d70555405777ce4e22a8 Reviewed-on: https://go-review.googlesource.com/c/go/+/647955 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Damien Neil <dneil@google.com> Auto-Submit: Ian Lance Taylor <iant@golang.org>
2024-04-17net/http: don't cancel Dials when requests are canceledDamien Neil
Currently, when a Transport creates a new connection for a request, it uses the request's Context to make the Dial. If a request times out or is canceled before a Dial completes, the Dial is canceled. Change this so that the lifetime of a Dial call is not bound by the request that originated it. This change avoids a scenario where a Transport can start and then cancel many Dial calls in rapid succession: - Request starts a Dial. - A previous request completes, making its connection available. - The new request uses the now-idle connection, and completes. - The request Context is canceled, and the Dial is aborted. Fixes #59017 Change-Id: I996ffabc56d3b1b43129cbfd9b3e9ea7d53d263c Reviewed-on: https://go-review.googlesource.com/c/go/+/576555 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
2024-03-21net/http: use slices to simplify the codeapocelipes
"strSliceContains" is replaced by "slices.Contains". Replace "sort.Strings" with "slices.Sort" since it becomes a wrapper of "slices.Sort" from Go 1.22. "headerSorter" no longer has to implement "sort.Interface". We use "slice.SortFunc" to sort kvs. Change-Id: Ic29b4c3db147c16079575eca7ad6ff6c0f581188 GitHub-Last-Rev: 78221d5aa223a259a89860b672f39a34897df253 GitHub-Pull-Request: golang/go#66440 Reviewed-on: https://go-review.googlesource.com/c/go/+/573275 Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: qiulaidongfeng <2645477756@qq.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Damien Neil <dneil@google.com> Auto-Submit: Ian Lance Taylor <iant@golang.org> Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com> TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Emmanuel Odeke <emmanuel@orijtech.com>
2023-09-13net/http: scale rstAvoidanceDelay to reduce test flakinessBryan C. Mills
As far as I can tell, some flakiness is unavoidable in tests that race a large client request write against a server's response when the server doesn't read the full request. It does not appear to be possible to simultaneously ensure that well-behaved clients see EOF instead of ECONNRESET and also prevent misbehaving clients from consuming arbitrary server resources. (See RFC 7230 §6.6 for more detail.) Since there doesn't appear to be a way to cleanly eliminate this source of flakiness, we can instead work around it: we can allow the test to adjust the hard-coded delay if it sees a plausibly-related failure, so that the test can retry with a longer delay. As a nice side benefit, this also allows the tests to run more quickly in the typical case: since the test will retry in case of spurious failures, we can start with an aggressively short delay, and only back off to a longer one if it is really needed on the specific machine running the test. Fixes #57084. Fixes #51104. For #58398. Change-Id: Ia4050679f0777e5eeba7670307a77d93cfce856f Cq-Include-Trybots: luci.golang.try:gotip-linux-amd64-longtest-race,gotip-linux-amd64-race,gotip-windows-amd64-race Reviewed-on: https://go-review.googlesource.com/c/go/+/527196 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Damien Neil <dneil@google.com> Auto-Submit: Bryan Mills <bcmills@google.com>
2023-04-07net/http: wait forever for write results in testsDamien Neil
After performing a round trip on a connection, the connection is usually returned to the idle connection pool. If the write of the request did not complete successfully, the connection is not returned. It is possible for the response to be read before the write goroutine has finished signalling that its write has completed. To allow for this, the check to see if the write completed successfully waits for 50ms for the write goroutine to report the result of the write. See comments in persistConn.wroteRequest for more details. On a slow builder, it is possible for the write goroutine to take longer than 50ms to report the status of its write, leading to test flakiness when successive requests unexpectedly use different connections. Set the timeout for waiting for the writer to an effectively infinite duration in tests. Fixes #51147 Fixes #56275 Fixes #56419 Fixes #56577 Fixes #57375 Fixes #57417 Fixes #57476 Fixes #57604 Fixes #57605 Change-Id: I5e92ffd66b676f3f976d8832c0910f27456a6991 Reviewed-on: https://go-review.googlesource.com/c/go/+/483116 TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Bryan Mills <bcmills@google.com> Run-TryBot: Bryan Mills <bcmills@google.com>
2023-03-22net/http: in the IdleConnStrsForTesting_h2 helper, omit conns that cannot be ↵Bryan C. Mills
reused In #59155, we observed that the IdleConnStrsForTesting_h2 helper function sometimes reported extra connections after a "client conn not usable" failure and retry. It turns out that that state corresponds exactly to the http2clientConnIdleState.canTakeNewRequest field, so (with a bit of extra nethttpomithttp2 plumbing) we can use that field in the helper to filter out the unusable connections. Fixes #59155. Change-Id: Ief6283c9c8c5ec47dd9f378beb0ddf720832484e Reviewed-on: https://go-review.googlesource.com/c/go/+/477856 Reviewed-by: Damien Neil <dneil@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Bryan Mills <bcmills@google.com> Auto-Submit: Bryan Mills <bcmills@google.com>
2022-10-07net/http: refactor tests to run most in HTTP/1 and HTTP/2 modesDamien Neil
Replace the ad-hoc approach to running tests in HTTP/1 and HTTP/2 modes with a 'run' function that executes a test in various modes. By default, these modes are HTTP/1 and HTTP/2, but tests can opt-in to HTTPS/1 as well. The 'run' function also takes care of post-test cleanup (running the afterTest function). The 'run' function runs tests in parallel by default. Tests which can't run in parallel (generally because they use global test hooks) pass a testNotParallel option to disable parallelism. Update clientServerTest to use t.Cleanup to clean up after itself, rather than leaving this up to tests to handle. Drop an unnecessary mutex in SetReadLoopBeforeNextReadHook. Test hooks can't be set in parallel, and we want the race detector to notify us if two simultaneous tests try to set a hook. Fixes #56032 Change-Id: I16be64913c426fc93d84abc6ad85dbd3bc191224 Reviewed-on: https://go-review.googlesource.com/c/go/+/438137 TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Damien Neil <dneil@google.com> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Reviewed-by: David Chase <drchase@google.com>
2022-04-20net/http: deflake TestTransportConnectionCloseOnRequestBrad Fitzpatrick
Fixes #52450 (hopefully) Change-Id: Ib723f8efb4a13af1b98c25cd02935425172d01e6 Reviewed-on: https://go-review.googlesource.com/c/go/+/401314 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Bryan Mills <bcmills@google.com> Reviewed-by: Damien Neil <dneil@google.com>
2021-11-05net/http: distinguish between timeouts and client hangups in TimeoutHandlerCharlie Getzen
Fixes #48948 Change-Id: I411e3be99c7979ae289fd937388aae63d81adb59 GitHub-Last-Rev: 14abd7e4d774ed5ef63aa0a69e80fbc8b5a5af26 GitHub-Pull-Request: golang/go#48993 Reviewed-on: https://go-review.googlesource.com/c/go/+/356009 Reviewed-by: Damien Neil <dneil@google.com> Trust: Damien Neil <dneil@google.com> Trust: Ian Lance Taylor <iant@golang.org> Run-TryBot: Damien Neil <dneil@google.com> TryBot-Result: Go Bot <gobot@golang.org>
2020-11-11all: update vendored dependencies for Go 1.16 releaseDmitri Shuralyov
The Go 1.16 code freeze has recently started. This is a time to update all golang.org/x/... module versions that contribute packages to the std and cmd modules in the standard library to latest master versions. Those versions have already gone through code review, and now they will undergo additional testing during the upcoming freeze period. If new issues in these dependencies are discovered, we have the freeze period to address them. By the end of the freeze period, we will have confidence that the Go 1.16 release and the dependency versions it has selected are robust. The dependency module versions that are selected in this commit are: github.com/google/pprof v0.0.0-20201007051231-1066cbb265c7 github.com/ianlancetaylor/demangle v0.0.0-20200414190113-039b1ae3a340 golang.org/x/arch v0.0.0-20201008161808-52c3e6f60cff golang.org/x/crypto v0.0.0-20201016220609-9e8e0b390897 golang.org/x/mod v0.3.1-0.20200828183125-ce943fd02449 golang.org/x/net v0.0.0-20201029221708-28c70e62bb1d golang.org/x/sys v0.0.0-20201110211018-35f3e6cf4a65 golang.org/x/text v0.3.4 golang.org/x/tools v0.0.0-20201110201400-7099162a900a golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 This change was created with a program from CL 256357 patch set 3 (which updates golang.org/x modules only) and the latest bundle tool, but replacing golang.org/x/net version with a slightly older commit golang/net@28c70e62bb1d140c3f2579fb7bb5095134d9cb1e due to #42498: $ updatestd -goroot=$HOME/gotip -branch=master > go version go version devel +ecc3f5112e Thu Nov 5 23:21:33 2020 +0000 darwin/amd64 > go env GOROOT /Users/dmitshur/gotip > go version -m /Users/dmitshur/go/bin/bundle /Users/dmitshur/go/bin/bundle: go1.15.4 path golang.org/x/tools/cmd/bundle mod golang.org/x/tools v0.0.0-20201110201400-7099162a900a h1:5E6TPwSBG74zT8xSrVc8W59K4ch4NFobVTnh2BYzHyU= dep golang.org/x/mod v0.3.0 h1:RM4zey1++hCTbCVQfnWeKs9/IEsaBLA8vTkd0WVtmH4= dep golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 h1:go1bK/D/BFZV2I8cIQd1NKEZ+0owSTG1fDTci4IqFcE= updating module cmd in /Users/dmitshur/gotip/src/cmd skipping github.com/chzyer/logex (out of scope, it's not a golang.org/x dependency) skipping github.com/chzyer/readline (out of scope, it's not a golang.org/x dependency) skipping github.com/chzyer/test (out of scope, it's not a golang.org/x dependency) skipping github.com/google/pprof (out of scope, it's not a golang.org/x dependency) skipping github.com/ianlancetaylor/demangle (out of scope, it's not a golang.org/x dependency) skipping github.com/yuin/goldmark (out of scope, it's not a golang.org/x dependency) skipping rsc.io/pdf (out of scope, it's not a golang.org/x dependency) > go mod edit -go=1.16 > go get -d golang.org/x/arch@52c3e6f60cffa0133a3f9b2fc7f6862504a6cba0 golang.org/x/crypto@9e8e0b390897c84cad53ebe9ed2d1d331a5394d9 golang.org/x/mod@ce943fd02449f621243c9ea6e64098e84752b92b golang.org/x/net@28c70e62bb1d140c3f2579fb7bb5095134d9cb1e golang.org/x/sync@67f06af15bc961c363a7260195bcd53487529a21 golang.org/x/sys@35f3e6cf4a65a85bc280e5fe63faed8ac8b25721 golang.org/x/text@22f1617af38ed4cd65b3b96e02bab267e560155c golang.org/x/tools@7099162a900ae8260c5b97cfaf5f374243dfa742 golang.org/x/xerrors@5ec99f83aff198f5fbd629d6c8d8eb38a04218ca > go mod tidy > go mod vendor updating module std in /Users/dmitshur/gotip/src > go mod edit -go=1.16 > go get -d golang.org/x/crypto@9e8e0b390897c84cad53ebe9ed2d1d331a5394d9 golang.org/x/net@28c70e62bb1d140c3f2579fb7bb5095134d9cb1e golang.org/x/sys@35f3e6cf4a65a85bc280e5fe63faed8ac8b25721 golang.org/x/text@22f1617af38ed4cd65b3b96e02bab267e560155c golang.org/x/tools@7099162a900ae8260c5b97cfaf5f374243dfa742 > go mod tidy > go mod vendor updating bundles in /Users/dmitshur/gotip/src > go generate -run=bundle std cmd golang.org/x/net will be updated further later, after #42498 is fixed. github.com/google/pprof and github.com/ianlancetaylor/demangle contribute packages but are out of scope for this generated CL. Also rename http2configureTransport in net/http to follow the internal rename that happened in CL 264017 to fix the build. For #36905. Updates #41721. Updates #42498. Change-Id: Ifcd2e76f0406e389b6db88041ca51cd0a2115152 Reviewed-on: https://go-review.googlesource.com/c/go/+/266898 Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com> Trust: Dmitri Shuralyov <dmitshur@golang.org>
2020-09-06net/http: mark http/2 connections activeMichael Fraenkel
On Server.Shutdown, all idle connections are closed. A caveat for new connections is that they are marked idle after 5 seconds. Previously new HTTP/2 connections were marked New, and after 5 seconds, they would then become idle. With this change, we now mark HTTP/2 connections as Active to allow the proper shutdown sequence to occur. Fixes #36946 Fixes #39776 Change-Id: I31efbf64b9a2850ca544da797f86d7e1b3378e8b Reviewed-on: https://go-review.googlesource.com/c/go/+/240278 Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com> Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com> TryBot-Result: Gobot Gobot <gobot@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-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-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>
2018-11-14net/http: make Transport respect {X-,}Idempotency-Key headerBrad Fitzpatrick
Fixes #19943 Change-Id: I5e0fefe44791d7b3556095d726c2a753ec551ef2 Reviewed-on: https://go-review.googlesource.com/c/147457 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
2018-10-02net/http: make Transport send WebSocket upgrade requests over HTTP/1Brad Fitzpatrick
WebSockets requires HTTP/1 in practice (no spec or implementations work over HTTP/2), so if we get an HTTP request that looks like it's trying to initiate WebSockets, use HTTP/1, like browsers do. This is part of a series of commits to make WebSockets work over httputil.ReverseProxy. See #26937. Updates #26937 Change-Id: I6ad3df9b0a21fddf62fa7d9cacef48e7d5d9585b Reviewed-on: https://go-review.googlesource.com/c/137437 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
2018-07-24net/http: document that Client methods always return *url.ErrorBrad Fitzpatrick
Updates #9424 Change-Id: If117ba3e7d031f84b30d3a721ef99fe622734de2 Reviewed-on: https://go-review.googlesource.com/125575 Reviewed-by: Ian Lance Taylor <iant@golang.org>
2018-07-24net/http: deflake TestRetryRequestsOnErrorBrad Fitzpatrick
There's a 50ms threshold in net/http.Transport that this test sometimes hitting on slower devices. That was unrelated to what this test was trying to test. So instead just t.Skip on RoundTrip errors unless the failure was quick (under 25ms), in which case the error must've been about something else. Our fast machines should catch regressions there. Fixes #25366 Change-Id: Ibe8e2716a5b68558b57d0b8b5c46f38e46a2cba2 Reviewed-on: https://go-review.googlesource.com/125555 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
2018-07-12net/http: make Transport.CloseIdleConnections close non-bundled http2.TransportBrad Fitzpatrick
Previously Transport.CloseIdleConnections only closed the HTTP/2 Transport's idle connections if the HTTP/2 transport was configured automatically via the bundled copy (in h2_bundle.go). This makes it also work if the user called http2.ConfigureTransport themselves using golang.org/x/net/http2 instead of the bundled copy. No tests because we have no current way to run such cross-repo tests, at least in any efficient or non-flaky way. Tested by hand that: package main import ( "net/http" "golang.org/x/net/http2" ) func main() { tr := &http.Transport{} http2.ConfigureTransport(tr) tr.CloseIdleConnections() } ... now works and calls the x/net/http2.Transport.CloseIdleConnections code. (I threw in a print statement locally) Fixes #22891 once CL 123656 is also in. Change-Id: Id697fd3e7877c3a988bc3c3368b88940ba56cfd0 Reviewed-on: https://go-review.googlesource.com/123657 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
2018-07-09net/http: add Transport.MaxConnsPerHost knobMark Fischer
Add field to http.Transport which limits connections per host, including dial-in-progress, in-use and idle (keep-alive) connections. For HTTP/2, this field only controls the number of dials in progress. Fixes #13957 Change-Id: I7a5e045b4d4793c6b5b1a7191e1342cd7df78e6c Reviewed-on: https://go-review.googlesource.com/71272 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>
2018-06-28net/http: make Server.Shutdown treat new connections as idle after 5 secondsBrad Fitzpatrick
The Server distinguishes "new" vs "idle" connections. A TCP connection from which no bytes have yet been written is "new". A connection that has previously served a request and is in "keep-alive" state while waiting for a second or further request is "idle". The graceful Server.Shutdown historically only shut down "idle" connections, with the assumption that a "new" connection was about to read its request and would then shut down on its own afterwards. But apparently some clients spin up connections and don't end up using them, so we have something that's "new" to us, but browsers or other clients are treating as "idle" to them. This CL tweaks our heuristic to treat a StateNew connection as StateIdle if it's been stuck in StateNew for over 5 seconds. Fixes #22682 Change-Id: I01ba59a6ab67755ca5ab567041b1f54aa7b7da6f Reviewed-on: https://go-review.googlesource.com/121419 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>
2018-05-29net/http: vendor x/net/http/httpproxy, use it in net/httpRoger Peppe
From x/net git rev c7086645de2. Updates #16704 Change-Id: I4d642478fc69a52c973964845fca2fd402716e57 Reviewed-on: https://go-review.googlesource.com/68091 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>
2017-12-01net/http: speed up and deflake TestServerKeepAlivesEnabled_h2Brad Fitzpatrick
Fixes #21724 Change-Id: I92571bf228781b17fdf012a2fb52a597c877cefe Reviewed-on: https://go-review.googlesource.com/81576 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Tom Bergan <tombergan@google.com>
2017-08-28net/http: Set a timeout on Request.Context when using TimeoutHandlerMichael Fraenkel
In TimeoutHandler, use a request whose context has been configured with the handler's timeout Fixes #20712 Change-Id: Ie670148f85fdad46841ff29232042309e15665ae Reviewed-on: https://go-review.googlesource.com/46412 Run-TryBot: Tom Bergan <tombergan@google.com> Reviewed-by: Tom Bergan <tombergan@google.com>
2017-06-05net/http: make Transport retry GetBody requests if nothing writtenDavid Glasser
This is another attempt at the change attempted in https://golang.org/cl/27117 and rolled back in https://golang.org/cl/34134 The difference between this and the previous attempt is that this version only retries if the new field GetBody is set on the Request. Additionally, this allows retries of requests with idempotent methods even if they have bodies, as long as GetBody is defined. This also fixes an existing bug where readLoop could make a redundant call to setReqCanceler for DELETE/POST/PUT/etc requests with no body with zero bytes written. This clarifies the existing TestRetryIdempotentRequestsOnError test (and changes it into a test with 4 subtests). When that test was written, it was in fact testing "retry idempotent requests" logic, but the logic had changed since then, and it was actually testing "retry requests with no body when no bytes have been written". (You can confirm this by changing the existing test from a GET to a DELETE; it passes without the changes in this CL.) We now test for the no-Body and GetBody cases for both idempotent and nothing-written-non-idempotent requests. Fixes #18241 Fixes #17844 Change-Id: I69a48691796f6dc08c31f7aa7887b7dfd67e278a Reviewed-on: https://go-review.googlesource.com/42142 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2017-04-28net/http: re-simplify HTTP/1.x status line writingBrad Fitzpatrick
It used to be simple, and then it got complicated for speed (to reduce allocations, mostly), but that involved a mutex and hurt multi-core performance, contending on the mutex. A change was sent to try to improve that mutex contention in https://go-review.googlesource.com/c/42110/2/src/net/http/server.go but that introduced its own allocations (the string->interface{} boxing for the sync.Map key), which runs counter to the whole point of that statusLine function: to remove allocations. Instead, make the code simple again and not have a mutex. It's a bit slower for the single-core case, but nobody with a single-user HTTP server cares about 50 nanoseconds: name old time/op new time/op delta ResponseStatusLine 37.5ns ± 2% 87.1ns ± 2% +132.42% (p=0.029 n=4+4) ResponseStatusLine-2 63.1ns ± 1% 43.1ns ±12% -31.67% (p=0.029 n=4+4) ResponseStatusLine-4 53.8ns ± 8% 40.2ns ± 2% -25.29% (p=0.029 n=4+4) name old alloc/op new alloc/op delta ResponseStatusLine 0.00B ±NaN% 0.00B ±NaN% ~ (all samples are equal) ResponseStatusLine-2 0.00B ±NaN% 0.00B ±NaN% ~ (all samples are equal) ResponseStatusLine-4 0.00B ±NaN% 0.00B ±NaN% ~ (all samples are equal) name old allocs/op new allocs/op delta ResponseStatusLine 0.00 ±NaN% 0.00 ±NaN% ~ (all samples are equal) ResponseStatusLine-2 0.00 ±NaN% 0.00 ±NaN% ~ (all samples are equal) ResponseStatusLine-4 0.00 ±NaN% 0.00 ±NaN% ~ (all samples are equal) (Note the code could be even simpler with fmt.Fprintf, but that is relatively slow and involves a bunch of allocations getting arguments into interface{} for the call) Change-Id: I1fa119132dbbf97a8e7204ce3e0707d433060da2 Reviewed-on: https://go-review.googlesource.com/42133 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Bryan Mills <bcmills@google.com>
2017-03-02net/http: clean up Transport.RoundTrip error handlingBrad Fitzpatrick
If I put a 10 millisecond sleep at testHookWaitResLoop, before the big select in (*persistConn).roundTrip, two flakes immediately started happening, TestTransportBodyReadError (#19231) and TestTransportPersistConnReadLoopEOF. The problem was that there are many ways for a RoundTrip call to fail (errors reading from Request.Body while writing the response, errors writing the response, errors reading the response due to server closes, errors due to servers sending malformed responses, cancelations, timeouts, etc.), and many of those failures then tear down the TCP connection, causing more failures, since there are always at least three goroutines involved (reading, writing, RoundTripping). Because the errors were communicated over buffered channels to a giant select, the error returned to the caller was a function of which random select case was called, which was why a 10ms delay before the select brought out so many bugs. (several fixed in my previous CLs the past few days). Instead, track the error explicitly in the transportRequest, guarded by a mutex. In addition, this CL now: * differentiates between the two ways writing a request can fail: the io.Copy reading from the Request.Body or the io.Copy writing to the network. A new io.Reader type notes read errors from the Request.Body. The read-from-body vs write-to-network errors are now prioritized differently. * unifies the two mapRoundTripErrorFromXXX methods into one mapRoundTripError method since their logic is now the same. * adds a (*Request).WithT(*testing.T) method in export_test.go, usable by tests, to call t.Logf at points during RoundTrip. This is disabled behind a constant except when debugging. * documents and deflakes TestClientRedirectContext I've tested this CL with high -count values, with/without -race, with/without delays before the select, etc. So far it seems robust. Fixes #19231 (TestTransportBodyReadError flake) Updates #14203 (source of errors unclear; they're now tracked more) Updates #15935 (document Transport errors more; at least understood more now) Change-Id: I3cccc3607f369724b5344763e35ad2b7ea415738 Reviewed-on: https://go-review.googlesource.com/37495 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
2016-11-03net/http: support If-Match in ServeContentDan Harrington
- Added support for If-Match and If-Unmodified-Since. - Precondition checks now more strictly follow RFC 7232 section 6, which affects precedence when multiple condition headers are present. - When serving a 304, Last-Modified header is now removed when no ETag is present (as suggested by RFC 7232 section 4.1). - If-None-Match supports multiple ETags. - ETag comparison now correctly handles weak ETags. Fixes #17572 Change-Id: I35039dea6811480ccf2889f8ed9c6a39ce34bfff Reviewed-on: https://go-review.googlesource.com/32014 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2016-11-03net/http: deflake TestServerSetKeepAlivesEnabledClosesConnsBrad Fitzpatrick
Fixes #17754 Updates #9478 (details in here) Change-Id: Iae2c1ca05a18ed266b53b2594c22fc57fab33c5e Reviewed-on: https://go-review.googlesource.com/32587 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
2016-11-01net/http: add Server.Close & Server.Shutdown for forced & graceful shutdownBrad Fitzpatrick
Also updates x/net/http2 to git rev 541150 for: http2: add support for graceful shutdown of Server https://golang.org/cl/32412 http2: make http2.Server access http1's Server via an interface check https://golang.org/cl/32417 Fixes #4674 Fixes #9478 Change-Id: I8021a18dee0ef2fe3946ac1776d2b10d3d429052 Reviewed-on: https://go-review.googlesource.com/32329 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2016-09-30net/http: update bundled http2, add h2 Transport.IdleConnTimeout testsBrad Fitzpatrick
Updates bundled http2 to x/net git rev a333c53 for: http2: add Transport support for IdleConnTimeout https://golang.org/cl/30075 And add tests. The bundled http2 also includes a change adding a Ping method to http2.ClientConn, but that type isn't exposed in the standard library. Nevertheless, the code gets compiled and adds a dependency on "crypto/rand", requiring an update to go/build's dependency test. Because net/http already depends on crypto/tls, which uses crypto/rand, it's not really a new dependency. Fixes #16808 Change-Id: I1ec8666ea74762f27c70a6f30a366a6647f923f7 Reviewed-on: https://go-review.googlesource.com/30078 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
2016-09-09net/http: make Client copy headers on redirectBrad Fitzpatrick
Copy all of the original request's headers on redirect, unless they're sensitive. Only send sensitive ones to the same origin, or subdomains thereof. Fixes #4800 Change-Id: Ie9fa75265c9d5e4c1012c028d31fd1fd74465712 Reviewed-on: https://go-review.googlesource.com/28930 Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com> Reviewed-by: Francesc Campoy Flores <campoy@golang.org> Reviewed-by: Ross Light <light@google.com> Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>
2016-05-20net/http: update bundled http2Brad Fitzpatrick
Updates x/net/http2 to git rev 8a52c78 for golang.org/cl/23258 (http2: fix Transport.CloseIdleConnections when http1+http2 are wired together) Fixes #14607 Change-Id: I038badc69e230715b8ce4e398eb5e6ede73af918 Reviewed-on: https://go-review.googlesource.com/23280 Reviewed-by: Andrew Gerrand <adg@golang.org> Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>
2016-05-01net/http: add Transport.IdleConnTimeoutBrad Fitzpatrick
Don't keep idle HTTP client connections open forever. Add a new knob, Transport.IdleConnTimeout, and make the default be 90 seconds. I figure 90 seconds is more than a minute, and less than infinite, and I figure enough code has things waking up once a minute polling APIs. This also removes the Transport's idleCount field which was unused and redundant with the size of the idleLRU map (which was actually used). Change-Id: Ibb698a9a9a26f28e00a20fe7ed23f4afb20c2322 Reviewed-on: https://go-review.googlesource.com/22670 Reviewed-by: Andrew Gerrand <adg@golang.org> Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>
2016-05-01net/http: add Transport.MaxIdleConns limitBrad Fitzpatrick
The HTTP client had a limit for the maximum number of idle connections per-host, but not a global limit. This CLs adds a global idle connection limit too, Transport.MaxIdleConns. All idle conns are now also stored in a doubly-linked list. When there are too many, the oldest one is closed. Fixes #15461 Change-Id: I72abbc28d140c73cf50f278fa70088b45ae0deef Reviewed-on: https://go-review.googlesource.com/22655 Reviewed-by: Andrew Gerrand <adg@golang.org> Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>
2016-03-21all: delete dead test codeDominik Honnef
This deletes unused code and helpers from tests. Change-Id: Ie31d46115f558ceb8da6efbf90c3c204e03b0d7e Reviewed-on: https://go-review.googlesource.com/20927 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>
2016-03-07net/http: TimeoutHandler should start timer when serving requestCaio Marcelo de Oliveira Filho
TimeoutHandler was starting the Timer when the handler was created, instead of when serving a request. It also was sharing it between multiple requests, which is incorrect, as the requests might start at different times. Store the timeout duration and create the Timer when ServeHTTP is called. Different requests will have different timers. The testing plumbing was simplified to store the channel used to control when timeout happens. It overrides the regular timer. Fixes #14568. Change-Id: I4bd51a83f412396f208682d3ae5e382db5f8dc81 Reviewed-on: https://go-review.googlesource.com/20046 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>
2016-03-01all: make copyright headers consistent with one space after periodBrad Fitzpatrick
This is a subset of https://golang.org/cl/20022 with only the copyright header lines, so the next CL will be smaller and more reviewable. Go policy has been single space after periods in comments for some time. The copyright header template at: https://golang.org/doc/contribute.html#copyright also uses a single space. Make them all consistent. Change-Id: Icc26c6b8495c3820da6b171ca96a74701b4a01b0 Reviewed-on: https://go-review.googlesource.com/20111 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org> Reviewed-by: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org>
2016-01-13net/http: fix Transport crash when abandoning dial which upgrades protosBrad Fitzpatrick
When the Transport was creating an bound HTTP connection (protocol unknown initially) and then ends up deciding it doesn't need it, a goroutine sits around to clean up whatever the result was. That goroutine made the false assumption that the result was always an HTTP/1 connection or an error. It may also be an alternate protocol in which case the *persistConn.conn net.Conn field is nil, and the alt field is non-nil. Fixes #13839 Change-Id: Ia4972e5eb1ad53fa00410b3466d4129c753e0871 Reviewed-on: https://go-review.googlesource.com/18573 Reviewed-by: Russ Cox <rsc@golang.org> Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>
2016-01-05net/http: tighten protocol between Transport.roundTrip and persistConn.readLoopBrad Fitzpatrick
In debugging the flaky test in #13825, I discovered that my previous change to tighten and simplify the communication protocol between Transport.roundTrip and persistConn.readLoop in https://golang.org/cl/17890 wasn't complete. This change simplifies it further: the buffered-vs-unbuffered complexity goes away, and we no longer need to re-try channel reads in the select case. It was trying to prioritize channels in the case that two were readable in the select. (it was only failing in the race builder because the race builds randomize select scheduling) The problem was that in the bodyless response case we had to return the idle connection before replying to roundTrip. But putIdleConn previously both added it to the free list (which we wanted), but also closed the connection, which made the caller goroutine (Transport.roundTrip) have two readable cases: pc.closech, and the response. We guarded against similar conditions in the caller's select for two readable channels, but such a fix wasn't possible here, and would be overly complicated. Instead, switch to unbuffered channels. The unbuffered channels were only to prevent goroutine leaks, so address that differently: add a "callerGone" channel closed by the caller on exit, and select on that during any unbuffered sends. As part of the fix, split putIdleConn into two halves: a part that just returns to the freelist, and a part that also closes. Update the four callers to the variants each wanted. Incidentally, the connections were closing on return to the pool due to MaxIdleConnsPerHost (somewhat related: #13801), but this bug could've manifested for plenty of other reasons. Fixes #13825 Change-Id: I6fa7136e2c52909d57a22ea4b74d0155fdf0e6fa Reviewed-on: https://go-review.googlesource.com/18282 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Andrew Gerrand <adg@golang.org>
2015-12-16net/http: fix Transport race returning bodyless responses and reusing connsBrad Fitzpatrick
The Transport had a delicate protocol between its readLoop goroutine and the goroutine calling RoundTrip. The basic concern is that the caller's RoundTrip goroutine wants to wait for either a connection-level error (the conn dying) or the response. But sometimes both happen: there's a valid response (without a body), but the conn is also going away. Both goroutines' logic dealing with this had grown large and complicated with hard-to-follow comments over the years. Simplify and document. Pull some bits into functions and do all bodyless stuff in one place (it's special enough), rather than having a bunch of conditionals scattered everywhere. One test is no longer even applicable since the race it tested is no longer possible (the code doesn't exist). The bug that this fixes is that when the Transport reads a bodyless response from a server, it was returning that response before returning the persistent connection to the idle pool. As a result, ~1/1000 of serial requests would end up creating a new connection rather than re-using the just-used connection due to goroutine scheduling chance. Instead, this now adds bodyless responses' connections back to the idle pool first, then sends the response to the RoundTrip goroutine, but making sure that the RoundTrip goroutine is outside of its select on the connection dying. There's a new buffered channel involved now, which is a minor complication, but it's much more self-contained and well-documented than the previous complexity. (The alternative of making the responseAndError channel itself unbuffered is too invasive and risky at this point; it would require a number of changes to avoid deadlocked goroutines in error cases) In any case, flakes look to be gone now. We'll see if trybots agree. Fixes #13633 Change-Id: I95a22942b2aa334ae7c87331fddd751d4cdfdffc Reviewed-on: https://go-review.googlesource.com/17890 Reviewed-by: Russ Cox <rsc@golang.org> Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
2015-12-14net/http: fix race in TimeoutHandlerBrad Fitzpatrick
New implementation of TimeoutHandler: buffer everything to memory. All or nothing: either the handler finishes completely within the timeout (in which case the wrapper writes it all), or it misses the timeout and none of it gets written, in which case handler wrapper can reliably print the error response without fear that some of the wrapped Handler's code already wrote to the output. Now the goroutine running the wrapped Handler has its own write buffer and Header copy. Document the limitations. Fixes #9162 Change-Id: Ia058c1d62cefd11843e7a2fc1ae1609d75de2441 Reviewed-on: https://go-review.googlesource.com/17752 Reviewed-by: David Symonds <dsymonds@golang.org> Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
2015-12-03net/http: deflake a non-short test, clean up export_test.goBrad Fitzpatrick
This makes TestTransportResponseCloseRace much faster and no longer flaky. In the process it also cleans up test hooks in net/http which were inconsistent and scattered. Change-Id: Ifd0b11dbc7e8915c24eb5bdc36731ed6751dd7ec Reviewed-on: https://go-review.googlesource.com/17316 Reviewed-by: Ian Lance Taylor <iant@golang.org> Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
2015-12-01net/http: retry idempotent HTTP reqs on dead reused connsBlake Gentry
If we try to reuse a connection that the server is in the process of closing, we may end up successfully writing out our request (or a portion of our request) only to find a connection error when we try to read from (or finish writing to) the socket. This manifests as an EOF returned from the Transport's RoundTrip. The issue, among others, is described in #4677. This change follows some of the Chromium guidelines for retrying idempotent requests only when the connection has been already been used successfully and no header data has yet been received for the response. As part of this change, an unexported error was defined for errMissingHost, which was previously defined inline. errMissingHost is the only non-network error returned from a Request's Write() method. Additionally, this breaks TestLinuxSendfile because its test server explicitly triggers the type of scenario this change is meant to retry on. Because that test server stops accepting conns on the test listener before the retry, the test would time out. To fix this, the test was altered to use a non-idempotent test type (POST). Change-Id: I1ca630b944f0ed7ec1d3d46056a50fb959481a16 Reviewed-on: https://go-review.googlesource.com/3210 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>