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>
There is a race between scheduling the response of the header frame and
when the data frame error is detected. The header frame may be
queued before or after the stream reset.
Fixesgolang/go#25645
Change-Id: Id598eedfec9538fb7b154102a1a6e28e08fe117f
Reviewed-on: https://go-review.googlesource.com/121197
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Don't assume goroutines are gone immediately.
Copy the waitCondition & waitErrCondition helpers from net/http's
tests, so we can assume both exist in both repos, even though we only
use one of them as of this CL.
Fixesgolang/go#22889
Change-Id: Ife251c9552bc68646e174a7ac082b363e32132a1
Reviewed-on: https://go-review.googlesource.com/114012
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Bonventre <andybons@golang.org>
When a Content-Type that triggers content sniffing in old (but still in
significant use) browsers is sent, add the
X-Content-Type-Options: nosniff header, unless explicitly disabled.
Expose httpguts.SniffedContentType for use in the HTTP 1 implementation.
Will be tested by net/http.TestNoSniffHeader_h2.
Updates golang/go#24513
Change-Id: Id1ffea867a496393cb52c5a9f45af97d4b2fcf12
Reviewed-on: https://go-review.googlesource.com/112015
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
httplex was the original package name for shared code between net/http
and x/net/http2, but its name was too specific, and http/httpguts was
added later for other shared code.
We discussed merging httplex into httpguts at the time, but it didn't
happen earlier. This finishes the move.
Updates golang/go#23908
Change-Id: Ic7d6f39e584ca579d34b5ef5ec6a0c002a38a83c
Reviewed-on: https://go-review.googlesource.com/111875
Reviewed-by: Andrew Bonventre <andybons@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Introduce a common package x/net/http/httpguts which can be vendored by
net/http to share detail implementations of the HTTP specification with
x/net/http2.
Updates golang/go#23908
Change-Id: Id5a2d51e05135436cf406c4c4d1b13fca7f84a32
Reviewed-on: https://go-review.googlesource.com/104042
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
iana.org, www.iana.org and data.iana.org all present a valid TLS
certificate, so let's use it when fetching data to avoid errors in
transit.
Change-Id: I1f295442d24a221fe2b722c4782dceee38b960ec
Reviewed-on: https://go-review.googlesource.com/89415
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
In a given program there may be two separate copies of
ErrNoCachedConn: the h2_bundle.go version in net/http, and the user's
golang.org/x/net/http2 version. We need to be able to detect either
in net/http.
This CL adds a function to report whether an error value represents
that type of error, and then a subsequent CL to net/http will use it
instead of ==.
Updates golang/go#22091
Change-Id: I86f1e20704eee29b8980707b700d7a290107dfd4
Reviewed-on: https://go-review.googlesource.com/87297
Reviewed-by: Tom Bergan <tombergan@google.com>
That test makes a request with no body and receives a response with no
body. The client will receive a HEADERS frame with END_STREAM. The test
assumes that the stream is closed immediately on receipt of that HEADERS
frame, i.e., before RoundTrip returns.
This assumption was broken by https://golang.org/cl/70510, which made
stream closure asynchronous w.r.t. RoundTrip.
To fix TestCloseIdleConnections_h2 while preserving the intent of CL
70510, we break processHeaders into two cases:
1. The request has a body. In this case, END_STREAM puts the stream in a
half-closed-remote state, which means the connection is not
necessarily idle when RoundTrip returns (since the request body is
still being uploaded). In this case, we preserve the behavior from CL
70510.
2. The request does not have a body. In this case, END_STREAM puts the
stream in a closed state and we must close the stream before
returning from RoundTrip.
The following command passes when this CL is merged into net/http:
go test -count=100000 -run=TestCloseIdleConnections_h2 net/http
Updates golang/go#22413
Change-Id: Iff2a0685a636ad51bff380e86a42b0d0eea984e5
Reviewed-on: https://go-review.googlesource.com/80139
Run-TryBot: Tom Bergan <tombergan@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
In invalid response tests logger write error messages to stderr and spam
test output.
Since we know response are invalid in these tests we can safely discard
logger output.
Fixesgolang/go#22850
Change-Id: Id8c97be910f0cf7dbe2380ba632960364bc8478b
Reviewed-on: https://go-review.googlesource.com/80235
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
AFAICT, activeRes serves no real purpose. It is used in just two ways:
- To reduce the number of calls to closeIfIdle, which reduces the number
of acquires of cc.mu when there are many concurrent streams. I dug
through the CL history and could not find any benchmarks showing that
this is necessary.
- To avoid redundant calls to cs.bufPipe.CloseWithError(err) when a read
loop is shutdown. This is unnecessary, since redundant CloseWithError
calls are ignored.
Since there isn't a good reason to have activeRes, the simplest way to
fix the leak is to remove activeRes entirely.
Updates golang/go#21543
Change-Id: I1d1d2dc6c946425a2772c8bf71436707021ac269
Reviewed-on: https://go-review.googlesource.com/80137
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
