Commit Graph

1119 Commits

Author SHA1 Message Date
Russ Cox ea1437a8cd net/http: fix handling of HTTP/2 upgrade failures
If an error occurs during the HTTP/2 upgrade phase, originally this
resulted in a pconn with pconn.alt set to an http2erringRoundTripper,
which always fails. This is not wanted - we want to retry in this case.

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

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

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

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

Fixes #34978.
Fixes #35113.

Change-Id: Ibd908b63c2ae96e159e8e604213d8373afb350e3
Reviewed-on: https://go-review.googlesource.com/c/go/+/220905
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-03-03 16:04:34 +00:00
Bryan C. Mills 12d02e7d8e net/http: verify RoundTripper invariants in the send function
Issue #37598 reports a nil-panic in *Client.send that can
only occur if one of the RoundTripper invariants is violated.
Unfortunately, that condition is currently difficult to diagnose: it
manifests as a panic during a Response field access, rather than
something the user can easily associate with an specific erroneous
RoundTripper implementation.

No test because the new code paths are supposed to be unreachable.

Updates #37598

Change-Id: If0451e9c6431f6fab7137de43727297a80def05b
Reviewed-on: https://go-review.googlesource.com/c/go/+/221818
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2020-03-02 15:39:23 +00:00
Ziheng Liu 42f8199290 all: fix incorrect channel and API usage in some unit tests
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-02-27 19:04:17 +00:00
Hana (Hyang-Ah) Kim f5ff00583f net/http/pprof: set content type for /debug/pprof
Fixes #37311

Change-Id: I9e1f37e991e5c203fe72061692f47584fbadfc58
Reviewed-on: https://go-review.googlesource.com/c/go/+/220324
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2020-02-21 19:28:40 +00:00
Brad Fitzpatrick 8390c47860 net/http/cgi: rename a test file to be less cute
My fault (from CL 4245070), sorry.

Change-Id: Ib95d3170dc326e74aa74c22421c4e44a8b00f577
Reviewed-on: https://go-review.googlesource.com/c/go/+/214920
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2020-01-31 23:17:00 +00:00
Ian Lance Taylor c436eadbc3 net/http: don't treat an alternate protocol as a known round tripper
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-29 04:04:52 +00:00
Filippo Valsorda f9fe783c6c Revert "net/http: support gzip, x-gzip Transfer-Encodings"
This reverts commit e6c12c3d02.

Reason for revert: the assumption that a T-E of "gzip" implies
"chunked" seems incorrect. The RFC does state that one "MUST apply
chunked as the final transfer coding" but that should be interpreted to
mean that a "chunked" encoding must be listed as the last one, not that
one should be assumed to be there if not. This is confirmed by the
alternative option to chunking on the server side being to "terminate
the message by closing the connection".

The issue seems confirmed by the fact that the code in the body of
#29162 fails with the following error:

    net/http: HTTP/1.x transport connection broken: http: failed to gunzip body: unexpected EOF

This late in the cycle, revert rather than fix, also because we don't
apparently have tests for the correct behavior.

Change-Id: I920ec928754cd8e96a06fb7ff8a53316c0f959e5
Reviewed-on: https://go-review.googlesource.com/c/go/+/215757
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2020-01-22 22:06:30 +00:00
Kévin Dunglas 88ed322afc net/http/httputil: add docs about X-Forwarded-For in ReverseProxy
ReverseProxy automatically sets the X-Forwarded-For header, if the request
already contains a X-Forwarded-For header, the value of the client IP is
appended to the existing header value.
This behavior isn't documented anywhere, and can lead to IP spoofing
security issues is the client is untrusted (the most common situation).
This PR documents this behavior.

For future versions, I proposed #36678 that implements a more secure
default behavior and adds support for other forwarded headers.

Change-Id: Ief14f5063caebfccb87714f54cffa927c714e5fd
GitHub-Last-Rev: fd0bd29a18
GitHub-Pull-Request: golang/go#36672
Reviewed-on: https://go-review.googlesource.com/c/go/+/215617
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2020-01-21 21:51:46 +00:00
Luka Zitnik 77c13021dd net/http: document that ParseForm consumes Request.Body
Fixes #35620

Change-Id: I71bc56ec7a7507d14b4f013177b4b816bb1a2094
Reviewed-on: https://go-review.googlesource.com/c/go/+/212458
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2020-01-08 15:48:04 +00:00
Bryan C. Mills 249c85d3aa net/http: avoid writing to Transport.ProxyConnectHeader
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>
2020-01-07 18:00:57 +00:00
Brad Fitzpatrick 100bf440b9 net/http: fix some awkward wording in Request.WithContext docs
Change-Id: Ib37db42c7f1fd6aa55f70fd2d65d56bb2ae6d26a
Reviewed-on: https://go-review.googlesource.com/c/go/+/211098
Reviewed-by: Bryan C. Mills <bcmills@google.com>
2019-12-12 17:47:19 +00:00
Bryan C. Mills ef3ef8fcdf net/http: use cancellation instead of a timeout in 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-11 15:43:50 +00:00
Bryan C. Mills 931fe39400 net/http: await state traces earlier in TestServerConnState
This approach attempts to ensure that the log for each connection is
complete before the next sequence of states begins.

Updates #32329

Change-Id: I25150d3ceab6568af56a40d2b14b5f544dc87f61
Reviewed-on: https://go-review.googlesource.com/c/go/+/210717
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2019-12-10 18:44:22 +00:00
Brad Fitzpatrick ecde0bfa1f net/http: fix timeout race in Transport proxy CONNECT
Fixes #36070

Change-Id: I99742aa153202436d802634c9e019a14b9ef9185
Reviewed-on: https://go-review.googlesource.com/c/go/+/210738
Reviewed-by: Bryan C. Mills <bcmills@google.com>
2019-12-10 18:30:47 +00:00
Brad Fitzpatrick 590052ba29 net/http: don't wait indefinitely in Transport for proxy CONNECT response
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-12-09 15:46:57 +00:00
Emmanuel T Odeke bff4ebb7aa net/http: unflake TestTimeoutHandlerSuperfluousLogs
Uses 2 channels to synchronize that test, because
relying on sleeps creates flaky behavior, thus:

