Fixes#16063
Change-Id: I2e8695beb657b0aef067e83f086828d8857787ed
Reviewed-on: https://go-review.googlesource.com/24130
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Fixes#15948
Change-Id: Idd79859b3e98d61cd4e3ef9caa5d3b2524fd026a
Reviewed-on: https://go-review.googlesource.com/23810
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Fixes#15960
Change-Id: I7503f6ede33e6a1a93cee811d40f7b297edf47bc
Reviewed-on: https://go-review.googlesource.com/23811
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Updates x/net/http2 to golang.org/cl/23220
(http2: with Go 1.7 set Request.Context in ServeHTTP handlers)
Fixes#15134
Change-Id: I73bac2601118614528f051e85dab51dc48e74f41
Reviewed-on: https://go-review.googlesource.com/23221
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Gerrand <adg@golang.org>
Verify that for a server doing chunked encoding, with the final data
and EOF arriving together, the client will reuse the connection even
if it closes the body without seeing an EOF. The server sends at least
one non-zero chunk and one zero chunk. This verifies that the client's
bufio reading reads ahead and notes the EOF, so even if the JSON
decoder doesn't read the EOF itself, as long as somebody sees it, a
close won't forcible tear down the connection. This was true at least
of https://golang.org/cl/21291
No code change. Test already passed (even with lots of runs, including
in race mode with randomized goroutine scheduling).
Updates #15703
Change-Id: I2140b3eec6b099b6b6e54f153fe271becac5d949
Reviewed-on: https://go-review.googlesource.com/23200
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Gerrand <adg@golang.org>
Might deflake the occasional linux-amd64-race failures.
Change-Id: I273b0e32bb92236168eb99887b166e079799c1f1
Reviewed-on: https://go-review.googlesource.com/22858
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This adds a context key named LocalAddrContextKey (for now, see #15229) to
let users access the net.Addr of the net.Listener that accepted the connection
that sent an HTTP request. This is similar to ServerContextKey which provides
access to the *Server. (A Server may have multiple Listeners)
Fixes#6732
Change-Id: I74296307b68aaaab8df7ad4a143e11b5227b5e62
Reviewed-on: https://go-review.googlesource.com/22672
Reviewed-by: Andrew Gerrand <adg@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The previous cleanup was done with a buggy tool, missing some potential
rewrites.
Change-Id: I333467036e355f999a6a493e8de87e084f374e26
Reviewed-on: https://go-review.googlesource.com/21378
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The http2 spec defines a magic string which initates an http2 session:
"PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n"
It was intentionally chosen to kinda look like an HTTP request, but
just different enough to break things not ready for it. This change
makes Go ready for it.
Notably: Go now accepts the request header (the prefix "PRI *
HTTP/2.0\r\n\r\n") as a valid request, even though it doesn't have a
Host header. But we now mark it as "Connection: close" and teach the
Server to never read a second request from the connection once that's
seen. If the http.Handler wants to deal with the upgrade, it has to
hijack the request, read out the "body", compare it against
"SM\r\n\r\n", and then speak http2. One of the new tests demonstrates
that hijacking.
Fixes#14451
Updates #14141 (h2c)
Change-Id: Ib46142f31c55be7d00c56fa2624ec8a232e00c43
Reviewed-on: https://go-review.googlesource.com/21327
Reviewed-by: Andrew Gerrand <adg@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
TimeoutHandler was starting the Timer when the handler was created,
instead of when serving a request. It also was sharing it between
multiple requests, which is incorrect, as the requests might start
at different times.
Store the timeout duration and create the Timer when ServeHTTP is
called. Different requests will have different timers.
The testing plumbing was simplified to store the channel used to
control when timeout happens. It overrides the regular timer.
Fixes#14568.
Change-Id: I4bd51a83f412396f208682d3ae5e382db5f8dc81
Reviewed-on: https://go-review.googlesource.com/20046
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The tree's pretty inconsistent about single space vs double space
after a period in documentation. Make it consistently a single space,
per earlier decisions. This means contributors won't be confused by
misleading precedence.
This CL doesn't use go/doc to parse. It only addresses // comments.
It was generated with:
$ perl -i -npe 's,^(\s*// .+[a-z]\.) +([A-Z]),$1 $2,' $(git grep -l -E '^\s*//(.+\.) +([A-Z])')
$ go test go/doc -update
Change-Id: Iccdb99c37c797ef1f804a94b22ba5ee4b500c4f7
Reviewed-on: https://go-review.googlesource.com/20022
Reviewed-by: Rob Pike <r@golang.org>
Reviewed-by: Dave Day <djd@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
ListenAndServeTLS doesn't require cert and key file names if the
server's TLSConfig has a cert configured. This code was never updated
when the GetCertificate hook was added to *tls.Config, however.
Fixes#14268
Change-Id: Ib282ebb05697edd37ed8ff105972cbd1176d900b
Reviewed-on: https://go-review.googlesource.com/19381
Reviewed-by: Russ Cox <rsc@golang.org>
The test sends two HTTP/1.1 pipelined requests. The first is
completedly by the second, and as such triggers an immediate call to the
CloseNotify channel. The second calls the CloseNotify channel after the
overall connection is closed.
The test was passing fine on gc because the code would enter the select
loop before running the handler, so the send on gotReq would always be
seen first. On gccgo the code would sometimes enter the select loop
after the handler had already finished, meaning that the select could
choose between gotReq and sawClose. If it picked sawClose, it would
never close the overall connection, and the httptest server would hang.
The same hang could be induced with gc by adding a time.Sleep
immediately before the select loop.
Deflake the test by 1) don't close the overall connection until both
requests have been seen; 2) don't exit the loop until both closes have
been seen.
Fixes#14231.
Change-Id: I9d20c309125422ce60ac545f78bcfa337aec1c7d
Reviewed-on: https://go-review.googlesource.com/19281
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Passes with go test -race -count=1000 -name=TestServerValidatesHostHeader now
without hanging.
Fixes#13950
Change-Id: I41c3a555c642595c95c8c52f19a05a4c68e67630
Reviewed-on: https://go-review.googlesource.com/18660
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Many browsers now support schemeless URLs in the Location headers
and also it is allowed in the draft HTTP/1.1 specification (see
http://stackoverflow.com/q/4831741#comment25926312_4831741), but
Go standard library lacks support for them.
This patch implements schemeless URLs support in http.Redirect().
Since url.Parse() correctly handles schemeless URLs, I've just added
an extra condition to verify URL's Host part in the absoulute/relative
check in the http.Redirect function.
Also I've moved oldpath variable initialization inside the block
of code where it is used.
Change-Id: Ib8a6347816a83e16576f00c4aa13224a89d610b5
Reviewed-on: https://go-review.googlesource.com/14172
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Third time's a charm.
Thanks to Ralph Corderoy for noticing the DEL omission.
Update #11207
Change-Id: I174fd01eaecceae1eb220f2c9136e12d40fbe943
Reviewed-on: https://go-review.googlesource.com/18375
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
As Andy Balholm noted in #11207:
"RFC2616 §4.2 says that a header's field-content can consist of *TEXT,
and RFC2616 §2.2 says that TEXT is <any OCTET except CTLs, but
including LWS>, so that would mean that bytes greater than 128 are
allowed."
This is a partial rollback of the strictness from
https://golang.org/cl/11207 (added in the Go 1.6 dev cycle, only
released in Go 1.6beta1)
Fixes#11207
Change-Id: I3a752a7941de100e4803ff16a5d626d5cfec4f03
Reviewed-on: https://go-review.googlesource.com/18374
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Andrew Gerrand <adg@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Update bundled http2 to git rev d1ba260648 (https://golang.org/cl/18288).
Fixes the flaky TestTransportAndServerSharedBodyRace_h2.
Also adds some debugging to TestTransportAndServerSharedBodyRace_h2
which I hope won't ever be necessary again, but I know will be.
Fixes#13556
Change-Id: Ibcf2fc23ec0122dcac8891fdc3bd7f8acddd880e
Reviewed-on: https://go-review.googlesource.com/18289
Reviewed-by: Andrew Gerrand <adg@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The CloseNotifier implementation and documentation was
substantially changed in https://golang.org/cl/17750 but it was a bit
too aggressive.
Issue #13666 highlighted that in addition to breaking external
projects, even the standard library (httputil.ReverseProxy) didn't
obey the new rules about not using CloseNotifier until the
Request.Body is fully consumed.
So, instead of fixing httputil.ReverseProxy, dial back the rules a
bit. It's now okay to call CloseNotify before consuming the request
body. The docs now say CloseNotifier may wait to fire before the
request body is fully consumed, but doesn't say that the behavior is
undefined anymore. Instead, we just wait until the request body is
consumed and start watching for EOF from the client then.
This CL also adds a test to ReverseProxy (using a POST request) that
would've caught this earlier.
Fixes#13666
Change-Id: Ib4e8c29c4bfbe7511f591cf9ffcda23a0f0b1269
Reviewed-on: https://go-review.googlesource.com/18144
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
https://golang.org/cl/18087 added a bunch of t.Parallel calls, which
aren't compatible with the afterTest func. But in short mode, afterTest
is a no-op. To keep all.bash (short mode) fast, conditionally set
t.Parallel when in short mode, but keep it unset for compatibility with
afterFunc otherwise.
Fixes#13804
Change-Id: Ie841fbc2544e1ffbee43ba1afbe895774e290da0
Reviewed-on: https://go-review.googlesource.com/18143
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Takes 3% off my all.bash run time.
For #10571.
Change-Id: I8f00f523d6919e87182d35722a669b0b96b8218b
Reviewed-on: https://go-review.googlesource.com/18087
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
CloseNotifier wasn't well specified previously. This CL simplifies its
implementation, clarifies the public documentation on CloseNotifier,
clarifies internal documentation on conn, and fixes two CloseNotifier
bugs in the process.
The main change, though, is tightening the rules and expectations for using
CloseNotifier:
* the caller must consume the Request.Body first (old rule, unwritten)
* the received value is the "true" value (old rule, unwritten)
* no promises for channel sends after Handler returns (old rule, unwritten)
* a subsequent pipelined request fires the CloseNotifier (new behavior;
previously it never fired and thus effectively deadlocked as in #13165)
* advise that it should only be used without HTTP/1.1 pipelining (use HTTP/2
or non-idempotent browsers). Not that browsers actually use pipelining.
The main implementation change is that each Handler now gets its own
CloseNotifier channel value, rather than sharing one between the whole
conn. This means Handlers can't affect subsequent requests. This is
how HTTP/2's Server works too. The old docs never clarified a behavior
either way. The other side effect of each request getting its own
CloseNotifier channel is that one handler can't "poison" the
underlying conn preventing subsequent requests on the same connection
from using CloseNotifier (this is #9763).
In the old implementation, once any request on a connection used
ClosedNotifier, the conn's underlying bufio.Reader source was switched
from the TCPConn to the read side of the pipe being fed by a
never-ending copy. Since it was impossible to abort that never-ending
copy, we could never get back to a fresh state where it was possible
to return the underlying TCPConn to callers of Hijack. Now, instead of
a never-ending Copy, the background goroutine doing a Read from the
TCPConn (or *tls.Conn) only reads a single byte. That single byte
can be in the request body, a socket timeout error, io.EOF error, or
the first byte of the second body. In any case, the new *connReader
type stitches sync and async reads together like an io.MultiReader. To
clarify the flow of Read data and combat the complexity of too many
wrapper Reader types, the *connReader absorbs the io.LimitReader
previously used for bounding request header reads. The
liveSwitchReader type is removed. (an unused switchWriter type is also
removed)
Many fields on *conn are also documented more fully.
Fixes#9763 (CloseNotify + Hijack together)
Fixes#13165 (deadlock with CloseNotify + pipelined requests)
Change-Id: I40abc0a1992d05b294d627d1838c33cbccb9dd65
Reviewed-on: https://go-review.googlesource.com/17750
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Updates bundled copy of x/net/http2 to include
https://golang.org/cl/17823 (catching panics in Handlers)
Fixes#13555
Change-Id: I08e4e38e736a8d93f5ec200e8041c143fc6eafce
Reviewed-on: https://go-review.googlesource.com/17824
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
New implementation of TimeoutHandler: buffer everything to memory.
All or nothing: either the handler finishes completely within the
timeout (in which case the wrapper writes it all), or it misses the
timeout and none of it gets written, in which case handler wrapper can
reliably print the error response without fear that some of the
wrapped Handler's code already wrote to the output.
Now the goroutine running the wrapped Handler has its own write buffer
and Header copy.
Document the limitations.
Fixes#9162
Change-Id: Ia058c1d62cefd11843e7a2fc1ae1609d75de2441
Reviewed-on: https://go-review.googlesource.com/17752
Reviewed-by: David Symonds <dsymonds@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
This is an example of converting an old HTTP/1-only test to test
against both HTTP/1 and HTTP/2.
Please send more of these!
Also, for comparing the http.Transport's responses between HTTP/1 and
HTTP/2, see clientserver_test.go's h12Compare type and tests using
h12Compare. Sometimes that's the more appropriate option.
Change-Id: Iea24d844481efd5849173b60e15dcc561a32b88f
Reviewed-on: https://go-review.googlesource.com/17409
Reviewed-by: Burcu Dogan <jbd@google.com>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This compares the behavior of server handlers and the net/http
Transport in both HTTP/1 and HTTP/2 mode and verifies they're the
same.
This also moves some client<->server tests into clientserver_test.go.
Many of them were in serve_test.go or transport_test.go but were
basically testing both.
h2_bundle.go is an update of the golang.org/x/net/http2 code
from https://golang.org/cl/17204 (x/net git rev c745c36eab10)
Fixes#13315Fixes#13316Fixes#13317
Fixes other stuff found in the process too
Updates #6891 (http2 support in general)
Change-Id: Id9c45fad44cdf70ac95d2b89e578d66e882d3cc2
Reviewed-on: https://go-review.googlesource.com/17205
Reviewed-by: Andrew Gerrand <adg@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
This CL adds skipped failing tests, showing differences between HTTP/1
and HTTP/2 behavior. They'll be fixed in later commits.
Only a tiny fraction of the net/http tests have been split into their
"_h1" and "_h2" variants. That will also continue. (help welcome)
Updates #6891
Updates #13315
Updates #13316
Updates #13317
Change-Id: I16c3c381dbe267a3098fb266ab0d804c36473a64
Reviewed-on: https://go-review.googlesource.com/17046
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Andrew Gerrand <adg@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The Server's server goroutine was panicing (but recovering) when
cleaning up after handling a request. It was pretty harmless (it just
closed that one connection and didn't kill the whole process) but it
was distracting.
Updates #13135
Change-Id: I2a0ce9e8b52c8d364e3f4ce245e05c6f8d62df14
Reviewed-on: https://go-review.googlesource.com/16572
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Gerrand <adg@golang.org>
* detect Content-Type on ReponseRecorder.Write[String] call
if header wasn't written yet, Content-Type header is not set and
Transfer-Encoding is not set.
* fix typos in serve_test.go
Updates #12986
Change-Id: Id2ed8b1994e64657370fed71eb3882d611f76b31
Reviewed-on: https://go-review.googlesource.com/16096
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Fixes#7782Fixes#9554
Updates #7237 (original metabug, before we switched to specific bugs)
Updates #11932 (plan9 still doesn't have net I/O deadline support)
Change-Id: I96f311b88b1501d884ebc008fd31ad2cf1e16d75
Reviewed-on: https://go-review.googlesource.com/15941
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: David du Colombier <0intro@gmail.com>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This enables HTTP/2 by default (for https only) if the user didn't
configure anything in their NPN/ALPN map. If they're using SPDY or an
alternate http2 or a newer http2 from x/net/http2, we do nothing
and don't use the standard library's vendored copy of x/net/http2.
Upstream remains golang.org/x/net/http2.
Update #6891
Change-Id: I69a8957a021a00ac353f9d7fdb9a40a5b69f2199
Reviewed-on: https://go-review.googlesource.com/15828
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Andrew Gerrand <adg@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The PROXY protocol is supported by several proxy servers such as haproxy
and Amazon ELB. This protocol allows services running behind a proxy to
learn the remote address of the actual client connecting to the proxy,
by including a single textual line at the beginning of the TCP
connection.
http://www.haproxy.org/download/1.5/doc/proxy-protocol.txt
There are several Go libraries for this protocol (such as
https://github.com/armon/go-proxyproto), which operate by wrapping a
net.Conn with an implementation whose RemoteAddr method reads the
protocol line before returning. This means that RemoteAddr is a blocking
call.
Before this change, http.Serve called RemoteAddr from the main Accepting
goroutine, not from the per-connection goroutine. This meant that it
would not Accept another connection until RemoteAddr returned, which is
not appropriate if RemoteAddr needs to do a blocking read from the
socket first.
Fixes#12943.
Change-Id: I1a242169e6e4aafd118b794e7c8ac45d0d573421
Reviewed-on: https://go-review.googlesource.com/15835
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
