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>
This change introduces a new write scheduler that prioritizes writes
based on RFC 9218. Eventually, this scheduler will be used to replace
the existing priority scheduler based on RFC 7540, which has been
deprecated in RFC 9113.
No behavioral changes has been introduced as this scheduler is not used
anywhere yet.
goos: linux
goarch: amd64
pkg: golang.org/x/net/http2
cpu: AMD EPYC 7B13
BenchmarkWriteSchedulerThroughputRoundRobin-64 100000 140884 ns/op 139201 B/op 2900 allocs/op
BenchmarkWriteSchedulerLifetimeRoundRobin-64 100000 149632 ns/op 139202 B/op 2900 allocs/op
BenchmarkWriteSchedulerThroughputRandom-64 100000 218311 ns/op 139201 B/op 2900 allocs/op
BenchmarkWriteSchedulerLifetimeRandom-64 100000 216559 ns/op 139203 B/op 2900 allocs/op
BenchmarkWriteSchedulerThroughputPriorityRFC7540-64 100000 587625 ns/op 139201 B/op 2900 allocs/op
BenchmarkWriteSchedulerThroughputPriorityRFC9218Incremental-64 100000 149563 ns/op 139200 B/op 2900 allocs/op
BenchmarkWriteSchedulerLifetimePriorityRFC9218Incremental-64 100000 163697 ns/op 139201 B/op 2900 allocs/op
BenchmarkWriteSchedulerThroughputPriorityRFC9218NonIncremental-64 100000 145364 ns/op 139201 B/op 2900 allocs/op
BenchmarkWriteSchedulerLifetimePriorityRFC9218NonIncremental-64 100000 159316 ns/op 139203 B/op 2900 allocs/op
For golang/go#75500
Change-Id: Id5db195f6f75970f9cc3c7b7a292df96a139de8b
Reviewed-on: https://go-review.googlesource.com/c/net/+/704758
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 renames the file for the RFC 7540 priority write scheduler
to writesched_priority_rfc7540.go file. Existing symbols have also been
renamed to make it explicit that they are only used for the RFC 7540
priority implementation.
This is done so that when we introduce the new RFC 9218 priority write
scheduler, we will not cause confusion with regards to which symbols are
used for which scheduler.
This CL only renames and moves symbols, no behavior changes have been
introduced.
For golang/go#75500
Change-Id: I5c31bd51bc0d25415ff72909cc0b8f9fef44c052
Reviewed-on: https://go-review.googlesource.com/c/net/+/704757
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Split testClientConnClose into four separate tests,
rather than having one big function which performs
different tests depending on its parameter.
Use the more modern testClientConn in these tests,
for simplicity.
Drop the activeStreams function, which pokes a bit too
far into implementation internals and isn't necessary
for the new tests. (It had one use outside of
testClientConnClose, which provides little useful
signal and can also be dropped.)
Change-Id: Id8d1c7feab59c1f041bc2d1cf0398e8b1e230c69
Reviewed-on: https://go-review.googlesource.com/c/net/+/701005
Reviewed-by: Nicholas Husin <nsh@golang.org>
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>
This is a transport test. Use a testClientConn rather than a
Transport attached to a test server, for better control over
the frames sent to the test transport.
Drop one section of the test which pokes into the response
body's transportResponseBody type, as being too coupled to the
implementation internals.
Change-Id: I7bc7c7c756fd0c596424fab9a892dda8d9e89d1c
Reviewed-on: https://go-review.googlesource.com/c/net/+/701004
Reviewed-by: Nicholas Husin <nsh@golang.org>
Reviewed-by: Nicholas Husin <husin@google.com>
Auto-Submit: Damien Neil <dneil@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Rewrite the slowest test in the http2 package.
This test was added in CL 23812 to verify that a Transport does not
provide flow control window updates until the user consumes data
from a stream.
Rewrite the test to use the more modern testClientConn, which permits
precise examination of when the transport sends WINDOW_UPDATE frames.
Change-Id: Ibcde492549cad6363ce0e1a5ba169da7a4427d85
Reviewed-on: https://go-review.googlesource.com/c/net/+/700923
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Nicholas Husin <nsh@golang.org>
Reviewed-by: Nicholas Husin <husin@google.com>
Auto-Submit: Damien Neil <dneil@google.com>
Rewrite the slowest test in the http2 package.
This test was added in CL 23287 to verify two changes:
- A server handler calling req.Body.Close does not
kill the request stream.
- A Transport does not leak a goroutine if a request body is
still being written when the request stream is closed.
Split the test into two individual tests, one for the
server behavior and one for the transport.
Change-Id: I211f458e1001df435d00c2e1ebd7f3072e053c89
Reviewed-on: https://go-review.googlesource.com/c/net/+/700922
Reviewed-by: Nicholas Husin <nsh@golang.org>
Reviewed-by: Nicholas Husin <husin@google.com>
Auto-Submit: Damien Neil <dneil@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
It is often useful in server tests to orchestrate a sequence
of actions that involve both a server connection and request handler.
For example, we might want to have the request handler read from
the request body at a precise point in the test.
Add support for this to serverTester (used for most server tests).
Pass a nil handler to serverTester, and it will provide synchronous
access to the handler:
call := st.nextHandlerCall()
call.do(func(w http.ResponseWriter, r *http.Request) {
// this executes in the handler goroutine
})
Replace the existing handlerPuppet type, which provided a
similar mechanism but only worked on a single call at a time.
Change-Id: I023e032084f911ab4f9b803c393e4a55b12af87f
Reviewed-on: https://go-review.googlesource.com/c/net/+/701002
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Nicholas Husin <husin@google.com>
Auto-Submit: Nicholas Husin <nsh@golang.org>
Reviewed-by: Nicholas Husin <nsh@golang.org>
requestBody.Read avoids panicking when requestBody.conn is unset
and we are in tests. This seems like an excellent way to hide a
non-test panic. Drop the exceptional path.
This path was introduced in golang.org/cl/31636 to support a
test verifying that concurrently closing and reading from a
Request.Body does not race. Rewrite this test to use a real
server handler.
Change-Id: I778e78ff9ab45e248769557fff94d17940eb7a18
Reviewed-on: https://go-review.googlesource.com/c/net/+/701000
Auto-Submit: Nicholas Husin <nsh@golang.org>
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>
The test-only function serverTester.wantFlowControlConsumed
purports to check for the amount of connection or stream level
flow control consumed. Calling it with a streamID of 0
checks the connection-level flow control tokens.
However, the stream-level flow control path is unimplemented and unused.
Calling this function with a non-zero streamID doesn't work,
and (fortunately) all tests that use it only check connection-level
flow control.
Rename the function to wantConnFlowControlConsumed.
Change-Id: I1d934e8b46b4c43d393d102f1b2621329a7472aa
Reviewed-on: https://go-review.googlesource.com/c/net/+/700920
Auto-Submit: Nicholas Husin <nsh@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Nicholas Husin <husin@google.com>
Reviewed-by: Nicholas Husin <nsh@golang.org>
Replace ad-hoc pre-synctest synchronization with testing/synctest.
Many of the http2 package tests use a custom-built fake time
implementation and a idleness-detection mechanism based on parsing
goroutine stack dumps. Experience with this approach to testing
eventually led to the development of the testing/synctest package.
Switch over to testing/synctest.
The synctest package became available as an experiment in Go 1.24
(only when GOEXPERIMENT=synctest is set), and was fully released
with some API changes in Go 1.25.
- Use the released synctest API on Go 1.25.
- Use the experimental API (synctest.Run) on Go 1.24 when
GOEXPERIMENT=synctest is set. (Note that we set this on trybots.)
- Skip tests on Go 1.24 when GOEXPERIMENT=synctest is not set.
The x/net module requires go1.24, so older versions
can be disregarded.
Change-Id: Ifc13bfdd9bdada6c016730a78bd5972a5193ee30
Reviewed-on: https://go-review.googlesource.com/c/net/+/700996
Reviewed-by: Nicholas Husin <husin@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Damien Neil <dneil@google.com>
Reviewed-by: Nicholas Husin <nsh@golang.org>
Avoid using T.Context within a synctest bubble. If the Context's
Done channel is created outside the bubble, waiting on it within
the bubble is durably blocking. If it's created within the bubble,
the testing package encounters a panic when closing it after
CL 671960.
Instead, create our own Context within the bubble and cancel it
before the bubble is destroyed.
This will be entirely obviated by synctest.Test, which creates
a testing.T that returns a properly bubbled context.
Change-Id: Iff93c296ccbc1ece8172cb0a60e626ea1bd895ad
Reviewed-on: https://go-review.googlesource.com/c/net/+/675615
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Damien Neil <dneil@google.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
FrameType is a dense integer range, so we can store the frameParsers
in an array instead of a map. This should be a very small performance
win on all Go http2 servers. For high QPS gRPC services, this function
is visible in the Go profiler. For example, it shows up as 0.16% of
all CPU time on one production service at Datadog.
Change FrameType.String() to use the same pattern.
Add a test for testFrameType with unknown FrameTypes.
Fixesgolang/go#73613
Change-Id: I5f5b523e011a99d6b428cbdbfd97415e488169d1
Reviewed-on: https://go-review.googlesource.com/c/net/+/670415
Reviewed-by: Sean Liao <sean@liao.dev>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
An "optimistic ACK attack" involves an attacker sending ACKs
for packets it hasn't received, causing the victim's
congestion controller to improperly send at a higher rate.
The standard defense against this attack is to skip the occasional
packet number, and to close the connection with an error if the
peer ACKs an unsent packet.
Implement this defense, increasing the gap between skipped
packet numbers as a connection's lifetime grows and correspondingly
the amount of work required on the part of the attacker.
Change-Id: I01f44f13367821b86af6535ffb69d380e2b4d7b7
Reviewed-on: https://go-review.googlesource.com/c/net/+/664298
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
Auto-Submit: Damien Neil <dneil@google.com>
The sentPacket type tracks the state of a packet sent to the peer.
Packets can be in progress, acknowledged, or lost.
Track this state with an enum rather than a set of bools,
to avoid the possibility of nonsensical states such as a packet
being both acknowledged and lost.
This also simplifies a following change to add an "unsent" state
for intentionally skipped packet numbers.
Change-Id: I87c8fc399c72337c033ab7ec5ec8db2c56c732f9
Reviewed-on: https://go-review.googlesource.com/c/net/+/664297
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
Auto-Submit: Damien Neil <dneil@google.com>
When discarding keys for a packet number space, we remove any
in-flight packets in that space from our count of bytes in flight.
Skip any packets in the space that were already acked or declared lost.
Also skip over acked/lost packets when discarding state in response
to a Reset message. This change shouldn't have any effect, since we
never declare packets acked/lost until we receive an ACK, and we
never handle a Reset after receiving an ACK.
Change-Id: I8842f03aa06e57a834a211f1284a48fe5d469db3
Reviewed-on: https://go-review.googlesource.com/c/net/+/664335
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
Auto-Submit: Damien Neil <dneil@google.com>
Decoding QUIC packet numbers requires keeping track of the largest
packet number received so far from the peer. Our tests haven't
bothered doing that so far, so tests can't work with packet
numbers past 255.
Fix that so we can write tests that use more packets.
Change-Id: Icb795e5cf69794381c12a3a03b0da6bcf47a69c0
Reviewed-on: https://go-review.googlesource.com/c/net/+/664296
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
Auto-Submit: Damien Neil <dneil@google.com>
The tests in qpack_decode_test.go require synctest helpers from
http3_test.go, but that file has a goexperiment.synctest build
constraint.
To make builds work when GOEXPERIMENT=nosynctest is specified the
synctest helpers are refactored into http3_synctest_test.go (with the
same build constraint) and the non-synctest related functionality is
kept in http3_test.go.
Change-Id: Iae339dc1895f27e7ac5ba985e204f4868c229a4d
Reviewed-on: https://go-review.googlesource.com/c/net/+/660535
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Damien Neil <dneil@google.com>