a) 1 buffered channel to send back the last spurious line
without having to reason about "happens before" behavior
a) 1 buffered channel at the end of the handler; it'll
be controlled by whether we expect to timeout or not,
but will always be closed when the test ends

Fixes #35051

Change-Id: Iff735aa8d1ed9de8d92b792374ec161cc0a72798
Reviewed-on: https://go-review.googlesource.com/c/go/+/208477
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
2019-12-07 00:46:38 +00:00
Brad Fitzpatrick 22688f740d net/http: update bundled x/net/http2
Updates bundled http2 to x/net git rev ef20fe5d7 for:

   http2: make Transport.IdleConnTimeout consider wall (not monotonic) time
   https://golang.org/cl/208798 (#29308)

   http2: make CipherSuites validation error more verbose
   https://golang.org/cl/200317 (#34776)

   http2: track unread bytes when the pipe is broken
   https://golang.org/cl/187377 (#28634)

   http2: split cookie pair into separate hpack header fields
   https://golang.org/cl/155657 (#29386)

Fixes #29308
Fixes #28634

Change-Id: I71a03ca62ccb5ff35a5cfadd8dc705a4491ae7ea
Reviewed-on: https://go-review.googlesource.com/c/go/+/209077
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2019-11-27 04:05:12 +00:00
Dmitri Shuralyov 6ba1bf393a net/http: rename tests for Redirect and StripPrefix
Before May 2018, I mistakenly thought the _suffix naming convention¹
used by examples also applied to tests. Thanks to a code review comment²
from Ian Lance Taylor, I have since learned that is not true.
This trivial change fixes some collateral damage from my earlier
misunderstanding, resulting in improved test naming consistency.

¹ https://golang.org/pkg/testing/#hdr-Examples
² https://go-review.googlesource.com/c/go/+/112935/1/src/path/filepath/path_test.go#1075

Change-Id: I555f60719629eb64bf2f096aa3dd5e00851827cd
Reviewed-on: https://go-review.googlesource.com/c/go/+/207446
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2019-11-22 23:23:13 +00:00
Brad Fitzpatrick 980f0c00bb net/http: make Transport.IdleConnTimeout consider wall (not monotonic) time
Both laptops closing their lids and cloud container runtimes
suspending VMs both faced the problem where an idle HTTP connection
used by the Transport could be cached for later reuse before the
machine is frozen, only to wake up many minutes later to think that
their HTTP connection was still good (because only a second or two of
monotonic time passed), only to find out that the peer hung up on them
when they went to write.

HTTP/1 connection reuse is inherently racy like this, but no need for
us to step into a trap if we can avoid it. Also, not everybody sets
Request.GetBody to enable re-tryable POSTs. And we can only safely
retry requests in some cases.

So with this CL, before reusing an old connection, double check the walltime.

Testing was done both with a laptop (closing the lid for a bit) and
with QEMU, running "stop" and "cont" commands in the monitor and
sending QMP guest agent commands to update its wall clock after the
"cont":

echo '{"execute":"guest-set-time"}' | socat STDIN UNIX-CONNECT:/var/run/qemu-server/108.qga

In both cases, I was running
https://gist.github.com/bradfitz/260851776f08e4bc4dacedd82afa7aea and
watching that the RemoteAddr changed after resume.

It's kinda difficult to write an automated test for. I gave a lightning talk on
using pure emulation user mode qemu for such tests:

   https://www.youtube.com/watch?v=69Zy77O-BUM
   https://docs.google.com/presentation/d/1rAAyOTCsB8GLbMgI0CAbn69r6EVWL8j3DPl4qc0sSlc/edit?usp=sharing
   https://github.com/google/embiggen-disk/blob/master/integration_test.go

... that would probably be a good direction if we want an automated
test here. But I don't have time to do that now.

Updates #29308 (HTTP/2 remains)

Change-Id: I03997e00491f861629d67a0292da000bd94ed5ca
Reviewed-on: https://go-review.googlesource.com/c/go/+/204797
Reviewed-by: Bryan C. Mills <bcmills@google.com>
2019-11-22 22:24:38 +00:00
Roman Kollár bbbc6589df net/http: fix Server.ConnContext modifying context for all new connections
Fixes #35750

Change-Id: I65d38cfc5ddd66131777e104c269cc3559b2471d
GitHub-Last-Rev: 953fdfd49b
GitHub-Pull-Request: golang/go#35751
Reviewed-on: https://go-review.googlesource.com/c/go/+/208318
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-11-21 23:15:28 +00:00
Ville Skyttä 440f7d6404 all: fix a bunch of misspellings
Change-Id: I5b909df0fd048cd66c5a27fca1b06466d3bcaac7
GitHub-Last-Rev: 778c5d2131
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-15 21:04:43 +00:00
Brad Fitzpatrick 54cf776020 net/http: add some debugging to TestDontCacheBrokenHTTP2Conn
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-13 14:12:20 +00:00
Gabriel Rosenhouse eb55a0c864 net/http: add DialTLSContext hook to Transport
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-11 20:17:03 +00:00
Brad Fitzpatrick 7a5e0fec09 net/http: relax TestLinuxSendfile, trace all system calls, match n64:sendfile
Fixes #35423

Change-Id: Idb254d6a2c4b147d20e290411e4380df5cdcb306
Reviewed-on: https://go-review.googlesource.com/c/go/+/206178
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2019-11-08 20:05:25 +00:00
Emmanuel T Odeke e6c12c3d02 net/http: support gzip, x-gzip Transfer-Encodings
Support "gzip" aka "x-gzip" as a transfer-encoding for
requests and responses as per RFC 7230 Section 3.3.1.

"gzip" and "x-gzip" are equivalents as requested by
RFC 7230 Section 4.2.3.

Transfer-Encoding is an on-fly property of the body
that can be applied by proxies, other servers and basically
any intermediary to transport the content e.g. across data centers
or backends/machine to machine that need compression.

For this change, "gzip" is both explicitly and implicitly combined
with transfer-encoding "chunked" in an ordering such as:

    Transfer-Encoding: gzip, chunked

and NOT

    Transfer-Encoding: chunked, gzip

Obviously the latter form is counter-intuitive for streaming.
Thus "chunked" is the last value to appear in that transfer-encoding header,
if explicitly included.

