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>
This is a copy of the CL 510255 with the difference that
it preserves use of fs.ErrClosed when reading after close.
goos: darwin
goarch: arm64
pkg: golang.org/x/net/http2
cpu: Apple M4
│ HEAD~1 │ HEAD │
│ sec/op │ sec/op vs base │
ClientGzip-10 752.8µ ± 1% 750.7µ ± 2% ~ (p=0.393 n=10)
│ HEAD~1 │ HEAD │
│ B/op │ B/op vs base │
ClientGzip-10 75.49Ki ± 0% 33.98Ki ± 3% -54.99% (p=0.000 n=10)
│ HEAD~1 │ HEAD │
│ allocs/op │ allocs/op vs base │
ClientGzip-10 705.0 ± 0% 698.5 ± 0% -0.92% (p=0.000 n=10)
Updates golang/go#61353
Change-Id: I0fdc0c0a5947d27dcc615e5bcf4d4620c2c95d9e
GitHub-Last-Rev: 4d04dc93f3
GitHub-Pull-Request: golang/net#239
Reviewed-on: https://go-review.googlesource.com/c/net/+/710235
Reviewed-by: Sean Liao <sean@liao.dev>
Reviewed-by: Michael Pratt <mpratt@google.com>
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>
The QUIC interop test suite confirms support for ChaCha20. Go's TLS
implementation doesn't allow configuring ciphersuites for TLS 1.3, so we
cannot force the client hello to offer only ChaCha20 as the test
requires.
When acting as a server, we still cannot control which ciphersuites we
offer, but we can make the binary choice of whether we respond to the
client hello (which includes its offer of ciphersuites).
Use that to implement the server side of the ChaCha20 interop test. This
tells a more complete story of our level of ChaCha20 support: it works
when negotiated.
Fixesgolang/go#75912
Change-Id: I1d8d08e4f4b8eb89bf11e9e4ae1aaa5c0709a530
Reviewed-on: https://go-review.googlesource.com/c/net/+/712120
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Damien Neil <dneil@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
This change implements the proposal to add new DNS message types HTTPS and
SVCB in the golang.org/x/net/dns/dnsmessage package, as described in
golang/go#43790.
The implementation includes:
- New types TypeHTTPS and TypeSVCB.
- SVCBResource and HTTPSResource structs, with HTTPSResource embedding
SVCBResource.
- SVCParam and SVCParamKey types for handling service parameters.
- pack and unpack methods for the new resource types.
- Integration into the Parser and Builder.
- Comprehensive tests, including for parameter handling logic.
I implemented the SVCB parsing code so that it performs only two
allocations: one for the []SVCParam slice, and one to hold the SVCParam
values. A test was added to demonstrate that.
Fixesgolang/go#43790
Change-Id: I60439772fe0e339ae3141bd1dd9829564efe0f2a
GitHub-Last-Rev: 49c2ac0102
GitHub-Pull-Request: golang/net#241
Reviewed-on: https://go-review.googlesource.com/c/net/+/710736
Reviewed-by: Damien Neil <dneil@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Vinicius Fortuna <fortuna@google.com>
This change exports two new methods on the Framer, ReadFrameHeader and
ReadFrameBodyForHeader, which split the functionality of the existing
ReadFrame method.
This provides more granular control, allowing callers to inspect the
frame header before deciding whether or how to read the frame body.
This is useful for applications that may need to make decisions based on frame
type.
Fixesgolang/go#73560
Change-Id: I60b42d2889095fac8e243022886740bc6dd94012
Reviewed-on: https://go-review.googlesource.com/c/net/+/710515
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>
TestGoroutineLock sets DebugGoroutines = true.
When a previous test leaves a server running after exiting,
this write to DebugGoroutines can race with reads from the server.
Obviously tests shouldn't leave goroutines around after they exit,
but it happens and when it does it can show up here as a rare
and hard-to-debug flake.
DebugGoroutines is always true in tests, so there's no need to set
it here. Just leave it alone.
Fixesgolang/go#75811
Change-Id: Iebeab2a22642cbd6867b9f4f5a171c91ea697b17
Reviewed-on: https://go-review.googlesource.com/c/net/+/710675
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Nicholas Husin <nsh@golang.org>
Reviewed-by: Nicholas Husin <husin@google.com>
Auto-Submit: Damien Neil <dneil@google.com>
Reviewed-by: Nicholas Husin <nsh@golang.org>
Our previous implementation of writeQueue relies on one
[]FrameWriteRequests, forcing us to copy the rest of the slice's content
whenever we remove an item from the front.
This change remedies this problem by implementing writeQueue using
two-stage queues, similar to Okasaki's purely functional queue.
With 25 frames per stream, we are observing the following performance
improvement:
goos: linux
goarch: amd64
pkg: golang.org/x/net/http2
cpu: AMD EPYC 7B13
│ /tmp/old │ /tmp/new │
│ sec/op │ sec/op vs base │
WriteQueue-64 508.3n ± 3% 305.7n ± 3% -39.86% (p=0.000 n=10)
│ /tmp/old │ /tmp/new │
│ B/op │ B/op vs base │
WriteQueue-64 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=10) ¹
¹ all samples are equal
│ /tmp/old │ /tmp/new │
│ allocs/op │ allocs/op vs base │
WriteQueue-64 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=10) ¹
¹ all samples are equal
As the number of frames increases, the performance difference becomes
more stark as the old implementation does a quadratic amount of copying
in total to be able to fully consume a queue.
Change-Id: Ide816ebdd89a41275b5829683c0f10d48321af50
Reviewed-on: https://go-review.googlesource.com/c/net/+/710635
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>
The HTML specification contains a number of algorithms which are
quadratic in complexity by design. Instead of adding complicated
workarounds to prevent these cases from becoming extremely expensive in
pathological cases, we impose a limit of 512 to the size of the stack of
open elements. It is extremely unlikely that non-adversarial HTML
documents will ever hit this limit (but if we see cases of this, we may
want to make the limit configurable via a ParseOption).
Thanks to Guido Vranken and Jakub Ciolek for both independently
reporting this issue.
Fixes CVE-2025-47911
Fixesgolang/go#75682
Change-Id: I890517b189af4ffbf427d25d3fde7ad7ec3509ad
Reviewed-on: https://go-review.googlesource.com/c/net/+/709876
Reviewed-by: Damien Neil <dneil@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Previously, the RFC 9218 write scheduler had a bug where AdjustStream()
did not update the stream's metadata after adjusting its priority. This
results in the function not being idempotent, where repeated calls to it
for the same stream can instead remove an unrelated stream from our
scheduler, and duplicate the stream whose priority is being updated.
For go/golang#75500
Change-Id: Iaf3dd819d02839bc6cff65027c4916f9f2fa3e5b
Reviewed-on: https://go-review.googlesource.com/c/net/+/709477
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>
This change introduces parsing functions for all item types defined in
RFC 8941, namely: integers, decimals, strings, tokens, byte sequences,
and booleans.
At this point, internal/httpsfv should be usable for parsing any RFC
8941-compliant HTTP Structured Field Values.
In a future CL, we will add support for parsing display strings and
dates, so that this package fully supports RFC 9651.
For golang/go#75500
Change-Id: Ib8ad2caa5f6ea4285d00506faa4b8127c2cc9419
Reviewed-on: https://go-review.googlesource.com/c/net/+/708435
Auto-Submit: Nicholas Husin <nsh@golang.org>
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>
This CL adds consumeDisplayString() and consumeDate() function, meaning
that we can now consume all types that are defined within RFC 9651. In
future CL, we will add the corresponding parsing function for all the
types, so callers of this package will not have to implement their own
parsing / formatting.
For golang/go#75500
Change-Id: I90aa132d3ab1385b310d821997da13a095cd71bc
Reviewed-on: https://go-review.googlesource.com/c/net/+/708015
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>
This change implements the Parse functions for the Dictionary and List
type. At this point, we should be able to use internal/httpsfv package
to extract information from any HTTP SFV that follows RFC 8941.
In future changes, we will add additional types introduced in RFC 9651
to achieve feature parity with it. Additionally, we will add Parse
functions for all the HTTP SFV types, such that users of the package do
not need to do their own type assertions and conversions.
Note that the Dictionary and List type do not have a consume function.
This is because both types never appear as a child of other types,
meaning it is guaranteed to always consume its entire string input.
For go/golang#75500
Change-Id: I376dca274d920a4bea276ebb4d49a9cd768c79fe
Reviewed-on: https://go-review.googlesource.com/c/net/+/707100
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 implements the consume and Parse functions for both the Item
and Bare Inner List type. This is part of a chain of changes that is needed
in order for us to fully support HTTP Structured Field Values parsing as
defined in RFC 9651.
In future changes, we will utilize the support for Bare Inner List and Item
that is added here to support more complex types, namely Dictionary and
List.
Note that Bare Inner List is something we define on our own. We define a
Bare Inner List as an Inner List without the top-most parameter meant
for the Inner List. For example, the Inner List `(a;b c;d);e` would
translate to the Bare Inner List `(a;b c;d)`. We have done this because
the parameter of an Inner List will be exposed to the user via
ParseDictionary() or ParseList() too. By implementing Bare Inner List,
we can avoid having two ways of accessing the Inner List parameter, and
incurring the cost of a more complex implementation for Inner List and
other types that utilize Inner List (e.g. if we have consumeInnerList,
ParseDictionary will have to use consumeInnerList and backtrack the
consumption to separate out the InnerList parameter).
For go/golang#75500
Change-Id: I9b418d10b5755195d1cc3ff5f7ea211423bc4b48
Reviewed-on: https://go-review.googlesource.com/c/net/+/707099
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>
This change implements the minimum set of functionality within RFC 8491
that is needed in order for us to be able to extract information out of
Parameters type.
Rather than parsing the given Structured Field Values as usual, we
instead allow users to give us functions that will be invoked as we walk
through the SFV. This allows users to still extract information out of
SFV, without incurring significant memory allocation, especially when
the input is large.
If the current API & approach is good, we will proceed further by
implementing walk functionality for the rest of the types within RFC
8491: Dictionary, List, Item, and Inner List. After that, we will also
add support for Date and Display String to fully support RFC 9651.
For golang/go#75500
Change-Id: I838a7267a54fcd64b019be0ac10fe86b1e3e2c8b
Reviewed-on: https://go-review.googlesource.com/c/net/+/706755
Auto-Submit: Nicholas Husin <nsh@golang.org>
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>
