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>
We use uint8 (0-255, inclusive) to represent the RFC 7540 priorities
weight (1-256, inclusive). To account for the difference, we add 1 to
the uint8 weight value within sortPriorityNodeSiblingsRFC7540.
However, the addition was done before converting the uint8 type to
float. As a result, when provided a maximum weight value, overflow will
happen and will cause the scheduler to treat the maximum weight as a
minimum weight instead.
This CL fixes the issue by making sure the addition happens after the
type conversion.
Change-Id: I404e87e5ad85fa06d5fa49cda613c93ac8847bdc
Reviewed-on: https://go-review.googlesource.com/c/net/+/714742
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>
Avoid a state where we can have a MAX_STREAMS frame to send,
but do not send the frame for an indefinite amount of time.
Conn.appendStreamFrames writes stream-related frames to
the current packet. It also handles removing streams
from the Conn when we no longer need to track their state.
Removing streams can affect the frames we want to send.
In particular, we may want to send a MAX_STREAMS to the
peer indicating that it can open more streams because
we've closed out some of the existing ones.
Add MAX_STREAMS after removing streams, to ensure we
pick up any changes to the sent value before adding it.
This case doesn't show up in tests, because the test harness's
idleness detection causes the Conn's event loop to run and notice
the pending MAX_STREAMS frame. Changing tests to use
testing/synctest (a followup CL) causes the problem to
appear, because the event loop isn't run while the Conn
is idle.
Change-Id: Ia7394891317dae6ecfd529a9b3501ac082cb453e
Reviewed-on: https://go-review.googlesource.com/c/net/+/714481
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Damien Neil <dneil@google.com>
Reviewed-by: Nicholas Husin <husin@google.com>
Reviewed-by: Nicholas Husin <nsh@golang.org>
testConn.handshake runs through the initial QUIC handshake
and verifies that the connection under test sends an expected
sequence of handshake messages.
The last datagram in the handshake is sent by the client,
and contains an ACK for the last datagram sent by the server.
The client sends this ACK after max_ack_delay (25ms) passes, minus
the timer granularity (1ms). The timer granularity is a constant
containing the expected maximum delay between a timer event's
scheduled time and the timer actually firing.
The expected handshake datagram used by testConn.handshake contains
an ACK with an ACK Delay value of 25ms (max_ack_delay).
This doesn't account for the timer granularity adjustment.
However, since testConn.handshake advances time by 25ms rather than 24ms,
the test connection sends the ACK at the later time and includes a
larger ACK Delay value.
Fix testConn.handshake to sleep for the expected delay (24ms).
Fix the expected handshake datagram accordingly.
This all avoids test failures after switching this package to use
testing/synctest's fake clock, under which the connection sends
this ACK at the scheduled time rather than a time under direct
control of the test.
Change-Id: I1af6e02e02f6493758e41db45a46d06a65441a7b
Reviewed-on: https://go-review.googlesource.com/c/net/+/714480
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Nicholas Husin <husin@google.com>
Auto-Submit: Damien Neil <dneil@google.com>
Reviewed-by: Nicholas Husin <nsh@golang.org>