When parsing the response, the chunked body is concatenated as "chunked" does,
before finally being decompressed as "gzip".

A chunked and compressed body would typically look like this:

<LENGTH_1>\r\n<CHUNK_1_GZIPPED_BODY>\r\n<LENGTH_2>\r\n<CHUNK_2_GZIPPED_BODY>\0\r\n

which when being processed we would contentate

    <FULL_BODY>  := <CHUNK_1_GZIPPED_BODY> + <CHUNK_2_GZIPPED_BODY> + ...

and then finally gunzip it
    <FINAL_BODY> := gunzip(<FULL_BODY>)

If a "chunked" transfer-encoding is NOT applied but "gzip" is applied,
we implicitly assume that they requested using "chunked" at the end.
This is as per the recommendation of RFC 3.3.1. which explicitly says
that for:

* Request:
"  If any transfer coding
   other than chunked is applied to a request payload body, the sender
   MUST apply chunked as the final transfer coding to ensure that the
   message is properly framed."

* Response:
"  If any transfer coding other than
   chunked is applied to a response payload body, the sender MUST either
   apply chunked as the final transfer coding or terminate the message
   by closing the connection."

RELNOTE=yes

Fixes #29162

Change-Id: Icb8b8b838cf4119705605b29725cabb1fe258491
Reviewed-on: https://go-review.googlesource.com/c/go/+/166517
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2019-11-08 19:24:30 +00:00
David Ndungu 1fd3f8bd67 net/http: refactor test TestParseFormUnknownContentType
Use names to better communicate when a test case fails.

Change-Id: Id882783cb5e444b705443fbcdf612713f8a3b032
Reviewed-on: https://go-review.googlesource.com/c/go/+/187823
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2019-11-08 18:11:01 +00:00
Brad Fitzpatrick 2566e21f24 net/http: support disabling built-in HTTP/2 with a new build tag
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-04 23:16:09 +00:00
Richard Musiol 54e6ba6724 syscall/js: garbage collect references to JavaScript values
The js.Value struct now contains a pointer, so a finalizer can
determine if the value is not referenced by Go any more.

Unfortunately this breaks Go's == operator with js.Value. This change
adds a new Equal method to check for the equality of two Values.
This is a breaking change. The == operator is now disallowed to
not silently break code.

Additionally the helper methods IsUndefined, IsNull and IsNaN got added.

Fixes #35111

Change-Id: I58a50ca18f477bf51a259c668a8ba15bfa76c955
Reviewed-on: https://go-review.googlesource.com/c/go/+/203600
Run-TryBot: Richard Musiol <neelance@gmail.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-11-04 22:50:43 +00:00
Harshavardhana bdb5e9d170 net/http/httputil: fix missing Transfer-Encoding header
Current implementation of httputil.DumpRequestOut
incorrectly resets the Request.Body prematurely
before Content-Length/Transfer-Encoding detection
in newTransferWriter()

This fix avoids resetting the Request.Body when
Request.ContentLength is set to '0' by the caller
and Request.Body is set to a custom reader. To allow
newTransferWriter() to treat this situation as
'Transfer-Encoding: chunked'.

Fixes #34504

Change-Id: Ieab6bf876ced28c32c084e0f4c8c4432964181f5
Reviewed-on: https://go-review.googlesource.com/c/go/+/197898
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-11-02 19:53:12 +00:00
Constantin Konstantinidis 1e4a358454 net/http: deflake TestCancelRequestWithChannelBeforeDo_Cancel
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-11-02 03:00:20 +00:00
empijei a570fcf74b net/http: clarify Header.WriteSubset behavior
The current implementation performs a plain map lookup,
but other header methods canonicalize header keys before
using them.

Fixes #34918

Change-Id: Id4120488b8b39ecee97fa7a6ad8a34158687ffcd
Reviewed-on: https://go-review.googlesource.com/c/go/+/201357
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2019-11-01 14:48:28 +00:00
Michael Fraenkel 1a058cd04b net/http: only decrement connection count if we removed a connection
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-29 16:45:39 +00:00
pokutuna 46e0d724b3 net/http: fix comment TimeoutHandler no longer supports Flusher
Fixes #35161
Updates #34439

Change-Id: I978534cbb8b9fb32c115dba0066cf099c61d8ee9
GitHub-Last-Rev: d60581635e
GitHub-Pull-Request: golang/go#35162
Reviewed-on: https://go-review.googlesource.com/c/go/+/203478
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-10-25 20:46:57 +00:00
Bryan C. Mills 8bb47a5eec net/http: skip failing test on windows-amd64-longtest builder
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-25 18:45:21 +00:00
Brad Fitzpatrick 88186e5e23 net/http: don't cache http2.erringRoundTripper connections
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-22 19:46:38 +00:00
Brave Cow 07ccdeb192 net/http: update sanitizeCookieValue description
Commit 8f6d68eb (CL 37328) changed the code of sanitizeCookieValue without updating its description.

Change-Id: Ib4a1a1f316548258b828458a31b09706bbd59b53
GitHub-Last-Rev: 9731494e17
GitHub-Pull-Request: golang/go#34790
Reviewed-on: https://go-review.googlesource.com/c/go/+/200041
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2019-10-22 15:03:09 +00:00
Dmitri Shuralyov 3409ce39bf net/http: remove parseURL variable
The parseURL variable was introduced in CL 49930 in order to work
around the fact that the name "url" was shadowed by a parameter of
exported functions, and couldn't be renamed without sacrificing
documentation readability. Documentation readability takes higher
priority than internal implementation details.

Back then, I considered renaming the net/url import but saw that it
would be too disruptive of a change to the large net/http package.

Now I see a better way: it's possible to import net/url both as url
and as urlpkg (the package is still imported just once, but it becomes
available via two names). This way we eliminate the need for wasting
(a little) memory on the parseURL variable, improve code readability
slightly, and delete some lines of code and comments.

Updates #21077

