The build constraint is no longer useful. It doesn't prevent this
package contributing module requirements to x/net, that was already
resolved by carving h2demo into its own module in golang/go#30685.
Few people do go get -u golang.org/x/net/... in GOPATH mode by now,
so there's no need to optimize for avoiding polluting GOPATH/bin.
Removing the build constraint allows the package to be visible and
tested by trybots and builders. It's also simpler.
Updates golang/go#34361
Change-Id: I84b5d70aab210ca8e4f5494160ae4d9049ef08ad
Reviewed-on: https://go-review.googlesource.com/c/net/+/196036
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
(cherry picked from commit a8b05e9114)
Reviewed-on: https://go-review.googlesource.com/c/net/+/198917
In certain shutdown cases (from the client and/or server), the http2
Server can Push stream-specific frames on closed streams. This caused
memory leaks in the random write scheduler.
As a conservative fix for backporting, just clear the map element
whenever its queue value is empty. The map entry is re-created as
needed anyway. This isn't perfectly ideal (it adds a map+delete and
free queue put+get) in the case where a stream is open & actively
writing, but it's an easy fix for now. A future CL can optimize all
this code. It looks like there are some other good optimization
opportunities in related code anyway. But I'd rather that happen on
master and not be done in a backported change.
Updates golang/go#34636 (needs bundle to std before fixed)
Updates golang/go#33812
Change-Id: I21508ba2ebc361e8b8532d0d1cebf882e82c473c
Reviewed-on: https://go-review.googlesource.com/c/net/+/198462
Reviewed-by: Bryan C. Mills <bcmills@google.com>
(cherry picked from commit d98b1b4438)
Reviewed-on: https://go-review.googlesource.com/c/net/+/198617
Reviewed-by: Andrew Bonventre <andybons@golang.org>
Run-TryBot: Andrew Bonventre <andybons@golang.org>
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.
This addresses CVE-2019-9512 and CVE-2019-9514.
Fixesgolang/go#33606
Change-Id: I842968fc6ed3eac654b497ade8cea86f7267886b
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/525552
Reviewed-by: Brad Fitzpatrick <bradfitz@google.com>
nextStreamID was used as a means to determine if the connection was
being reused. Multiple requests can see a new connection because the
nextStreamID is updated after a ClientTrace reports it is being reused.
Updates golang/go#31982
Change-Id: Iaa4b62b217f015423cddb99fd86de75a352f8320
Reviewed-on: https://go-review.googlesource.com/c/net/+/176720
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Pre-declared trailers are written after a server's request handler
returns, in which case the trailer header frame is also responsible
for closing the stream. However, if the server handler does not
define a trailer header value before returning, the headers are empty
and no header frame is written. In this case, the stream would simply
hang.
This change fixes this by closing the stream if there are no header
values but endStream is true. Some other options to consider:
1. Reset the stream with an error; the user didn't define a trailer value
as they said they would. From my brief reading of the http/2 RFC,
this isn't actually required.
2. Send an explicitly empty header value.
Change-Id: I07e01250df60ac33fa6d34beb81ec2fa7e43c146
GitHub-Last-Rev: dfc532d785
GitHub-Pull-Request: golang/net#31
Reviewed-on: https://go-review.googlesource.com/c/net/+/161958
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This change adds a go.mod and go.sum file to this repo, following the
requirements stated in bcmills's comment here:
https://golang.org/issue/28136#issuecomment-462971974. It's
important to note that we will not be
adding versions to the repo for now.
This change also creates a new module with the module path
golang.org/x/net/http2/h2demo, and moves the h2demo command into it.
This is done by adding a go.mod file in the http2/h2demo directory.
As a result, the h2demo command and its dependencies are removed
from this and later pseudo-versions of the golang.org/x/net module.
The change was generated using Go 1.12.
Updates golang/go#28136Fixesgolang/go#30685
Change-Id: Ia5b0f6623c3374355b76106432190a0c9acf3d05
Reviewed-on: https://go-review.googlesource.com/c/net/+/162822
Run-TryBot: Michael Matloob <matloob@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
We were previously demoing HTTP/1 vs HTTP/2 loading performance using
http:// scheme (which uses HTTP/1 implicitly) on the http2.golang.org
domain for HTTP/1, and https://http2.golang.org for HTTP/2.
But then golang.org got into the HSTS preload list, forcing all
*.golang.org to only be HTTPS.
So now, rather than find a new base domain name, to compare against
HTTP/1 we instead use https://http1.golang.org/ and then use the SNI
name to determine whether we advertise "h2".
Also, some cleanup:
* remove launch.go; it's no longer used since we moved to kubernetes
* use a multi-stage Dockerfile rather than the hacky workarounds
that used to be necessary to simulate multiple stages
* modernize the kubernetes deployment stuff to match how we do
it elsewhere ("gcloud docker" is long deprecated too)
* update from Go 1.9 to Go 1.11 for the prod base
Fixesgolang/go#30033
Change-Id: I9f6b1f496d4005e5a08bf990843d440005a5b3e8
Reviewed-on: https://go-review.googlesource.com/c/160857
Reviewed-by: Andrew Bonventre <andybons@golang.org>
dynamic table size updates must occur at the beginning of the first
header block. The original fix, golang/go#25023, guaranteed it was at
the beginning of the very first block. The Close method implicitly
marked the end of the current header. We now document the Close behavior
and can track when we are at the beginning of the first block.
Updates golang/go#29187
Change-Id: I83ec39546527cb17d7de8a88ec417a46443d2baa
Reviewed-on: https://go-review.googlesource.com/c/153978
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
And add the http2.Transport.StrictMaxConcurrentStreams bool knob to
behavior being reverted.
In CL 53250 for golang/go#13774 (for Go 1.10) we changed the HTTP/2
Transport's policy such that a server's advertisement of a
MAX_CONCURRENT_STREAMS value meant that it was a maximum for the
entire process, instead of just a single connection.
We thought that was a reasonable interpretation of the spec and
provided nice safety against slamming a server from a bunch of
goroutines doing concurrent requests, but it's been largely
unpopular (see golang/go#27044). It's also different behavior from
HTTP/1 and because you're usually not sure which protocol version
you're going to get, you need to limit your outbound HTTP requests
anyway in case you're hitting an HTTP/1 server.
And nowadays we have the Go 1.11 Transport.MaxConnsPerHost knob too
(CL 71272 for golang/go#13957). It doesn't yet work for HTTP/2, but it
will in either Go 1.12 or Go 1.13 (golang/go#27753)
After this is bundled into net/http's, the default HTTP client will
have this knob set false, restoring the old Go 1.9 behavior where new
TCP connections are created as necessary. Users wanting the strict
behavior and import golang.org/x/net/http2 themselves and make a
Transport with StrictMaxConcurrentStreams set to true. Or they can set
Transport.MaxConnsPerHost, once that works for HTTP/2.
Updates golang/go#27044 (fixes after bundle into std)
Change-Id: I4efdad7698feaf674ee8e01032d2dfa5c2f8a3a8
Reviewed-on: https://go-review.googlesource.com/c/151857
Reviewed-by: Andrew Bonventre <andybons@golang.org>
On Solaris, the MakeRaw and Restore functions of the
x/crypto/ssh/terminal package have been implemented, but on upcoming
platforms such as AIX, Fuchsia and Hurd still have no implementation.
Change-Id: I253249376802273f0160bf3ff1062a66e07b280f
Reviewed-on: https://go-review.googlesource.com/c/147677
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Go's policy is to only support the past two releases (which is
currently Go 1.11 and Go 1.10). But because App Engine was stuck on Go
1.6 and Go 1.8 for so long, we kept kinda supporting Go 1.6 anyway,
even though we didn't actively test it with any CI system.
But that led to code getting disgusting and full of too many
+build-tagged files and indirection, as this change shows.
So, remove Go 1.8, Go 1.7, and Go 1.6 support. We still "support" Go
1.9 for now, even though it's also not actively tested.
Fixesgolang/go#26302
Change-Id: I4aa5793173e50ffcd67be52a464492ca48fc9a23
Reviewed-on: https://go-review.googlesource.com/c/145677
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
This generated 120 kB on the heap before at init, regardless of
whether somebody used http2. Worse, because we vendored it into std,
users would have two copies, for about 256 kB of memory. After CL
127235 that went down to 60 kB per copy, so 120 kB for a binary using
golang.org/x/net/http2 explicitly.
With this, it goes to 0 until one of the two copies in the binary
actually uses one of the http2 packages.
I wasn't able to measure any difference with the Once.Do in the decode
path:
name old time/op new time/op delta
HuffmanDecode-4 732ns ± 8% 707ns ± 3% ~ (p=0.268 n=10+9)
(admittedly noisy)
Change-Id: I6c1065abc0c3458f3cb69e0f678978267ff35ea2
Reviewed-on: https://go-review.googlesource.com/127275
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reduces process-wide heap (inuse_space) by 60kB by using a pointer to
a fixed-sized array instead of a slice of a fixed size.
Before:
119.44kB 23.43% 23.43% 147.88kB 29.01% golang.org/x/net/http2/hpack.addDecoderNode
After:
59.72kB 13.28% 39.85% 87.94kB 19.56% golang.org/x/net/http2/hpack.addDecoderNode
(This is all work from an init func in http2/hpack)
Doesn't seem to affect runtime performance.
Measured with:
$ cat huffman_test.go
package main
import (
"testing"
_ "golang.org/x/net/http2"
)
func TestMem(t *testing.T) {}
$ GODEBUG=memprofilerate=1 go test -memprofilerate=1 -memprofile=mem.prof -v .
=== RUN TestMem
--- PASS: TestMem (0.00s)
PASS
ok huffmem 0.052s
$ go tool pprof --inuse_space mem.prof
Change-Id: I5e56a5a2682f1063c955b342b37e97ca4c303dab
Reviewed-on: https://go-review.googlesource.com/127235
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Implements h2c by leveraging the existing http2.Server by implementing the 2
ways to start an h2c connection as described in RFC 7540, which are: (1)
create a connection starting with HTTP/1 and then upgrading to h2c [Section 3.2]
and (2) starting a connection directly speaking h2c (aka starting with prior
knowledge) [Section 3.4].
For both of the above connection methods the implementation strategy is to
hijack a HTTP/1 connection, perform the h2c connection on the hijacked
net.Conn, and create a suitably configured net.Conn to pass into
http2.Server.ServeConn.
For h2c with prior knowledge this is relatively simple. For that we just have
to verify the HTTP/2 client preface has been written to the net.Conn, and
then reforward the client preface to the hijacked connection.
For h2c upgraded from HTTP/1, this is a bit more involved. First we validate
the HTTP/1 Upgrade request, and respond to the client with 101 Switching
Protocols. Then we write a HTTP/2 client preface on behalf of the client,
and a settings frame and a headers frame which correspond to what was in
the upgrade request. Then since http2.Server is going respond with a
settings ACK, we swallow it so that it is not forwarded to the client since
for h2c upgrade from HTTP/1 the 101 Switching Protocols response replaces
the settins ACK.
Fixesgolang/go#14141
Change-Id: I435f40216c883809c0dcb75426c9c59e2ea31182
Reviewed-on: https://go-review.googlesource.com/112999
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
It confused somebody who thought things were hanging because they had
expected to see a response before they started streaming data.
Change-Id: If672956efde3756c966b0c88b9c15ed21daeccba
Reviewed-on: https://go-review.googlesource.com/125644
Reviewed-by: Ian Lance Taylor <iant@golang.org>
We were previously only using the new-ish Request.GetBody to "rewind"
a Request.Body on retry when it seemed that we hadn't started the
read+write body copy process from the old request yet.
Apparently there's a bug somewhere, so this is a safe minimal fix for
now, unconditionally using GetBody when it's available, rather than
only using it when it seems we need to. Should have no performance impact
because it's supposed to be cheap, and this only happens on rare retries
where the server's GOAWAY came in-flight while we were writing a request.
Updates golang/go#25009 (not a fix, but enough for Go 1.11)
Change-Id: Ia462944d4a68cf2fde8d32b7b357b450c509a349
Reviewed-on: https://go-review.googlesource.com/123476
Reviewed-by: Ian Lance Taylor <iant@golang.org>
There is no guaranteed ordering between writing headers and graceful
shutdown, both put a message on different channels.
Fixesgolang/go#26190
Change-Id: I883fcf1de4bbe5a87af418d1b897a8aa941f1fd4
Reviewed-on: https://go-review.googlesource.com/122335
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
In Go's HTTP/1.x Server, a "Connection: close" response from a handler results
in the TCP connection being closed.
In HTTP/2, a "Connection" header is illegal and we weren't previously
handling it, generating malformed responses to clients. (This is one
of our violations listed in golang/go#25023)
There was also a feature request in golang/go#20977 for a way for
HTTP/2 handlers to close the connection after the response. Since we
already close the connection for "Connection: close" for HTTP/1.x, do
the same for HTTP/2 and map it to a graceful GOAWAY errcode=NO
response.
Updates golang/go#25023 (improves 8.1.2.2. Connection-Specific Header Fields)
Updates golang/go#20977 (fixes after vendor into std)
Change-Id: Iefb33ea73be616052533080c63b54ae679b1d154
Reviewed-on: https://go-review.googlesource.com/121415
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
