http://httpwg.org/specs/rfc7540.html#rfc.section.6.1 says:
> The entire DATA frame payload is included in flow control, including
> the Pad Length and Padding fields if present.
But we were just ignoring the padding and pad length, which could lead
to clients and servers getting out of sync and deadlock if peers used
padding. (Go never does, so it didn't affect Go<->Go)
In the process, fix a lingering bug from golang/go#16481 where we
didn't account for flow control returned to peers in the stream's
inflow, despite sending the peer a WINDOW_UPDATE.
Fixesgolang/go#16556
Updates golang/go#16481
Change-Id: If7150fa8f0da92a60f34af9c3f754a0346526ece
Reviewed-on: https://go-review.googlesource.com/25382
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Gerrand <adg@golang.org>
For both the server and the transport, return connection-level flow control in
two cases: 1) when a stream is closed with buffered data not read by the user,
or 2) when a DATA frame arrives but there the stream has since been closed.
Fixesgolang/go#16481
Change-Id: Ic7404180ed04a2903e8fd6e9599a907f88b4f72e
Reviewed-on: https://go-review.googlesource.com/25231
Reviewed-by: Andrew Gerrand <adg@golang.org>
NewClientConn starts a goroutine (which reads cc.singleUse at start),
so it's not safe to set cc.singleUse after calling NewClientConn.
Change-Id: I5c28db15e8e8fd8ec5ef0e04b7f81e61efbd873e
Reviewed-on: https://go-review.googlesource.com/25230
Reviewed-by: Andrew Gerrand <adg@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Fixes flakes like:
https://build.golang.org/log/56e0561f9f6a740b75538692a1c3288f08949dd0
I reproduced the flake above with a time.Sleep(50 * time.Millisecond)
before ClientConn.RoundTrip's final for{select{...}} loop (to give
multiple channels time to become readable on the first select), and
then ran (on linux/amd64) with:
go test -v -run=TestTransportReqBodyAfterResponse_403 -count=50 -race
Before, many failed. With this CL it never fails. (crosses fingers)
Fixesgolang/go#16102 (again)
Change-Id: I93fde46846cd09445e53c140e9668382965d95c5
Reviewed-on: https://go-review.googlesource.com/24984
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Gerrand <adg@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The test's server was replying with the stream closure before reading
the test client's request body. This should in theory be handled
better (and golang/go#16029 will be kept open to track it), but that's not
what this test was trying to test anyway.
Instead, make the test do things in the expected order so it can
continue to test its original subject reliably. (It was trying to test
all the orders of optional & optionally fragmented headers in responses.)
Updates golang/go#16029 (flaky http2 TestTransportResPattern_*)
Updates golang/go#11811 (subrepos need to be green)
Change-Id: I631730fce5dad598120bb2d1ea8895e39255d711
Reviewed-on: https://go-review.googlesource.com/24970
Reviewed-by: Andrew Gerrand <adg@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The old support for Request.Close was optimistic, but there were still
windows where two connections should share the same
connection. Instead, support it earlier and more explicitly.
This fixes a flaky test on FreeBSD which was probably only flaky due
to unfortunate scheduling/timing.
Change-Id: I2eed9e59116356257e1b7df7a034b1c5d5f6533a
Reviewed-on: https://go-review.googlesource.com/24810
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Also drop some leading zeroes in in the generated table.go file. Its
size drops from 140K to 124K. After gzipping, this is from 46K to 45K.
It's not a huge win, but it's a win.
Change-Id: Ib1c13fb8b58b4eca231b5d74c15297f245c62e7c
Reviewed-on: https://go-review.googlesource.com/24753
Reviewed-by: Andrew Gerrand <adg@golang.org>
TLS_RSA_WITH_AES_128_GCM_SHA256 and TLS_RSA_WITH_AES_256_GCM_SHA384
are also blacklisted in RFC 7540 Appendix A.
Because they are were implemented in Go 1.6, the isBadCipher
function has been moved into the version-tagged files so that the
repo doesn't break for Go 1.5 users.
Change-Id: If8ca058ede51e9a4a337ce8d645cfd9f370d49b4
Reviewed-on: https://go-review.googlesource.com/24214
Reviewed-by: Adam Langley <agl@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The http2 spec permits multiple GOAWAY frames if the conditions
change. Merge their error messages together, preferring the first
non-empty debug data, and first non-ErrCodeNo error code.
Updates golang/go#14627
Change-Id: I073b9234d71f128ed0713f09a24c728b56d7c1ae
Reviewed-on: https://go-review.googlesource.com/24600
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Andrew Gerrand <adg@golang.org>
Also follow the "go generate" convention, and have the generation
command be a bare "go run gen.go" without any need for explicit flags.
Stripping the for-debugging comments shrinks the generated table.go file
from 538K to 139K.
Also update table to latest list from publicsuffix.org:
revision ea9f4f9c51f5bfc5c85a41a5637ab671c07c542a
(2016-05-31T18:30:56Z)
Change-Id: Ib59366106e0142fd65a2102e73d6a9a29e53930e
Reviewed-on: https://go-review.googlesource.com/23930
Reviewed-by: Andrew Gerrand <adg@golang.org>
The contract for Read specifies that a caller should process n bytes of
data before handling an error value. The previous implementation may
have discarded the final data read from the underlying reader if err
was non nil.
Change-Id: I81547cd9ea62c4060f79523679beed6bbc6ccd95
Reviewed-on: https://go-review.googlesource.com/23534
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The existing transport implementation limit scheme to https, so
simple h2c can not be used, if we loose the limit to http scheme,
it works well with existing h2c server support.
Fixesgolang/go#15830
Change-Id: Icfe65d72b3e6fb41f69019539fb5b86144814378
Reviewed-on: https://go-review.googlesource.com/23181
Reviewed-by: Andrew Gerrand <adg@golang.org>
Run-TryBot: Andrew Gerrand <adg@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Since Go's HTTP/2 Handlers can read & write concurrently, it was
previously possible for a Handler to get an HTTP Request with "Expect:
100-continue" and to first Write some data, then read some data, and
the read of the data was triggering the server's automatic
"100-continue" response path. But if the server had already replied
with a non-100 HEADERS frame, sending a HEADERS frame with :status 100
later is in violation of the spec.
Fix it by tracking whether we've already replied with something, and
ignoring the 100-continue when it's inappropriate.
Updates golang/go#14030
Change-Id: I6745ba0f257a31eaf63816e4263cadbef5a896a2
Reviewed-on: https://go-review.googlesource.com/23311
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Go 1.6+ wires up its http.Transport values to http2 with ConfigureTransport.
ConfigureTransport sets the connection pool to a "noDialClientConnPool"
type. But the Transport.CloseIdleConnections implementation was specifically
looking for a concrete type. Look for an interface instead.
Updates golang/go#14607 (needs bundle into the main repo before fix)
Change-Id: I6ad23b6facab5d3c2cbe71a1809a52794375d803
Reviewed-on: https://go-review.googlesource.com/23258
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Gerrand <adg@golang.org>
In Go 1.6, the HTTP/1 client got Transport.ExpectContinueTimeout.
This makes the HTTP/2 client respect a Request's "Expect:
100-continue" field and the Transport.ExpectContinueTimeout
configuration.
This also makes sure to call the traceWroteRequest hook if the server
replied while we're still writing the request, since that code was
in the same spot and it couldn't be trivially separated.
Updates golang/go#13851 (fixed after integrating it into std)
Updates golang/go#15744
Change-Id: I67dfd68532daa6c4a0c026549c6e5cbfce50e1ea
Reviewed-on: https://go-review.googlesource.com/23235
Reviewed-by: Andrew Gerrand <adg@golang.org>
The http2.Transport was able to send bogus header keys & values.
This changes rejects them earlier, before they hit the wire.
In the process, mirror the lexical rules from the http package to x/net.
Maintaining two copies has gotten increasingly annoying.
Updates golang/go#14048
Change-Id: I20abcdeea92e7dc8706a1bbd60688ee8843a2b12
Reviewed-on: https://go-review.googlesource.com/23229
Reviewed-by: Andrew Gerrand <adg@golang.org>
This change introduces a package that provides the basic manipulation of
routing facilities on BSD variants.
Unlike the existing APIs in syscall package of standard library, the
package tries to provide operating system and its architecture agnostic
APIs. At present, the package supports any version of Darwin, any
version of DragonFly BSD, FreeBSD 7 through 11, NetBSD 6 and above, and
OpenBSD 5.6 and above.
Updates golang/go#14724.
Change-Id: Id964ea22dec491ddac3776e3a8c8c10f140f96ac
Reviewed-on: https://go-review.googlesource.com/22446
Run-TryBot: Mikio Hara <mikioh.mikioh@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This change fixes a few bugs in the HPACK decoder:
* Excess trailing padding is treated as an error per the HPACK Spec
section 5.2
* Non EOS prefix padding is treated as an error
* Max length is now enforced for all decoded symbols
The idea here is to keep track of the decoded symbol length, rather
than the number of unconsumed bits in cur. To this end, nbits has
been renamed cbits (cur bits), and sbits (sym bits) has been
introduced. The main problem with using nbits is that it can easily
be zero, such as when decoding {0xff, 0xff}. Using a clear moniker
makes it easier to see why checking cbits > 0 at the end of the
function is incorrect.
Fixesgolang/go#15614
Change-Id: I1ae868caa9c207fcf9c9dec7f10ee9f400211f99
Reviewed-on: https://go-review.googlesource.com/23067
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Go 1.7 introduces Request.Context and Response.Uncompressed. Use them
both in the http2 Transport when building for Go 1.7.
Tests are in the main repo.
Updates golang/go#15366
Updates golang/go#15134
Change-Id: I4b78a2710b6cf30c769c7f85cea15ecde653a317
Reviewed-on: https://go-review.googlesource.com/23002
Reviewed-by: Andrew Gerrand <adg@golang.org>