Change-Id: I42cd9833afdcf4a5f5874fb7ee9c8c11eae557dc
Reviewed-on: https://go-review.googlesource.com/c/go/+/202482
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
2019-10-21 20:43:40 +00:00
harsimran1 3b2eb699a0 net/http/pprof: explicitly mention DefaultServeMux for default handlers
Change-Id: I224db88f3809001802e004077ce856f0e3347c67
Reviewed-on: https://go-review.googlesource.com/c/go/+/169017
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2019-10-21 18:43:12 +00:00
Emmanuel T Odeke ff4e0e42d8 net/http: make TimeoutHandler log spurious WriteHeader calls
Makes TimeoutHandler consistent with other handlers, by
logging any spurious WriteHeader calls.

Fixes #30803

Change-Id: I693fbdf8378f31bca13d579eece8e8e00eb175bf
Reviewed-on: https://go-review.googlesource.com/c/go/+/200518
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-10-21 18:38:15 +00:00
Emmanuel T Odeke ff9f7bc9da net/http: make Transport.RoundTrip close body on any invalid request
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-21 17:10:59 +00:00
Luke Young 39cfb7609d net/http: make Transport.roundTrip close body on invalid method
Updates #35015

Change-Id: Ibfe8f72ed3887ca88ce9c1d8a29dacda72f3fe17
GitHub-Last-Rev: 4bfc56e716
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-10-20 06:25:35 +00:00
Emmanuel T Odeke 5375c71289 net/http/httptest: add EnableHTTP2 to Server
Adds a knob EnableHTTP2, that enables an unstarted
Server and its respective client to speak HTTP/2,
but only after StartTLS has been invoked.

Fixes #34939

Change-Id: I287c568b8708a4d3c03e7d9eca7c323b8f4c65b6
Reviewed-on: https://go-review.googlesource.com/c/go/+/201557
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2019-10-18 19:29:10 +00:00
Trung Nguyen 3972f97d04 net/http, net/textproto: add Header.Values, MIMEHeader.Values methods
Fixes #34799

Change-Id: I134b2717fa90c8955902e7eeaaf8510dcc28340e
Reviewed-on: https://go-review.googlesource.com/c/go/+/200760
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2019-10-17 18:21:01 +00:00
Brad Fitzpatrick 6da300b196 net/http: remove references to old NPN support
We now only support ALPN.

Updates #28362

Change-Id: I8d9461c7a91315ee92e712448d0bf5c4070d09ae
Reviewed-on: https://go-review.googlesource.com/c/go/+/201202
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-10-15 18:20:42 +00:00
Anmol Sethi 9f95a30977 net/http: clean up checkIfModifiedSince and checkIfUnmodifiedSince
The comment in both functions referred to the wrong header and I made
the checks easier to read.

Change-Id: Ifb46729cee631a3305f557863818e3487b8eed71
Reviewed-on: https://go-review.googlesource.com/c/go/+/71753
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2019-10-14 21:10:00 +00:00
Emmanuel T Odeke 19e0799ba0 net/http: fix and lock-in Client.Do docs on request cancelation
Fixes the docs to correctly match the implementation
and also adds a test locking-in the behavior to prevent
any accidental future regressions on the docs.

Fixes #33545

Change-Id: I6fdac6048cce8ac99beaa2db0dfc81d0dccc10f1
Reviewed-on: https://go-review.googlesource.com/c/go/+/200798
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2019-10-14 20:50:54 +00:00
Emmanuel T Odeke 9969c72080 net/http: fix Transport panic with nil Request.Header
For Go 1.13 we introduced Header.Clone and it returns
nil if a nil Header is cloned. Unfortunately, though,
this exported Header.Clone nil behavior differed from
the old Go 1.12 and earlier internal header clone
behavior which always returned non-nil Headers.
This CL fixes the places where that distinction mattered.

Fixes #34878

Change-Id: Id19dea2272948c8dd10883b18ea7f7b8b33ea8eb
Reviewed-on: https://go-review.googlesource.com/c/go/+/200977
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2019-10-14 17:53:36 +00:00
Emmanuel T Odeke 829fae3b5e net/http: update bundled x/net/http2
Updates x/net/http2 to git rev d66e71096ffb9f08f36d9aefcae80ce319de6d68

    http2: end stream eagerly after sending the request body
    https://golang.org/cl/181157 (fixes #32254)

    all: fix typos
    https://golang.org/cl/193799

    http2: fix memory leak in random write scheduler
    https://golang.org/cl/198462 (fixes #33812)

    http2: do not sniff body if Content-Encoding is set
    https://golang.org/cl/199841 (updates #31753)

Also unskips tests from CL 199799.

Change-Id: I241c0b1cd18cad5041485be92809137a973e33bd
Reviewed-on: https://go-review.googlesource.com/c/go/+/200102
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-10-09 20:09:29 +00:00
Emmanuel T Odeke e24a628ab1 net/http: do not sniff response if Content-Encoding header is set
Fixes #31753

Change-Id: I32ec5906ef6714e19b094f67cb0f10a211a9c500
Reviewed-on: https://go-review.googlesource.com/c/go/+/199799
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2019-10-09 19:49:39 +00:00
Davor Kapsa decf9f6fef net/http: use err as error var in server.Serve
Change-Id: Icbf97d640fb26eed646f9e85c1f1c94b1469ca4f
Reviewed-on: https://go-review.googlesource.com/c/go/+/199778
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-10-08 19:00:51 +00:00
Brad Fitzpatrick 0ba0ea172d net/http: document that Request.Host includes HTTP/2 :authority
Fixes #34640

Change-Id: I4a6c9414fe369cd5e9915472331c4bd8a21d8b0e
Reviewed-on: https://go-review.googlesource.com/c/go/+/198457
Reviewed-by: Filippo Valsorda <filippo@golang.org>
2019-10-02 16:47:29 +00:00
Kyohei Kadota 148ec3e3bc net/http/cgi: skip tests if not functional perl
TestEnvOverride sets PATH to /wibble before executing a CGI.
So customized Perl that is starting with '#!/usr/bin/env bash' will fail
because /usr/bin/env can't lookup bash.

Fixes #27790

Change-Id: I25e433061a7ff9da8c86429e934418fc15f12f90
Reviewed-on: https://go-review.googlesource.com/c/go/+/196845
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-10-02 05:52:16 +00:00
Nuno Cruces a49067aab6 net/http: avoid sending unspecified time for directories
Change applies to sendFile.
This is already done for sendContent.

