RoundTrip will now call httptrace.ClientTrace.Got1xxResponse, if any,
when receiving 1xx status response from a peer. This allows our client
and server to use HTTP 103 end-to-end.
Got100Continue and Wait100Continue have also been added to RoundTrip as
they are nearby. The rest of httptrace.ClientTrace will be added in the
future.
For golang/go#70914
Change-Id: Ia7ef7dd026a5390225149da3d76b06a2a372c009
Reviewed-on: https://go-review.googlesource.com/c/net/+/749265
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Nicholas Husin <husin@google.com>
TestRoundTripRequestBodyErrors/read_error currently has a small chance
of failing due to a race condition.
Most of the time, within writeBodyAndTrailer which runs concurrently,
when a request body returns an error on Read, rt.abort will be called
with the Read error. Since rt.abort is idempotent, the Read error will
then be propagated and the test would pass as expected.
However, before rt.abort is called with the Read error,
rt.reqBodyWriter.Close is first called, which will close the write
direction of the QUIC stream. This creates a small chance where the main
goroutine which runs RoundTrip can encounter an EOF error due to the
closed stream, and subsequently call rt.abort with a different error
than expected, causing test flakiness.
To fix this, this change makes sure that rt.abort is immediately called
when a request body Read returns an error. This should be a no-op, since
a body Read error has always been prioritized over a potential error
from rt.reqBodyWriter.Close anyways.
Fixesgolang/go#77782
Change-Id: I9ec5acf82dd95ac91963ce5c8ddbf08b49af9508
Reviewed-on: https://go-review.googlesource.com/c/net/+/751280
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Nicholas Husin <husin@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
This change makes it easier to move x/net/http2 into std.
Moving the http2 package into std and importing it from net/http
(rather than bundling it as net/http/h2_bundle.go) requires
removing the http2->net/http dependency. Moving tests into
the http2_test package allows them to continue importing net/http
without creating a cycle.
Change-Id: If0799a94a6d2c90f02d7f391e352e14e6a6a6964
Reviewed-on: https://go-review.googlesource.com/c/net/+/749280
Auto-Submit: Damien Neil <dneil@google.com>
Reviewed-by: Nicholas Husin <husin@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Nicholas Husin <nsh@golang.org>
Years back, our HTTP/2 server implementation had a stream accounting bug
that would cause it to improperly report a PROTOCOL_ERROR. In response
to this, we modified our Transport to retry RoundTrip when a RST_STREAM
with PROTOCOL_ERROR was received from a peer.
At this point, this retry logic had outlived its usefulness. Instead, it
might cause issues, e.g. a client that sends a malformed request will
keep retrying repeatedly, despite there being zero chance for the
request to actually succeed.
Fixesgolang/go#77843
Change-Id: Ic043723e3535f68f91db33d8f6bcd7fc2dbce856
Reviewed-on: https://go-review.googlesource.com/c/net/+/750720
Reviewed-by: Nicholas Husin <husin@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
The addition of FramePriorityUpdate (0x10) in RFC 9218 introduced a gap
in the frameParsers array indices (0x0a-0x0f). These indices were
initialized to nil, causing a panic when typeFrameParser accessed them
for unassigned frame types (e.g., ALTSVC 0x0a).
This change adds a nil check in typeFrameParser to safely fallback to
parseUnknownFrame for these unassigned types, preventing the crash.
Fixesgolang/go#77652
Change-Id: I14d7ad85afc1eafabc46417a9fff10f9e0a22446
Reviewed-on: https://go-review.googlesource.com/c/net/+/746180
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Damien Neil <dneil@google.com>
Auto-Submit: Damien Neil <dneil@google.com>
Reviewed-by: Mark Freeman <markfreeman@google.com>
This change implements support for Server to send trailer headers, and
for ClientConn to receive said trailer headers. This is just like
go.dev/cl/743600, but in the opposite direction.
The bulk of the implementation relies on the trailer header encoding and
decoding support that was added to bodyWriter and bodyReader
respectively in go.dev/cl/743600.
For golang/go#70914
Change-Id: I0efded4b1ac3e3c6b9479f18402e02e9e764d4a2
Reviewed-on: https://go-review.googlesource.com/c/net/+/744220
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Nicholas Husin <husin@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
This change adds support for ClientConn to send trailer headers, and for
Server to receive trailer headers. Support for the other direction will
be done in a follow-up change.
The encoding and decoding of trailer headers are done using bodyWriter
and bodyReader respectively, as these logic are agnostic to whether the
trailer header is sent by the client or the server.
For golang/go#70914
Change-Id: I646d193ae1bc44ddea69b8397d4473d3b11eddf2
Reviewed-on: https://go-review.googlesource.com/c/net/+/743600
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Nicholas Husin <husin@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
In HTTP/3, zero or more DATA frames can come after a HEADERS frame to
represent a request or response body. Our current implementation can
behave rather badly when zero DATA frame is sent.
ClientConn does not close the write direction of the stream when it has
no body to send. As a result, our Server can end up reading the next
frame after a HEADERS frame, only to hang infinitely until the timeout
is reached. To fix this, when there is no body to send, ClientConn now
closes the write direction of the stream as soon as it has finished
writing its HEADERS frame. Server will also prevent itself from reading
the stream if a Content-Length header with the value 0 is received.
In the opposite direction (client reading response from a server), a
similar problem also exists, with a slight variant. While our Server
reliably closes its write direction of the stream as soon as the server
handler exits, a problem can still occur when a client receives an empty
response body due to sending a HEAD request. In this case, if the client
decides to read the response body, bodyReader might throw an error due
to a mismatch between the Content-Length header given by the server and
the actual body length. This is fixed by making ClientConn aware that
HEAD requests will always result in an empty response body.
For golang/go#70914
Change-Id: I1e8970672e7076c9dbf84aec8808632d04bac807
Reviewed-on: https://go-review.googlesource.com/c/net/+/742960
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Nicholas Husin <husin@google.com>
While running net/http tests against our HTTP/3 implementation locally,
some tests fail due to slight behavior differences in responseWriter
compared to other http.ResponseWriter implementations:
- responseWriter does not return a 200 OK response if a server handler
is completely empty.
- responseWriter does not have a Flush method, and therefore does not
implement http.Flusher.
There are surely more differences, but these are straightforward to fix
right now.
For golang/go#70914
Change-Id: Ieb729a4de4ccb55d670eac2369e73c240b9ac8f8
Reviewed-on: https://go-review.googlesource.com/c/net/+/741720
Reviewed-by: Nicholas Husin <husin@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Damien Neil <dneil@google.com>
Transport currently deadlocks when receiving a WINDOW_UPDATE for a
non-zero stream that increments its window beyond the 2^31-1 bytes
limit.
This is because endStreamError is called to end the non-zero stream,
which tries to lock an already-locked mutex. Therefore, create and use
endStreamErrorLocked instead, which assumes the mutex is already locked.
Fixesgolang/go#77331
Change-Id: Iea212f49a1f305d1bddefb8831dbaca00840870c
Reviewed-on: https://go-review.googlesource.com/c/net/+/739700
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Nicholas Husin <husin@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
Currently, we are passing a very barebone http.Request to the server
handler: we only initialize an empty http.Request and put whatever info
we can get while decoding QPACK headers.
Unfortunately, this causes the Server to panic when parsing requests
whose headers are meant to be written to http.Request.URL, as
http.Request.URL was never initialized.
Therefore, make sure that http.Request.URL is initialized. Also,
populate other http.Request fields that we can easily figure out as of
now.
For golang/go#70914
Change-Id: Ie6552d6678b430fe4b51069616c0e366791c4e34
Reviewed-on: https://go-review.googlesource.com/c/net/+/738880
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Nicholas Husin <husin@google.com>
As part of adding support for HTTP/2 stream prioritization, a
DisableClientPriority field will be added to http.Server to allow users
to completely disable client prioritization if desired. When
DisableClientPriority is set to true, HTTP/2 server will revert back to
the old behavior where streams are processed in a round-robin manner.
For golang/go#75500
Change-Id: Ida083b3ac17a953e5ddb3ad7ab8a81f9cde2bfc1
Reviewed-on: https://go-review.googlesource.com/c/net/+/737521
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Nicholas Husin <husin@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
internal/http3 currently depends on internal/quic/quicwire only for its
SizeVarint when writing SETTINGS frame. This makes bundling
internal/http3 into std more complicated since it forces us to also
bundle internal/quic/quicwire and do import remapping. This CL breaks
this dependency for easier bundling.
Dependency on internal/quic/quicwire in test codes have been left alone
since they do not affect bundling into std, and uses more than just
SizeVarint.
Change-Id: I2b85c5487ae6cce422deb4509f68f932d3f0de6b
Reviewed-on: https://go-review.googlesource.com/c/net/+/738060
Reviewed-by: Nicholas Husin <husin@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Damien Neil <dneil@google.com>
To make sure that clients do not unnecessarily send RFC 7540 priority
signals when it would be treated as a no-op, this change makes it so
that our server always sends SETTINGS_NO_RFC7540_PRIORITIES in our
SETTINGS frame when our write scheduler is set to anything other than
the RFC 7540 write scheduler.
For golang/go#75500
Change-Id: I7a54251022087319999deda7efb663f8b251aa95
Reviewed-on: https://go-review.googlesource.com/c/net/+/729141
Reviewed-by: Damien Neil <dneil@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Nicholas Husin <husin@google.com>
In RFC 9218, streams are non-incremental by default, meaning that they
are processed one-by-one to completion. This behavior is the opposite of
our current default of handling streams in a round-robin manner.
This might cause a surprising behavior change once we make the RFC 9218
priority scheduler the default write scheduler for most users (we assume
that most users will not be sending RFC 9218 priority signals, at least
initially). To avoid surprising users with such a behavior change, this
CL makes it so that the streams are only made non-incremental once there
has been a clear signal that the end-user is aware of RFC 9218.
For golang/go#75500
Change-Id: Ibd22cb279c43de0190962904c3809007447a5fe3
Reviewed-on: https://go-review.googlesource.com/c/net/+/729140
Reviewed-by: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Nicholas Husin <husin@google.com>
RFC 9218 allows HTTP/2 stream priority to be set in two ways: via
PRIORITY_UPDATE frame and via header field. This change adds support for
the latter method.
As part of supporting priority adjustment via header field, this CL
also makes sure to look for the existence of an intermediary. If an
intermediary exists, default priority will be used for all streams to
ensure fairness between multiple clients who could be using the same
intermediary.
For golang/go#75500
For golang/go#75936
Change-Id: I6dc409b650fd52fa192d771a16b7a4ac5e51c9aa
Reviewed-on: https://go-review.googlesource.com/c/net/+/729120
Reviewed-by: Damien Neil <dneil@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Nicholas Husin <husin@google.com>
This change adds initial support for the PRIORITY_UPDATE frame
introduced in RFC 9218.
Clients can now use a new exported function to write PRIORITY_UPDATE
frames easily. However, sending PRIORITY_UPDATE frames to the server
does not currently cause any behavior changes: we only use
PRIORITY_UPDATE frames to adjust stream priority when the RFC 9218 write
scheduler is being used for a particular connection. However, this
scheduler is not currently usable yet from any configuration surfaces
exposed to the user.
For golang/go#75500
Change-Id: Ie2c821cb0d2faa6e942e209e11638f190fc98e2b
Reviewed-on: https://go-review.googlesource.com/c/net/+/705917
Reviewed-by: Nicholas Husin <husin@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Damien Neil <dneil@google.com>
internal/http3 was written back in go1.24, and relies on synctest.Run
and goexperiment.synctest. This prevents us from running tests with
newer versions of Go.
This CL updates the existing build constraint and synctest usages to
go1.25 so we can still run tests using go1.25 and tip. Support for
running synctest with go1.24 is not kept, since go1.26 release (and
therefore x/net go.mod updating to go1.25) is expected soon.
Change-Id: Iebfa82ebd1da4a06ba613ce0f45051f4691037fc
Reviewed-on: https://go-review.googlesource.com/c/net/+/734940
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Nicholas Husin <husin@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Within hybiClientHandshake, after getting a response from
http.ReadResponse, the body is never closed.
When running a debugger with all of our test cases, this seems to not
matter usually since the body is typically a http.noBody, whose Close
just returns nil. However, this is not always the case. Therefore, this
CL adds the missing resp.Body.Close call.
Fixesgolang/go#76952
Change-Id: I292d6ccc8eb101b806738aa7fc3e5446b623d861
Reviewed-on: https://go-review.googlesource.com/c/net/+/734240
Reviewed-by: Damien Neil <dneil@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Nicholas Husin <husin@google.com>
When sending a RST_STREAM for a canceled request, we sometimes send
a PING frame along with the reset to confirm that the server is responsive
and has received the reset.
Sending too many PINGs trips denial-of-service detection on some servers,
causing them to close a connection with an ENHANCE_YOUR_CALM error.
Do not send a PING frame along with an RST_STREAM if the connection
has displayed signs of life since the canceled request began.
Specifically, if we've received any stream-related frames since the
request was sent, assume the server is responsive and do not send a PING.
We still send a PING if a request is canceled and no stream-related
frames have been received from the server since the request was first
sent.
For golang/go#76296
Change-Id: I1be3532febf9ac99d65e9cd35346c02306db5f9d
Reviewed-on: https://go-review.googlesource.com/c/net/+/720300
Reviewed-by: Nicholas Husin <husin@google.com>
Reviewed-by: Nicholas Husin <nsh@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Permit net/http to create new HTTP/2 client connections.
We do this by adding a NewClientConn method to the type the HTTP/2 client
registers with net/http.Transport.RegisterProtocol, which creates a
persistent connection from a net.Conn.
No tests in this CL. Tests will be in net/http, and will cover
both the HTTP/1 and HTTP/2 paths for NewClientConn.
For golang/go#75772
Change-Id: Ib1a06b4d13fdd6008e5db9a090c6e9632029a2a4
Reviewed-on: https://go-review.googlesource.com/c/net/+/722200
Reviewed-by: Nicholas Husin <husin@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Nicholas Husin <nsh@golang.org>
