Replace Transport's limit of 5 1xx responses with a limit based
on the maximum header size: The total size of all 1xx response
headers must not exceed the limit we use on the size of the
final response headers.
(This differs slightly from the corresponding HTTP/1 change,
which imposes a limit on all 1xx response headers *plus* the
final response headers. The difference isn't substantial,
and this implementation fits better with the HTTP/2 framer.)
When the user is reading 1xx responses using a Got1xxResponse
client trace hook, disable the limit: Each 1xx response is
individually limited by the header size limit, but there
is no limit on the total number of responses. The user is
responsible for imposing a limit if they want one.
For golang/go#65035
Change-Id: I9c19dbf068e0f580789d952f63113b3d21ad86fc
Reviewed-on: https://go-review.googlesource.com/c/net/+/615295
Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Damien Neil <dneil@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Very minor tweaks:
- Remove (c) pseudosymbol.
- Remove "All Rights Reserved."
- Change "Google Inc." (no longer exists) to "Google LLC".
[git-generate]
echo '
,s/\(c\) //
,s/ All rights reserved.//
,s/Google Inc./Google LLC/
w
q
' | sam -d LICENSE
Change-Id: Ibaa49e00dd08950a577e4343bfc574980d327995
Reviewed-on: https://go-review.googlesource.com/c/net/+/598579
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
CL 111835 changed Transport stream ID numbering to start at
stream 3 when AllowHTTP is set. This was based on a
misunderstanding:
When a connection upgrades an HTTP/1.1 request to HTTP/2,
the initial HTTP/1.1 request occupies stream 1.
However, Transport does not perform HTTP protocol upgrades.
When using a Transport to send unencrypted HTTP/2 requests,
the entire connection uses HTTP/2, the first request is
sent as HTTP/2, and there is no reason not to use stream 1
for this request.
Starting from stream 3 is mostly harmless,
but ClientConn.idleStateLocked assumes that client streams
start from 1. This causes it to misidentify new single-use
connections as having already sent a request (when AllowHTTP
is set), and therefore not suitable for use.
Revert to always starting stream IDs at 1.
Fixesgolang/go#67671
Change-Id: I97c89de4ae49623d916f9dbd200f8252d2fd4247
Reviewed-on: https://go-review.googlesource.com/c/net/+/591275
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
Client and server tests both write frames to a test connection
and read frames back. Frame reads are usually paired with
test expectations.
Unify the API used for frame reads/writes in tests.
Introduce a testConnFramer type that implements a common set
of read/write methods, and embed it in both client and server
test types.
Change-Id: I6927c43459ba24f150a21c058a92797754f82bf1
Reviewed-on: https://go-review.googlesource.com/c/net/+/586249
Reviewed-by: Jonathan Amsterdam <jba@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
clientStream.requestedGzip is set from clientStream.writeRequest,
and examined by clientConn.readLoop. I'm not sure if there's
any possible way for an actual data race to happen here in
practice, since the read loop should only examine the field
after the request is sent by writeRequest, but it's enough
for the race detector to complain.
Set the field in ClientConn.roundTrip instead, before
the clientStream has become accessible to any other goroutines.
No test, but a following CL has race detector failures without
this change.
Change-Id: Id30f1b95bcfcc35c213440e0e47cce3feaaff06d
Reviewed-on: https://go-review.googlesource.com/c/net/+/586245
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
When run with -coverspec, tests track which portions of the
specification they cover and the TestSpecCoverage test produces
an error if any sections lack test coverage.
This is a lovely idea, and perhaps we should resurrect it at
some point, but there is currently exactly one coverage
annotation, dating back to the first commit of this package.
Change-Id: I5d2d8a1032b783d113ed0982f7e97a3bd1c07a33
Reviewed-on: https://go-review.googlesource.com/c/net/+/586243
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
newServerTester is used to create an HTTP/2 server for testing.
It returns a *serverTester, which includes a number of methods
for sending frames to and reading frames from a server connection
under test.
Many tests also use newServerTester to simply start an
*httptest.Server. These tests pass an "optOnlyServer" to
indicate that they don't need a client connection to interact with.
They interact with the *httptest.Server, and use no methods or
fields of *serverTester.
Make a clearer distinction between test types.
Add a newTestServer function which starts and returns
an *httptest.Server.
This function replaces use of newServerTester with optOnlyServer.
Change-Id: Ia590c9b4dcc23c17e530b0cc273b9120965da11a
Reviewed-on: https://go-review.googlesource.com/c/net/+/586155
Reviewed-by: Jonathan Amsterdam <jba@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Rewrite the synchronization used in Transport tests.
This replaces the explicit synchronization, which requires
annotating every point where a goroutine in the code under test
may block, with implicit syncronization based on parsing
goroutine stacks to identify when all goroutines of interest are
blocked.
Change-Id: I02646e2752c359ed1b08126370a48f3d3c1fde77
Reviewed-on: https://go-review.googlesource.com/c/net/+/584895
Reviewed-by: Jonathan Amsterdam <jba@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Drop a trivial "gate" type used in exactly one location,
replacing it with an equivalent channel operation.
We've been using a "gate" with a different definition in
the quic package; dropping this hapax legomenon type from
the http2 package frees up the name if we want to use the
same gate here.
Change-Id: Id9c7d05daf7ee920c38090df960822fcc1168a4d
Reviewed-on: https://go-review.googlesource.com/c/net/+/584896
Reviewed-by: Jonathan Amsterdam <jba@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
The QUIC interop runner "keyrotate" test requires that the client
initiate a key rotation early in the connection. With our current
ack frequency, it seems that we need to rotate within the first
300-400 packets for the test to pass.
Reduce the initial key rotation from 1000 to 100 packets.
Rotating earlier shouldn't have any real downsides
(rotation is cheap and generally done once per connection,
except for very long-lived connections), and this is simpler
than providing a way to tune the rotation interval in one
specific test.
For golang/go#67138
Change-Id: I33d47ea35ed39f0a13c171adb2b0698f8c93050e
Reviewed-on: https://go-review.googlesource.com/c/net/+/582855
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
When a server sends a GOAWAY frame, it indicates the ID of the
last stream it processed. We use this to mark any requests after
that stream as being safe to retry on a new connection.
Change this to not retry the first request on a connection if we
get a GOAWAY with an error, even if the GOAWAY has a stream ID
of 0 indicating that it didn't process that request.
If we're getting an error as the first result on a new connection,
then there's either something wrong with the server or something
wrong with our request; either way, retrying isn't likely to be
productive and may be unsafe.
This matches the behavior of the HTTP/1 client, which
also avoids retrying the first request on a new connection.
For golang/go#66668Fixesgolang/go#60636
Change-Id: I90ea7cfce2974dd413f7cd8b78541678850376a5
Reviewed-on: https://go-review.googlesource.com/c/net/+/576895
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
When closing a connection because a stream contained a request we
didn't like (for example, because the request headers exceed
the maximum we will accept), set the LastStreamID in the GOAWAY
frame to include the offending stream. This informs the client
that retrying the request is unlikely to succeed, and avoids
retry loops.
This change requires passing the stream ID of the offending
stream from Framer.ReadFrame up to the caller. The most sensible
way to do this would probably be in the error. However,
ReadFrame currently returns a defined error type for
connection-ending errors (ConnectionError), and that type is a
uint32 with no place to put the stream ID. Rather than changing
the returned errors, ReadFrame now returns an error along with
a non-nil Frame containing the stream ID, when a stream is
responsible for a connection-ending error.
For golang/go#66668
Change-Id: Iba07ccbd70ab4939aa56903605474d01703ac6e4
Reviewed-on: https://go-review.googlesource.com/c/net/+/576756
Reviewed-by: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Auto-Submit: Damien Neil <dneil@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
This test causes the server to send a GOAWAY and close a connection.
The server GOAWAY path writes a GOAWAY frame asynchronously, and
closes the connection if the write doesn't complete within 1s.
This is causing failures on some builders, when the frame write
doesn't complete in time.
The important aspect of this test is that the connection be closed.
Drop the check for the GOAWAY frame.
Change-Id: I099413be9c4dfe71d8fe83d2c6242e82e282293e
Reviewed-on: https://go-review.googlesource.com/c/net/+/576235
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