Change-Id: If43d9ab99e6e66a1363b08e0bdcceb57df1f855c
GitHub-Last-Rev: 1c47620a09
GitHub-Pull-Request: golang/go#34631
Reviewed-on: https://go-review.googlesource.com/c/go/+/198139
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-10-01 15:27:28 +00:00
Emmanuel T Odeke 55738850c4 net/http: remove TestTimeoutHandlerAndFlusher due to flakes
Removes TestTimeoutHandlerAndFlusher due to flakes on
one of the builders due to timing issues.

Perhaps later, we might need to bring it back when we've
figured out the timing issues.

Fixes #34573.

Change-Id: Ia88d4da31fb228296144dc31f9a4288167fb4a53
Reviewed-on: https://go-review.googlesource.com/c/go/+/197757
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-09-27 15:38:16 +00:00
Emmanuel T Odeke 4faf8a8dc4 net/http, doc/go1.13.html: revert TimeoutHandler.Flush
Also added a test to ensure that any interactions
between TimeoutHandler and Flusher result in the
correct status code and body, but also that we don't
get superfluous logs from stray writes as was seen
in the bug report.

Fixes #34439.

Change-Id: I4af62db256742326f9353f98a2fcb5f71d2a5fd9
Reviewed-on: https://go-review.googlesource.com/c/go/+/197659
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-09-26 21:12:34 +00:00
Filippo Valsorda 41b1f88efa net/textproto: don't normalize headers with spaces before the colon
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-26 16:43:06 +00:00
Brad Fitzpatrick 7fc2625ef1 net/http: propagate Client.Timeout down into Request's context deadline
Fixes #31657

Change-Id: I85e9595d3ea30d410f1f4b787925a6879a72bdf2
Reviewed-on: https://go-review.googlesource.com/c/go/+/175857
Reviewed-by: Benny Siegert <bsiegert@gmail.com>
Run-TryBot: Benny Siegert <bsiegert@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-09-25 04:18:18 +00:00
Michael Fraenkel eb4e5defb4 net/http: remove http2 connections when no longer cached
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-24 15:21:08 +00:00
Tom Thorogood 582d5194fa net/http: fix HTTP/2 idle pool tracing
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: 2b7d66a1ce
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-09-18 18:52:45 +00:00
Ainar Garipov 0efbd10157 all: fix typos
Use the following (suboptimal) script to obtain a list of possible
typos:

  #!/usr/bin/env sh

  set -x

  git ls-files |\
    grep -e '\.\(c\|cc\|go\)$' |\
    xargs -n 1\
    awk\
    '/\/\// { gsub(/.*\/\//, ""); print; } /\/\*/, /\*\// { gsub(/.*\/\*/, ""); gsub(/\*\/.*/, ""); }' |\
    hunspell -d en_US -l |\
    grep '^[[:upper:]]\{0,1\}[[:lower:]]\{1,\}$' |\
    grep -v -e '^.\{1,4\}$' -e '^.\{16,\}$' |\
    sort -f |\
    uniq -c |\
    awk '$1 == 1 { print $2; }'

Then, go through the results manually and fix the most obvious typos in
the non-vendored code.

Change-Id: I3cb5830a176850e1a0584b8a40b47bde7b260eae
Reviewed-on: https://go-review.googlesource.com/c/go/+/193848
Reviewed-by: Robert Griesemer <gri@golang.org>
2019-09-08 17:28:20 +00:00
smasher164 141b09726d net/http: make copyValues append instead of calling Add
This results in a performance boost:

name          old time/op    new time/op    delta
CopyValues-4    3.46µs ± 3%    1.53µs ± 3%  -55.85%  (p=0.000 n=18+19)

name          old alloc/op   new alloc/op   delta
CopyValues-4    1.52kB ± 0%    0.74kB ± 0%  -51.58%  (p=0.000 n=20+20)

name          old allocs/op  new allocs/op  delta
CopyValues-4      24.0 ± 0%      11.0 ± 0%  -54.17%  (p=0.000 n=20+20)

Fixes #33744.

Change-Id: Ibc653fb076a9a6aaa775fcc9ca720fb90e68cf96
Reviewed-on: https://go-review.googlesource.com/c/go/+/191057
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2019-09-07 22:21:30 +00:00
Daniel Martí 03ac39ce5e std: remove unused bits of code all over the place
Some were never used, and some haven't been used for years.

One exception is net/http's readerAndCloser, which was only used in a
test. Move it to a test file.

While at it, remove a check in regexp that could never fire; the field
is an uint32, so it can never be negative.

Change-Id: Ia2200f6afa106bae4034045ea8233b452f38747b
Reviewed-on: https://go-review.googlesource.com/c/go/+/192621
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2019-09-02 12:57:37 +00:00
spacewander 4a457e3edc net/http: merge IsDir checks in fs.go's serveFile function
Fixes #33385

Change-Id: I497ccd868d408a9c5648c72aa5ce41221368daf4
GitHub-Last-Rev: 3bf483808e
GitHub-Pull-Request: golang/go#33423
Reviewed-on: https://go-review.googlesource.com/c/go/+/188677
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-09-02 11:13:03 +00:00
Derek Phan 9c295bdeab net/http: remove redundant port validation
The URL port is already checked in net/url, so we can remove the redundant validation in net/http.

Fixes #33600

Change-Id: I62511a452df6262d4b66180933758d34627ff9df
GitHub-Last-Rev: c19afa3102
GitHub-Pull-Request: golang/go#33611
Reviewed-on: https://go-review.googlesource.com/c/go/+/190057
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2019-09-02 09:45:08 +00:00
Brad Fitzpatrick 7ed973b4d9 net/http: don't panic serving dir in ServeFile with empty Request.URL.Path
Updates #30165
Updates #31622

Change-Id: I7a4b91aa7c5c3af8c0b1273cbb42046feddf7d78
Reviewed-on: https://go-review.googlesource.com/c/go/+/180499
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-08-28 18:38:52 +00:00
Daniel Martí 81fde0c941 net/http: fix a long test after CL 185117
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-28 14:02:41 +00:00
Stefan Baebler 64cfe9fe22 net/url: improve url parsing error messages by quoting
Current implementation doesn't always make it obvious what the exact
problem with the URL is, so this makes it clearer by consistently quoting
the invalid URL, as is the norm in other parsing implementations, eg.:
strconv.Atoi(" 123") returns an error: parsing " 123": invalid syntax

