Updates http2 to x/net/http2 git rev 0e2717d for:
http2: conditionally log stacks from panics in Server Handlers like net/http
https://golang.org/cl/33102Fixes#17790
Change-Id: Idd3f0c65540398d41b412a33f1d80de3f7f31409
Reviewed-on: https://go-review.googlesource.com/33103
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
Add an explicit way for Handlers to abort their response to the client
and also not spam their error log with stack traces.
panic(nil) also worked in the past (for http1 at least), so continue
to make that work (and test it). But ErrAbortHandler is more explicit.
Updates #17790 (needs http2 updates also)
Change-Id: Ib1456905b27e2ae8cf04c0983dc73e314a4a751e
Reviewed-on: https://go-review.googlesource.com/33099
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Server.TLSNextProto being nil is necessary but not sufficient but
http2 being automatically enabled.
Fixes#16588
Change-Id: I5b18690582f9b12ef05b58235e1eaa52483be285
Reviewed-on: https://go-review.googlesource.com/33090
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Go's http1 implementation originally had a mechanism to send HTTP
trailers based on pre-declaring the trailer keys whose values you'd
later let after the header was written.
http2 copied the same mechanism, but it was found to be unsufficient
for gRPC's wire protocol. A second trailer mechanism was added later
(but only to http2) for handlers that want to send a trailer without
knowing in advance they'd need to.
Copy the same mechanism back to http1 and document it.
Fixes#15754
Change-Id: I8c40d55e28b0e5b7087d3d1a904a392c56ee1f9b
Reviewed-on: https://go-review.googlesource.com/32479
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Also updates x/net/http2 to git rev 541150 for:
http2: add support for graceful shutdown of Server
https://golang.org/cl/32412
http2: make http2.Server access http1's Server via an interface check
https://golang.org/cl/32417Fixes#4674Fixes#9478
Change-Id: I8021a18dee0ef2fe3946ac1776d2b10d3d429052
Reviewed-on: https://go-review.googlesource.com/32329
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This is an alternate solution to https://golang.org/cl/31445
Instead of making NewRequest return a request with Request.Body == nil
to signal a zero byte body, add a well-known variable that means
explicitly zero.
Too many tests inside Google (and presumably the outside world)
broke.
Change-Id: I78f6ecca8e8aa1e12179c234ccfb6bcf0ee29ba8
Reviewed-on: https://go-review.googlesource.com/31726
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
This CL changes how the http1 Server reads from the client.
The goal of this change is to make the Request.Context given to Server
Handlers become done when the TCP connection dies (has seen any read
or write error). I didn't finish that for Go 1.7 when Context was
added to http package.
We can't notice the peer disconnect unless we're blocked in a Read
call, though, and previously we were only doing read calls as needed,
when reading the body or the next request. One exception to that was
the old pre-context CloseNotifier mechanism.
The implementation of CloseNotifier has always been tricky. The past
few releases have contained the complexity and moved the
reading-from-TCP-conn logic into the "connReader" type. This CL
extends connReader to make sure that it's always blocked in a Read
call, at least once the request body has been fully consumed.
In the process, this deletes all the old CloseNotify code and unifies
it with the context cancelation code. The two notification mechanisms
are nearly identical, except the CloseNotify path always notifies on
the arrival of pipelined HTTP/1 requests. We might want to change that
in a subsequent commit. I left a TODO for that. For now there's no
change in behavior except that the context now cancels as it was
supposed to.
As a bonus that fell out for free, a Handler can now use CloseNotifier
and Hijack together in the same request now.
Fixes#15224 (make http1 Server always in a Read, like http2)
Fixes#15927 (cancel context when underlying connection closes)
Updates #9763 (CloseNotifier + Hijack)
Change-Id: I972cf6ecbab7f1230efe8cc971e89f8e6e56196b
Reviewed-on: https://go-review.googlesource.com/31173
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Make a zero-byte write to a hijacked connection not log anything, so handlers
can test whether a connection is hacked by doing a Write(nil).
Fixes#16456
Change-Id: Id56caf822c8592067bd8422672f0c1aec89e866c
Reviewed-on: https://go-review.googlesource.com/30812
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
I decided not to expand the API for this per discusion on #16220.
Fixes#16220
Change-Id: I65cb2eacd4ec28c79438a8f7c30024524a484ce6
Reviewed-on: https://go-review.googlesource.com/30082
Reviewed-by: Daniel Theophanes <kardianos@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Regression from Go 1.6 to Go 1.7rc1: we had broken the ability for
users to vendor "golang.org/x/net/http2" or "golang.org/x/net/route"
because we were vendoring them ourselves and cmd/go and cmd/compile do
not understand multiple vendor directories across multiple GOPATH
workspaces (e.g. user's $GOPATH and default $GOROOT).
As a short-term fix, since fixing cmd/go and cmd/compile is too
invasive at this point in the cycle, just rename "golang.org" to
"golang_org" for the standard library's vendored copy.
Fixes#16333
Change-Id: I9bfaed91e9f7d4ca6bab07befe80d71d437a21af
Reviewed-on: https://go-review.googlesource.com/24902
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Don't configure HTTP/2 in http.Server.Serve(net.Listener) if the
Server's TLSConfig is set and doesn't include the "h2" NextProto
value. This avoids mutating a *tls.Config already in use if
previously passed to tls.NewListener.
Also document this. (it's come up a few times now)
Fixes#15908
Change-Id: I283eed82fdb29a791f80d801aadd9f75db244de0
Reviewed-on: https://go-review.googlesource.com/24508
Reviewed-by: Andrew Gerrand <adg@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
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>
Make clear negotiation can happen via NPN or ALPN, similar to
http.Transport.TLSNextProto and x/net/http2.NextProtoTLS.
Change-Id: Ied00b842bc04e11159d6d2107beda921cefbc6ca
Reviewed-on: https://go-review.googlesource.com/23108
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Summary: Go's HTTP/1.x server closes the request body once writes are
flushed. Go's HTTP/2 server supports concurrent read & write.
Added a TODO to make the HTTP/1.x server also support concurrent
read+write. But for now, document it.
Updates #15527
Change-Id: I81f7354923d37bfc1632629679c75c06a62bb584
Reviewed-on: https://go-review.googlesource.com/23011
Reviewed-by: Andrew Gerrand <adg@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>
Clarify that it includes the RFC 7230 "request-line".
Fixes#15494
Change-Id: I9cc5dd5f2d85ebf903229539208cec4da5c38d04
Reviewed-on: https://go-review.googlesource.com/22656
Reviewed-by: Andrew Gerrand <adg@golang.org>
Standardize on space between "RFC" and number. Additionally change
the couple "a RFC" instances to "an RFC."
Fixes#15258
Change-Id: I2b17ecd06be07dfbb4207c690f52a59ea9b04808
Reviewed-on: https://go-review.googlesource.com/21902
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Also, don't read from the Request.Headers in the http Server code once
ServeHTTP has started. This is partially redundant with documenting
that handlers shouldn't mutate request, but: the space is free due to
bool packing, it's faster to do the checks once instead of N times in
writeChunk, and it's a little nicer to code which previously didn't
play by the unwritten rules. But I'm not going to fix all the cases.
Fixes#14940
Change-Id: I612a8826b41c8682b59515081c590c512ee6949e
Reviewed-on: https://go-review.googlesource.com/21530
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Gerrand <adg@golang.org>
Go 1.6's HTTP/1.x Transport started enforcing that responses have 3
status digits, per the spec, but we could still write out invalid
status codes ourselves if the called
ResponseWriter.WriteHeader(0). That is bogus anyway, since the minimum
status code is 1xx, but be a little bit less bogus (and consistent)
and zero pad our responses.
Change-Id: I6883901fd95073cb72f6b74035cabf1a79c35e1c
Reviewed-on: https://go-review.googlesource.com/19130
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Gerrand <adg@golang.org>
The default is 10MB, like http2, but can be configured with a new
field http.Transport.MaxResponseHeaderBytes.
Fixes#9115
Change-Id: I01808ac631ce4794ef2b0dfc391ed51cf951ceb1
Reviewed-on: https://go-review.googlesource.com/21329
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Gerrand <adg@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>
You can not use cannot, but you cannot spell cannot can not.
Change-Id: I2f0971481a460804de96fd8c9e46a9cc62a3fc5b
Reviewed-on: https://go-review.googlesource.com/19772
Reviewed-by: Rob Pike <r@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>
Fixes#14001
Change-Id: I6f9bc3028345081758d8f537c3aaddb2e254e69e
Reviewed-on: https://go-review.googlesource.com/18708
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Brad Fitzpatrick
Emmanuel Odeke
Andrew Gerrand
Dan Peterson
Caio Marcelo de Oliveira Filho
Martin Möhrmann
Shawn Smith
Josh Bleecher Snyder