aboutsummaryrefslogtreecommitdiff
path: root/src/net/http/transport_test.go
AgeCommit message (Collapse)Author
2021-08-04[release-branch.go1.15] net/http: speed up and deflake ↵Damien Neil
TestCancelRequestWhenSharingConnection 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. Fixes #47534. 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> (cherry picked from commit 6e738868a7a943d7d4fd6bb1963e7f6d78111726) Reviewed-on: https://go-review.googlesource.com/c/go/+/339829
2021-06-02[release-branch.go1.15] net/http: prevent infinite wait during ↵Michael Fraenkel
TestMissingStatusNoPanic 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> (cherry picked from commit c0a7ecfae775a9d50d338e8123fac32a5d04308c) Reviewed-on: https://go-review.googlesource.com/c/go/+/320189 Reviewed-by: Damien Neil <dneil@google.com> Run-TryBot: Bryan C. Mills <bcmills@google.com> Run-TryBot: Damien Neil <dneil@google.com> TryBot-Result: Go Bot <gobot@golang.org>
2021-03-02[release-branch.go1.15] net/http: ignore connection closes once done with ↵Michael Fraenkel
the connection Once the connection is put back into the idle pool, the request should not take any action if the connection is closed. For #42935. Updates #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> (cherry picked from commit 212d385a2f723a8dd5e7d2e83efb478ddd139349) Reviewed-on: https://go-review.googlesource.com/c/go/+/297909 Trust: Dmitri Shuralyov <dmitshur@golang.org> Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Damien Neil <dneil@google.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>
2019-04-16net/http: rename DialerAndTLSConfigSupportsHTTP2 to ForceAttemptHTTP2Kunpei Sakai
Transport.DialerAndTLSConfigSupportsHTTP2 was added just earlier in CL 130256 but we thought of a better name moments after submitting. ForceAttemptHTTP2 is shorter, more direct, and doesn't constrain what we can use it with in the future. Updates #14391 Updates #27011 Change-Id: Ie5fc71bafcbcaa1941b5d49f748b6d710503d477 Reviewed-on: https://go-review.googlesource.com/c/go/+/172299 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2019-04-16net/http: introduce DialerAndTLSConfigSupportsHTTP2 in TransportKunpei Sakai
Even when a custom TLS config or custom dialer is specified, enables HTTP/2 if DialerAndTLSConfigSupportsHTTP2 is true. By this change, avoid automatically enabling HTTP/2 if DialContext is set. This change also ensures that DefaultTransport still automatically enable HTTP/2 as discussed in #14391. Updates #14391 Fixes #27011 Change-Id: Icc46416810bee61dbd65ebc96468335030b80573 Reviewed-on: https://go-review.googlesource.com/c/go/+/130256 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> Run-TryBot: Kunpei Sakai <namusyaka@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-03-11net/http: add missing error checks in testsLeon Klingele
Change-Id: I73441ba2eb349f0e0f25068e6b24c74dd33f1456 GitHub-Last-Rev: b9e6705962b94af3b1b720cc9ad6d33d7d3f1425 GitHub-Pull-Request: golang/go#30017 Reviewed-on: https://go-review.googlesource.com/c/go/+/160441 Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com> Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-03-11all: move internal/x to vendor/golang.org/x and revendor using 'go mod vendor'Bryan C. Mills
This also updates the vendored-in versions of several packages: 'go mod vendor' selects a consistent version of each module, but we had previously vendored an ad-hoc selection of packages. Notably, x/crypto/hkdf was previously vendored in at a much newer commit than the rest of x/crypto. Bringing the rest of x/crypto up to that commit introduced an import of golang.org/x/sys/cpu, which broke the js/wasm build, requiring an upgrade of x/sys to pick up CL 165749. Updates #30228 Updates #30241 Updates #25822 Change-Id: I5b3dbc232b7e6a048a158cbd8d36137af1efb711 Reviewed-on: https://go-review.googlesource.com/c/go/+/164623 Reviewed-by: Filippo Valsorda <filippo@golang.org>
2019-03-07net/http: unfurl persistConnWriter's underlying writerChris Marchesi
Make persistConnWriter implement io.ReaderFrom, via an io.Copy on the underlying net.Conn. This in turn enables it to use OS level optimizations such as sendfile. This has been observed giving performance gains even in the absence of ReaderFrom, more than likely due to the difference in io's default buffer (32 KB) versus bufio's (4 KB). Speedups on linux/amd64: benchmark old MB/s new MB/s speedup BenchmarkFileAndServer_16MB/NoTLS-4 662.96 2703.74 4.08x BenchmarkFileAndServer_16MB/TLS-4 552.76 1420.72 2.57x Speedups on darwin/amd64: benchmark old MB/s new MB/s speedup BenchmarkFileAndServer_16MB/NoTLS-8 357.58 1972.86 5.52x BenchmarkFileAndServer_16MB/TLS-8 346.20 1067.41 3.08x Updates #30377. Change-Id: Ic88d4ac254f665223536fcba4d551fc32ae105b6 GitHub-Last-Rev: a6f67cda2ed63ac61a1dffc87f0ea396363f72c6 GitHub-Pull-Request: golang/go#30390 Reviewed-on: https://go-review.googlesource.com/c/go/+/163737 Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2018-11-29vendor/golang_org/x: move to internal/xBryan C. Mills
Packages in vendor/ directories have a "vendor/" path prefix in GOPATH mode, but intentionally do not in module mode. Since the import path is embedded in the compiled output, changing that path invalidates cache entries and causes cmd/go to try to rebuild (and reinstall) the vendored libraries, which will fail if the directory containing those libraries is read-only. If I understood correctly, this is the approach Russ suggested as an alternative to https://golang.org/cl/136138. Fixes #27285 Fixes #26988 Change-Id: I8a2507fa892b84cde0a803aaa79e460723da572b Reviewed-on: https://go-review.googlesource.com/c/147443 Run-TryBot: Bryan C. Mills <bcmills@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Russ Cox <rsc@golang.org>
2018-11-28net/http: prevent transport sends two "Connection: close" headersMeng Zhuo
There are three functions that do Connection header write: 1. transport.go/ persistConn.roundTrip 2. transfer.go/ transferWriter.writeHeader 3. request.go/ Request.write The root cause is roundTrip didn't lookup into request.Close and transferWriter didn't take care of extraHeaders. Fixes #28886 Change-Id: I1d131019c7cd42eb1bcc972c631b7df7511c1f39 Reviewed-on: https://go-review.googlesource.com/c/150722 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@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-25net: make cgo resolver work more accurately with network parameterEugene Kalinin
Unlike the go resolver, the existing cgo resolver exchanges both DNS A and AAAA RR queries unconditionally and causes unreasonable connection setup latencies to applications using the cgo resolver. This change adds new argument (`network`) in all functions through the series of calls: from Resolver.internetAddrList to cgoLookupIPCNAME. Benefit: no redundant DNS calls if certain IP version is used IPv4/IPv6 (no `AAAA` DNS requests if used tcp4, udp4, ip4 network. And vice versa: no `A` DNS requests if used tcp6, udp6, ip6 network) Fixes #25947 Change-Id: I39edbd726d82d6133fdada4d06cd90d401e7e669 Reviewed-on: https://go-review.googlesource.com/c/120215 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2018-10-12net/http: flesh out Transport's HTTP/1 CONNECT+bidi support to match HTTP/2Brad Fitzpatrick
Fixes #17227 Change-Id: I0f8964593d69623b85d5759f6276063ee62b2915 Reviewed-on: https://go-review.googlesource.com/c/123156 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2018-10-06all: fix a bunch of misspellingsIgor Zhilianin
Change-Id: If2954bdfc551515403706b2cd0dde94e45936e08 GitHub-Last-Rev: d4cfc41a5504cf10befefdb881d4c45986a1d1f8 GitHub-Pull-Request: golang/go#28049 Reviewed-on: https://go-review.googlesource.com/c/140299 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>
2018-09-28all: remove repeated "the" from commentsIan Davis
A simple grep over the codebase for "the the" which is often missed by humans. Change-Id: Ie4b4f07abfc24c73dcd51c8ef1edf4f73514a21c Reviewed-on: https://go-review.googlesource.com/138335 Reviewed-by: Dave Cheney <dave@cheney.net>
2018-08-25net/http: make Transport return Writable Response.Body on protocol switchBrad Fitzpatrick
Updates #26937 Updates #17227 Change-Id: I79865938b05c219e1947822e60e4f52bb2604b70 Reviewed-on: https://go-review.googlesource.com/131279 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2018-08-20all: update stale test skipsDaniel Martí
Issues #10043, #15405, and #22660 appear to have been fixed, and whatever tests I could run locally do succeed, so remove the skips. Issue #7237 was closed in favor of #17906, so update its skip line. Issue #7634 was closed as it had not appeared for over three years. Re-enable it for now. An issue should be open if the test starts being skipped again. Change-Id: I67daade906744ed49223291035baddaad9f56dca Reviewed-on: https://go-review.googlesource.com/121735 Run-TryBot: Daniel Martí <mvdan@mvdan.cc> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
7apu2bq3rhoylwmucxizk54afw5jyda7pho4j5zdcqpwkufke7zdzwad.onion