Updates #29261

Change-Id: Icc6bff8b4a4584677c0f769992823e6e1e0d397d
GitHub-Last-Rev: 648b9d93fe
GitHub-Pull-Request: golang/go#29384
Reviewed-on: https://go-review.googlesource.com/c/go/+/185117
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-08-28 12:47:06 +00:00
Agniva De Sarker e7a4ab427d net/http/httputil: fix goroutine leak for DumpRequestOut
When an invalid URL was passed to DumpRequestOut, it would directly return
without gracefully shutting down the reader goroutine.

So we create a channel and signal the reader goroutine to exit
if an error occurs during roundtrip.

Fixes #32571

Change-Id: I8c2970f1601e599f3d1ebfed298faf5f5716fc2c
Reviewed-on: https://go-review.googlesource.com/c/go/+/182037
Run-TryBot: Agniva De Sarker <agniva.quicksilver@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2019-08-28 04:44:32 +00:00
Anderson Queiroz 2b59894425 net/http: enhance documentation for Server.Addr
Fixes golang/go#31249

Change-Id: I3280f8ab170ed31d4efb71106533e016d430d44c
Reviewed-on: https://go-review.googlesource.com/c/go/+/191557
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2019-08-27 20:52:49 +00:00
Bryan C. Mills 94bf9a8d4a net/http: fix wantConnQueue memory leaks in Transport
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-27 18:23:13 +00:00
Javier Revillas cd33d271b4 net/http: fix a typo in comments
HTTP is an initialism, not an acronym, where you pronounce each letter as a
word. It's "an H", not "a H".

Running `find src/net/http -type f | xargs grep -n 'an HTTP' | wc -l` shows
that the "an HTTP" form is used 67 times across the `net/http` package.
Furthermore, `find src/net/http -type f | xargs grep -n 'a HTTP' | wc -l`
yields only 4 results.

Change-Id: I219c292a9e2c9bf7a009dbfe82ea8b15874685e9
GitHub-Last-Rev: 6ebd095023
GitHub-Pull-Request: golang/go#33810
Reviewed-on: https://go-review.googlesource.com/c/go/+/191700
Reviewed-by: Toshihiro Shiino <shiino.toshihiro@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2019-08-27 16:50:35 +00:00
Kevin Gillette 8b03a3992b net/http: make docs refer to Context.Value as a getter instead of context.WithValue
The doc comments of both ServerContextKey and LocalAddrContextKey both suggest that context.WithValue can be used to access (get) properties of the server or connection. This PR fixes those comments to refer to Context.Value instead.

Change-Id: I4ed383ef97ba1951f90c555243007469cfc18d4d
GitHub-Last-Rev: 05bc3acf82
GitHub-Pull-Request: golang/go#33833
Reviewed-on: https://go-review.googlesource.com/c/go/+/191838
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2019-08-26 16:03:18 +00:00
Filippo Valsorda 145e193131 net/http: update bundled golang.org/x/net/http2 to import security fix
Update golang.org/x/net to v0.0.0-20190813141303-74dc4d7220e7 to import
the following security fix.

    commit 74dc4d7220e7acc4e100824340f3e66577424772
    Author: Filippo Valsorda <filippo@golang.org>
    Date:   Sun Aug 11 02:12:18 2019 -0400

    http2: limit number of control frames in server send queue

    An attacker could cause servers to queue an unlimited number of PING
    ACKs or RST_STREAM frames by soliciting them and not reading them, until
    the program runs out of memory.

    Limit control frames in the queue to a few thousands (matching the limit
    imposed by other vendors) by counting as they enter and exit the scheduler,
    so the protection will work with any WriteScheduler.

    Once the limit is exceeded, close the connection, as we have no way to
    communicate with the peer.

    Change-Id: I842968fc6ed3eac654b497ade8cea86f7267886b
    Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/525552
    Reviewed-by: Brad Fitzpatrick <bradfitz@google.com>

This change was generated with cmd/go and cmd/bundle:

$ go get -u golang.org/x/net
$ go mod tidy
$ go mod vendor
$ go generate net/http

Fixes CVE-2019-9512 and CVE-2019-9514
Fixes #33606

Change-Id: I464baf96175006aa101d65d3b0f6494f28a626ab
Reviewed-on: https://go-review.googlesource.com/c/go/+/190137
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
2019-08-23 17:03:17 +00:00
Jeff Hodges f3e3b71a50 net/http: change TimeoutHandler's docs to match its new interfaces
As of Go 1.13rc1, TimeoutHandler supports the Flusher and Pusher interfaces and
this change corrects its documentation to say that.

Fixes #33769
Updates #29193

Change-Id: Ia0523f7f2e3dc1f8f0b68950b85a7bf81c4abe60
GitHub-Last-Rev: 5310d2c960
GitHub-Pull-Request: golang/go#33770
Reviewed-on: https://go-review.googlesource.com/c/go/+/191237
Reviewed-by: Andrew Bonventre <andybons@golang.org>
Run-TryBot: Andrew Bonventre <andybons@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-08-22 17:04:05 +00:00
Filippo Valsorda 61bb56ad63 net/url: make Hostname and Port predictable for invalid Host values
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-08-12 23:12:29 +00:00
Filippo Valsorda 45504066d7 src/go.mod: sync golang.org/x/net with h2_bundle.go
The bundle included changes from a commit after the one referred to by
the go.mod, probably due to cmd/bundle using the GOPATH source.

Identified with the new go/packages based cmd/bundle from CL 189818.

$ go get golang.org/x/net@461777fb6f
$ go mod tidy
$ go mod vendor
$ go generate net/http # with CL 189818

Also, updated the socks_bundle.go generate command to drop obsolete
options and match h2_bundle.go. It caused no output changes.

Updates #32031

Change-Id: I0322d4e842dbfdad749455111072ca4872a62ad4
Reviewed-on: https://go-review.googlesource.com/c/go/+/189897
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
2019-08-12 20:14:38 +00:00
Shivani Singhal 09c9d3a58a net/http: fix typo in Response.Body's doc
Fixes #33453.

Change-Id: If0aa48f953633690884ddcff033b3be6d1d66443
Reviewed-on: https://go-review.googlesource.com/c/go/+/188541
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2019-08-05 05:44:06 +00:00
Damien Neil 0e54d28ff7 all: remove os.ErrTimeout
It is unclear whether the current definition of os.IsTimeout is
desirable or not. Drop ErrTimeout for now so we can consider adding it
(or some other error) in a future release with a corrected definition.

Fixes #33411

Change-Id: I8b880da7d22afc343a08339eb5f0efd1075ecafe
Reviewed-on: https://go-review.googlesource.com/c/go/+/188758
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Damien Neil <dneil@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-08-02 17:57:18 +00:00
Darren McCleary e7c51cad71 net/http: fix a typo in spelling of MultiPartForm
Change-Id: I33a5313ef10e8c88d9c12507573b385fa0843afe
GitHub-Last-Rev: 844d435158
GitHub-Pull-Request: golang/go#33412
Reviewed-on: https://go-review.googlesource.com/c/go/+/188498
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2019-08-01 20:35:26 +00:00
Damien Neil ea8b0acdac all: remove os.ErrTemporary
As discussed in
https://github.com/golang/go/issues/32463#issuecomment-506833421
the classification of deadline-based timeouts as "temporary" errors is a
historical accident. I/O timeouts used to be duration-based, so they
really were temporary--retrying a timed-out operation could succeed. Now
that they're deadline-based, timeouts aren't temporary unless you reset
the deadline.

Drop ErrTemporary from Go 1.13, since its definition is wrong. We'll
consider putting it back in Go 1.14 with a clear definition and
deprecate net.OpError.Temporary.

Fixes #32463

Change-Id: I70cda664590d8872541e17409a5780da76920891
Reviewed-on: https://go-review.googlesource.com/c/go/+/188398
Reviewed-by: Jonathan Amsterdam <jba@google.com>
2019-08-01 15:39:45 +00:00
Andrew Bonventre 8dddf7556e net/http: return nil from Header.Clone if the receiver is nil
Fixes #33141

Change-Id: I84a8b3496fc9396fd1c09ba9505697c34bdf7105
Reviewed-on: https://go-review.googlesource.com/c/go/+/188022
Run-TryBot: Andrew Bonventre <andybons@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
2019-07-31 16:15:56 +00:00
David Ndungu 1ad64fafc8 net/http: improve Request.Form and Request.PostForm documentation
Request.PostForm gets populated with form data for PATCH, POST, or PUT
http verbs.

Change-Id: I33065aa78a8470c4e9490aac830aa6f5963c61cb
Reviewed-on: https://go-review.googlesource.com/c/go/+/187821
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2019-07-30 20:49:54 +00:00
Joe Tsai f93234ad62 net/http/httputil: fix regression in ReverseProxy.ServeHTTP
In Go1.12 and below, the logic in ReverseProxy.ServeHTTP would always
allocate request.Header even if it were not present in the incoming request.
CL 174324 added http.Request.Clone and re-factors ReverseProxy.ServeHTTP
to use the new Clone method. However, the new Clone logic is not equivalent
to the former logic. We preserve former semantics by explicitly allocating
the Header map if nil.

Fixes #33142

Change-Id: I356f94a915dd9779584ce3fe31e56e5474b9ad37
Reviewed-on: https://go-review.googlesource.com/c/go/+/186437
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Bonventre <andybons@golang.org>
2019-07-17 21:52:45 +00:00
Russ Cox fbaf881cc6 net/http: fix Transport.MaxConnsPerHost limits & idle pool races
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-07-08 14:31:42 +00:00
Patrik Lundin 04e2e81ecb net/http: stop ExampleServer_Shutdown from hanging on error
Running the example code when not having permissions
to bind to port 80 will cause the program to hang after
printing the error message.

Change-Id: I2433ba2629b362fc8f1731e40cab5eea72ec354f
GitHub-Last-Rev: 0bb3dc08b6
GitHub-Pull-Request: golang/go#32947
Reviewed-on: https://go-review.googlesource.com/c/go/+/185157
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-07-05 00:26:14 +00:00
Raghavendra Nagaraj 1962dc88eb net/http: fix Server.IdleTimeout and Server.ReadHeaderTimeout docs
CL 46434 changed the doc for Server.IdleTimeout to include
falling back to Server.ReadHeaderTimeout if both
Server.IdleTimeout and Server.ReadTimeout are zero.
However, we explicitly set the ReadDeadlines firstly based
off Server.IdleTimeout or Server.ReadTimeout before attempting
to read the next request, thus the current doc is incorrect.

This CL reverts CL 46434 and also updates the doc for
Server.ReadHeaderTimeout to documenting falling back
to Server.ReadTimeout, if the former is zero, otherwise
there is no timeout.

Fixes #32053

Change-Id: I43dd0252d1bcee6c29a8529abd84c84a49b2fba9
GitHub-Last-Rev: e1cdb59977
GitHub-Pull-Request: golang/go#32164
Reviewed-on: https://go-review.googlesource.com/c/go/+/178337
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
2019-06-19 05:46:46 +00:00
Andrew Bonventre 5ce1819cca doc, net/http: add release notes for net/http and fix doc comments
Change-Id: I684e3522e387b2d96d5cfb2878d2f77bf4558443
Reviewed-on: https://go-review.googlesource.com/c/go/+/181545
Reviewed-by: Bryan C. Mills <bcmills@google.com>
2019-06-10 20:32:36 +00:00
Brad Fitzpatrick 4c84d87813 net/http: support BaseContext & ConnContext for http2 Server
This is the net/http half of #32476. This supplies the method needed
by the other half in x/net/http2 in the already-submitted CL 181259,
which this CL also bundles in h2_bundle.go.

Thanks to Tom Thorogood (@tmthrgd) for the bug report and test.

Fixes #32476
Updates #30694

Change-Id: I79d2a280e486fbf75d116f6695fd3abb61278765
Reviewed-on: https://go-review.googlesource.com/c/go/+/181260
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2019-06-07 18:48:19 +00:00
Dmitri Shuralyov 003dbc4cda net/http: roll back "clean the path of the stripped URL by StripPrefix"
Roll back CL 161738. That fix changed StripPrefix behavior in the
general case, not just in the situation where where stripping the
prefix from path resulted in the empty string, causing issue #31622.

That kind of change to StripPrefix behavior is not backwards compatible,
and there can be a smaller, more targeted fix for the original issue.

Fixes #31622
Updates #30165

Change-Id: Ie2fcfe6787a32e44f71d564d8f9c9d580fc6f704
Reviewed-on: https://go-review.googlesource.com/c/go/+/180498
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2019-06-04 22:36:51 +00:00
Brad Fitzpatrick ba66d89d78 net/http: prevent Transport from spamming stderr on server 408 reply
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-30 22:29:29 +00:00
Johan Brandhorst 220552f662 net/http: enable WASM fetch where supported
The existing check was introduced to allow tests to pass
on WASM without an environment where the fetch RoundTripper
could run. However, the check now prohibits the use of the
Fetch RoundTripper in all WASM tests, even where the
RoundTripper could run. The new change should only disable
the RoundTripper when used in an environment without fetch.

Fixes #32289

Change-Id: I30d2e0dbcb0e64d4b1a46b583f7e984c2a57d5e5
Reviewed-on: https://go-review.googlesource.com/c/go/+/179118
Run-TryBot: Agniva De Sarker <agniva.quicksilver@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2019-05-30 17:59:57 +00:00
Brad Fitzpatrick 770746af1c net/http: quiet some log spam in tests
One of these tests creates a bunch of connections concurrently, then
discovers it doesn't need them all, which then makes the server log
that the client went away midway through the TLS handshake. Perhaps
the server should recognize that as a case not worthy of logging
about, but this is a safer way to eliminate the stderr spam during go
test for now.

The other test's client gives up on its connection and closes it,
similarly confusing the server.

Change-Id: I49ce442c9a63fc437e58ca79f044aa76e8c317b5
Reviewed-on: https://go-review.googlesource.com/c/go/+/179179
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
2019-05-28 18:17:57 +00:00
Brad Fitzpatrick 3970667d95 net/http: fix TestTransportServerClosingUnexpectedly flake
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-28 00:54:10 +00:00
Richard Musiol c468ad0417 syscall/js: replace TypedArrayOf with CopyBytesToGo/CopyBytesToJS
The typed arrays returned by TypedArrayOf were backed by WebAssembly
memory. They became invalid each time we grow the WebAssembly memory.
This made them very error prone and hard to use correctly.

This change removes TypedArrayOf completely and instead introduces
CopyBytesToGo and CopyBytesToJS for copying bytes between a byte
slice and an Uint8Array. This breaking change is still allowed for
the syscall/js package.

Fixes #31980.
Fixes #31812.

Change-Id: I14c76fdd60b48dd517c1593972a56d04965cb272
Reviewed-on: https://go-review.googlesource.com/c/go/+/177537
Run-TryBot: Richard Musiol <neelance@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2019-05-24 09:38:37 +00:00
Russ Cox 06b0babf31 all: shorten some tests
Shorten some of the longest tests that run during all.bash.
Removes 7r 50u 21s from all.bash.

After this change, all.bash is under 5 minutes again on my laptop.

For #26473.

Change-Id: Ie0460aa935808d65460408feaed210fbaa1d5d79
Reviewed-on: https://go-review.googlesource.com/c/go/+/177559
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2019-05-22 12:54:00 +00:00
Russ Cox 02fe6ba958 all: remove PEM-encoded private keys from tests
Gerrit is complaining about pushes that affect these files
and forcing people to use -o nokeycheck, which defeats
the point of the check. Hide the keys from this kind of scan
by marking them explicitly as testing keys.

This is a little annoying but better than training everyone
who ever edits one of these test files to reflexively override
the Gerrit check.

The only remaining keys explicitly marked as private instead
of testing are in examples, and there's not much to do
about those. Hopefully they are not edited as much.

Change-Id: I4431592b5266cb39fe6a80b40e742d97da803a0b
Reviewed-on: https://go-review.googlesource.com/c/go/+/178178
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2019-05-21 20:03:55 +00:00
Brad Fitzpatrick c6f9321b5a net/http/httptest: update docs, remove old inaccurate sentence
The "After it is called, changing rw.Header will not affect
rw.HeaderMap" claim predates the Result method which changed how the
Recorder should be used.

Fixes #32144
Fixes #32136

Change-Id: I95bdfa5ac489ce7b0202824bb5663f4da188e8a7
Reviewed-on: https://go-review.googlesource.com/c/go/+/178058
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
2019-05-20 04:22:40 +00:00
Dmitri Shuralyov 3f7c560829 net/http/httputil: remove os.Stderr claim in ReverseProxy.ErrorLog docs
The motivation for doing so is to avoid making inaccurate claims.
Logging may not go to os.Stderr if anyone overrides the log package's
default output via https://godoc.org/log#SetOutput. Saying that
the standard logger is used should be sufficient to explain the
behavior, and users can infer that os.Stderr is used by default,
unless it's changed.

This change is the same as what was applied to http.Server.ErrorLog
documentation in CL 53950.

Change-Id: I32873fc548ceee573f8616b4d49b8a8b98881803
Reviewed-on: https://go-review.googlesource.com/c/go/+/176817
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2019-05-17 17:33:16 +00:00
Brad Fitzpatrick cb5c82bc3d net/http: update vendored, bundled x/net/http2
For:

    http2: track reused connections
    https://golang.org/cl/176720 (updates golang/go#31982)

Some x/sys/unix updates come along for the ride too.

I filed #32031 for making the bundling process less difficult and
error-prone in the future.

Change-Id: Ic822080991ffa2d50352c5f613e45648a327cf16
Reviewed-on: https://go-review.googlesource.com/c/go/+/177037
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-05-16 19:56:27 +00:00
Jonathon Lacher a5cea062b3 net/http/httputil: remove all fields in Connection header
In the reverseproxy, replace use (Header).Get, which returns only one value
of a multiple value header, with using the Header map directly. Also fixes
corresponding tests which hid the bug, and adds more tests.

Fixes #30303

Change-Id: Ic9094b5983043460697748759f6dfd95fc111db7
GitHub-Last-Rev: b41038143f
GitHub-Pull-Request: golang/go#30687
Reviewed-on: https://go-review.googlesource.com/c/go/+/166298
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2019-05-14 15:37:14 +00:00