nick/amethyst
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 1 Wiki Activity

chore(quic): audit cleanup — drop redundant copy, rename queue, extract test fixture

Browse Source 
Three small follow-ups from the audit pass:

1. Drop redundant `challengeData.copyOf()` in
   `queuePathResponseLocked` — the parser produces a fresh
   ByteArray per PATH_CHALLENGE via `QuicReader.readBytes`'s
   `copyOfRange`, so the defensive copy was a wasted allocation.
   One-line fix.

2. Rename `pendingPathResponses` → `pendingPathChallengePayloads`.
   The queue holds inbound CHALLENGE payloads we owe RESPONSES
   for — old name conflated the two. Pure rename across
   QuicConnection / Parser / Writer / PathValidationTest.

3. Extract shared `newConnectedClient(...)` test fixture
   (`ConnectedClientFixture.kt`). The 6 test files each repeated
   ~40 lines of identical handshake-pipe boilerplate; folded into
   one parameterized helper accepting transport-cap overrides.
   Net −164 lines across the test tree; per-test helper is now a
   one-liner that documents the cap shape.

No behavior change. Full quic suite + amethyst hook test green.

https://claude.ai/code/session_018KPKWRg5baX5Anf7zfEyec
This commit is contained in:
Claude
2026-05-08 00:03:47 +00:00
parent afe3aaf020
commit 71e14fe639
10 changed files with 146 additions and 268 deletions
Download Patch File Download Diff File Expand all files Collapse all files
quic/src/commonMain/kotlin/com/vitorpamplona/quic/connection/QuicConnection.kt
+9 -8
View File
@@ -503,7 +503,7 @@ class QuicConnection(
*
* Caller must hold [streamsLock] for any read/write.
*/
internal val pendingPathResponses: ArrayDeque<ByteArray> = ArrayDeque()
internal val pendingPathChallengePayloads: ArrayDeque<ByteArray> = ArrayDeque()
/**
* RFC 9002 RTT estimator + loss-detection algorithm. Single
@@ -1896,12 +1896,13 @@ class QuicConnection(
* Caller must hold [streamsLock].
*/
internal fun queuePathResponseLocked(challengeData: ByteArray) {
if (pendingPathResponses.size >= MAX_PENDING_PATH_RESPONSES) return
// Defensive copy: the parser hands us a slice of the inbound
// packet's plaintext payload, which the parser may free /
// reuse after we return. Copying preserves the bytes for the
// writer to encode later.
pendingPathResponses.addLast(challengeData.copyOf())
if (pendingPathChallengePayloads.size >= MAX_PENDING_PATH_RESPONSES) return
// The parser produces a fresh ByteArray per PATH_CHALLENGE
// (see [com.vitorpamplona.quic.Buffer.QuicReader.readBytes],
// which `copyOfRange`s a slice off the inbound payload), so
// we can keep the reference directly without a defensive
// copy.
pendingPathChallengePayloads.addLast(challengeData)
}
/**
@@ -2122,7 +2123,7 @@ class QuicConnection(
const val RETIRED_STREAM_ID_RING_SIZE: Int = 4_096
/**
* Bound on the [pendingPathResponses] queue. RFC 9000 §8.2
* Bound on the [pendingPathChallengePayloads] queue. RFC 9000 §8.2
* doesn't cap PATH_CHALLENGE rate, so a malicious peer could
* spam them to exhaust our memory. 64 entries × 8 bytes = 512 B
* worst case — trivial to absorb but tight enough that an
quic/src/commonMain/kotlin/com/vitorpamplona/quic/connection/QuicConnectionParser.kt
+1 -1
View File
@@ -716,7 +716,7 @@ private fun dispatchFrames(
// RFC 9000 §8.2.2 — peer is validating that a path is
// alive. We MUST echo the SAME 8-byte payload in a
// PATH_RESPONSE on the path the challenge arrived on.
// The writer drains [pendingPathResponses] on the next
// The writer drains [pendingPathChallengePayloads] on the next
// application-level packet build.
//
// Common practical trigger: server-side path
quic/src/commonMain/kotlin/com/vitorpamplona/quic/connection/QuicConnectionWriter.kt
+2 -2
View File
@@ -957,8 +957,8 @@ private fun appendFlowControlUpdates(
// packet is lost, the peer's next PATH_CHALLENGE retry queues a
// fresh entry here and we respond again. The peer is responsible
// for retrying its challenge until it sees a matching response.
while (conn.pendingPathResponses.isNotEmpty()) {
val data = conn.pendingPathResponses.removeFirst()
while (conn.pendingPathChallengePayloads.isNotEmpty()) {
val data = conn.pendingPathChallengePayloads.removeFirst()
frames += PathResponseFrame(data)
}
quic/src/commonTest/kotlin/com/vitorpamplona/quic/connection/CloseUnderLoadTest.kt
+8 -44
View File
@@ -23,8 +23,6 @@ package com.vitorpamplona.quic.connection
import com.vitorpamplona.quic.frame.AckFrame
import com.vitorpamplona.quic.frame.StreamFrame
import com.vitorpamplona.quic.stream.StreamId
import com.vitorpamplona.quic.tls.InProcessTlsServer
import com.vitorpamplona.quic.tls.PermissiveCertificateValidator
import kotlinx.coroutines.async
import kotlinx.coroutines.awaitAll
import kotlinx.coroutines.coroutineScope
@@ -298,49 +296,15 @@ class CloseUnderLoadTest {
}
}
// 1024-stream caps so the 100-bidi-stream open burst doesn't
// brush the cap mid-test, and 16 MiB data window so multi-payload
// traffic doesn't trip flow-control mid-close.
private fun newConnectedClient(): Pair<QuicConnection, InMemoryQuicPipe> =
runBlocking {
val client =
QuicConnection(
serverName = "closeunderload.test",
config =
QuicConnectionConfig(
initialMaxStreamsBidi = 1024,
initialMaxStreamsUni = 1024,
initialMaxData = 16L * 1024 * 1024,
initialMaxStreamDataBidiLocal = 64L * 1024,
initialMaxStreamDataBidiRemote = 64L * 1024,
initialMaxStreamDataUni = 64L * 1024,
),
tlsCertificateValidator = PermissiveCertificateValidator(),
)
val serverScid = ConnectionId.random(8)
val tlsServer =
InProcessTlsServer(
transportParameters =
TransportParameters(
initialMaxData = 16L * 1024 * 1024,
initialMaxStreamDataBidiLocal = 64L * 1024,
initialMaxStreamDataBidiRemote = 64L * 1024,
initialMaxStreamDataUni = 64L * 1024,
initialMaxStreamsBidi = 1024,
initialMaxStreamsUni = 1024,
initialSourceConnectionId = serverScid.bytes,
originalDestinationConnectionId = client.destinationConnectionId.bytes,
).encode(),
)
val pipe =
InMemoryQuicPipe(
client = client,
initialDcid = client.destinationConnectionId.bytes,
serverScid = serverScid,
tlsServer = tlsServer,
)
client.start()
pipe.drive(maxRounds = 16)
assertEquals(QuicConnection.Status.CONNECTED, client.status)
client to pipe
}
com.vitorpamplona.quic.connection.newConnectedClient(
maxStreamsBidi = 1024,
maxStreamsUni = 1024,
maxData = 16L * 1024 * 1024,
)
}
/**
quic/src/commonTest/kotlin/com/vitorpamplona/quic/connection/ConnectedClientFixture.kt
+94
View File
@@ -0,0 +1,94 @@
/*
* Copyright (c) 2025 Vitor Pamplona
*
* Permission is hereby granted, free of charge, to any person obtaining a copy of
* this software and associated documentation files (the "Software"), to deal in
* the Software without restriction, including without limitation the rights to use,
* copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the
* Software, and to permit persons to whom the Software is furnished to do so,
* subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in all
* copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS
* FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
* COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN
* AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
* WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
*/
package com.vitorpamplona.quic.connection
import com.vitorpamplona.quic.tls.InProcessTlsServer
import com.vitorpamplona.quic.tls.PermissiveCertificateValidator
import kotlinx.coroutines.runBlocking
import kotlin.test.assertEquals
/**
* Stand up a fresh [QuicConnection] wired through an
* [InMemoryQuicPipe] and drive the handshake to CONNECTED. Most of
* the audio-rooms tests start from this exact shape — extracted
* here to keep each test file focused on its own assertions
* rather than ~40 lines of identical fixture boilerplate.
*
* The transport-parameter knobs cover the only variation real tests
* need:
* - moq-lite-shaped tests (many peer-uni streams, large data
* window): pass `maxStreamsUni = 65_536`, `maxData = 16 MiB`.
* - single-stream / control-frame tests (defaults are plenty).
*
* Both client and server advertise the same caps so flow-control
* regressions surface as caps-mismatch failures rather than tests
* passing accidentally because ONE side was generous.
*/
fun newConnectedClient(
serverName: String = "example.test",
maxStreamsBidi: Long = 16,
maxStreamsUni: Long = 16,
maxData: Long = 1L * 1024 * 1024,
maxStreamData: Long = 64L * 1024,
handshakeRounds: Int = 16,
): Pair<QuicConnection, InMemoryQuicPipe> =
runBlocking {
val client =
QuicConnection(
serverName = serverName,
config =
QuicConnectionConfig(
initialMaxStreamsBidi = maxStreamsBidi,
initialMaxStreamsUni = maxStreamsUni,
initialMaxData = maxData,
initialMaxStreamDataBidiLocal = maxStreamData,
initialMaxStreamDataBidiRemote = maxStreamData,
initialMaxStreamDataUni = maxStreamData,
),
tlsCertificateValidator = PermissiveCertificateValidator(),
)
val serverScid = ConnectionId.random(8)
val tlsServer =
InProcessTlsServer(
transportParameters =
TransportParameters(
initialMaxData = maxData,
initialMaxStreamDataBidiLocal = maxStreamData,
initialMaxStreamDataBidiRemote = maxStreamData,
initialMaxStreamDataUni = maxStreamData,
initialMaxStreamsBidi = maxStreamsBidi,
initialMaxStreamsUni = maxStreamsUni,
initialSourceConnectionId = serverScid.bytes,
originalDestinationConnectionId = client.destinationConnectionId.bytes,
).encode(),
)
val pipe =
InMemoryQuicPipe(
client = client,
initialDcid = client.destinationConnectionId.bytes,
serverScid = serverScid,
tlsServer = tlsServer,
)
client.start()
pipe.drive(maxRounds = handshakeRounds)
assertEquals(QuicConnection.Status.CONNECTED, client.status)
client to pipe
}
quic/src/commonTest/kotlin/com/vitorpamplona/quic/connection/KeyUpdatePeerInitiatedTest.kt
+4 -44
View File
@@ -24,8 +24,6 @@ import com.vitorpamplona.quic.frame.PingFrame
import com.vitorpamplona.quic.frame.StreamFrame
import com.vitorpamplona.quic.packet.ShortHeaderPacket
import com.vitorpamplona.quic.stream.StreamId
import com.vitorpamplona.quic.tls.InProcessTlsServer
import com.vitorpamplona.quic.tls.PermissiveCertificateValidator
import kotlinx.coroutines.flow.toList
import kotlinx.coroutines.runBlocking
import kotlinx.coroutines.withTimeoutOrNull
@@ -356,47 +354,9 @@ class KeyUpdatePeerInitiatedTest {
return null
}
// Default caps (16/16 streams, 1 MiB data) — the rotation tests
// don't push much traffic; small caps keep the pipe handshake fast.
private fun newConnectedClient(): Pair<QuicConnection, InMemoryQuicPipe> =
runBlocking {
val client =
QuicConnection(
serverName = "keyupdate.test",
config =
QuicConnectionConfig(
initialMaxStreamsBidi = 16,
initialMaxStreamsUni = 16,
initialMaxData = 1L * 1024 * 1024,
initialMaxStreamDataBidiLocal = 64L * 1024,
initialMaxStreamDataBidiRemote = 64L * 1024,
initialMaxStreamDataUni = 64L * 1024,
),
tlsCertificateValidator = PermissiveCertificateValidator(),
)
val serverScid = ConnectionId.random(8)
val tlsServer =
InProcessTlsServer(
transportParameters =
TransportParameters(
initialMaxData = 1L * 1024 * 1024,
initialMaxStreamDataBidiLocal = 64L * 1024,
initialMaxStreamDataBidiRemote = 64L * 1024,
initialMaxStreamDataUni = 64L * 1024,
initialMaxStreamsBidi = 16,
initialMaxStreamsUni = 16,
initialSourceConnectionId = serverScid.bytes,
originalDestinationConnectionId = client.destinationConnectionId.bytes,
).encode(),
)
val pipe =
InMemoryQuicPipe(
client = client,
initialDcid = client.destinationConnectionId.bytes,
serverScid = serverScid,
tlsServer = tlsServer,
)
client.start()
pipe.drive(maxRounds = 16)
assertEquals(QuicConnection.Status.CONNECTED, client.status)
client to pipe
}
com.vitorpamplona.quic.connection
.newConnectedClient()
}
quic/src/commonTest/kotlin/com/vitorpamplona/quic/connection/MoqLiteLossHarnessTest.kt
+7 -44
View File
@@ -22,8 +22,6 @@ package com.vitorpamplona.quic.connection
import com.vitorpamplona.quic.frame.StreamFrame
import com.vitorpamplona.quic.stream.StreamId
import com.vitorpamplona.quic.tls.InProcessTlsServer
import com.vitorpamplona.quic.tls.PermissiveCertificateValidator
import kotlinx.coroutines.flow.toList
import kotlinx.coroutines.runBlocking
import kotlinx.coroutines.withTimeoutOrNull
@@ -392,47 +390,12 @@ class MoqLiteLossHarnessTest {
assertEquals(QuicConnection.Status.CONNECTED, client.status)
}
// 1024 streams per direction + 16 MiB connection-level data —
// headroom for 200-stream loss tests without bumping caps.
private fun newConnectedClient(): Pair<QuicConnection, InMemoryQuicPipe> =
runBlocking {
val client =
QuicConnection(
serverName = "loss.test",
config =
QuicConnectionConfig(
initialMaxStreamsBidi = 1024,
initialMaxStreamsUni = 1024,
initialMaxData = 16L * 1024 * 1024,
initialMaxStreamDataBidiLocal = 64L * 1024,
initialMaxStreamDataBidiRemote = 64L * 1024,
initialMaxStreamDataUni = 64L * 1024,
),
tlsCertificateValidator = PermissiveCertificateValidator(),
)
val serverScid = ConnectionId.random(8)
val tlsServer =
InProcessTlsServer(
transportParameters =
TransportParameters(
initialMaxData = 16L * 1024 * 1024,
initialMaxStreamDataBidiLocal = 64L * 1024,
initialMaxStreamDataBidiRemote = 64L * 1024,
initialMaxStreamDataUni = 64L * 1024,
initialMaxStreamsBidi = 1024,
initialMaxStreamsUni = 1024,
initialSourceConnectionId = serverScid.bytes,
originalDestinationConnectionId = client.destinationConnectionId.bytes,
).encode(),
)
val pipe =
InMemoryQuicPipe(
client = client,
initialDcid = client.destinationConnectionId.bytes,
serverScid = serverScid,
tlsServer = tlsServer,
)
client.start()
pipe.drive(maxRounds = 16)
assertEquals(QuicConnection.Status.CONNECTED, client.status)
client to pipe
}
com.vitorpamplona.quic.connection.newConnectedClient(
maxStreamsBidi = 1024,
maxStreamsUni = 1024,
maxData = 16L * 1024 * 1024,
)
}
quic/src/commonTest/kotlin/com/vitorpamplona/quic/connection/PathValidationTest.kt
+4 -37
View File
@@ -24,8 +24,6 @@ import com.vitorpamplona.quic.frame.PathChallengeFrame
import com.vitorpamplona.quic.frame.PathResponseFrame
import com.vitorpamplona.quic.frame.decodeFrames
import com.vitorpamplona.quic.frame.encodeFrames
import com.vitorpamplona.quic.tls.InProcessTlsServer
import com.vitorpamplona.quic.tls.PermissiveCertificateValidator
import kotlinx.coroutines.runBlocking
import kotlin.test.Test
import kotlin.test.assertContentEquals
@@ -181,7 +179,7 @@ class PathValidationTest {
fun pathResponseQueueIsBoundedAgainstChallengeFlood() =
runBlocking {
// Defence-in-depth: an attacker spamming PATH_CHALLENGE
// shouldn't pin arbitrary memory in our pendingPathResponses
// shouldn't pin arbitrary memory in our pendingPathChallengePayloads
// queue. Cap is MAX_PENDING_PATH_RESPONSES (64); excess
// challenges are silently dropped — the protocol allows
// it (peer would retransmit on PTO if a response actually
@@ -212,39 +210,8 @@ class PathValidationTest {
assertEquals(QuicConnection.Status.CONNECTED, client.status)
}
// Default caps — path validation tests don't open streams.
private fun newConnectedClient(): Pair<QuicConnection, InMemoryQuicPipe> =
runBlocking {
val client =
QuicConnection(
serverName = "path.test",
config = QuicConnectionConfig(),
tlsCertificateValidator = PermissiveCertificateValidator(),
)
val serverScid = ConnectionId.random(8)
val tlsServer =
InProcessTlsServer(
transportParameters =
TransportParameters(
initialMaxData = 1L * 1024 * 1024,
initialMaxStreamDataBidiLocal = 64L * 1024,
initialMaxStreamDataBidiRemote = 64L * 1024,
initialMaxStreamDataUni = 64L * 1024,
initialMaxStreamsBidi = 16,
initialMaxStreamsUni = 16,
initialSourceConnectionId = serverScid.bytes,
originalDestinationConnectionId = client.destinationConnectionId.bytes,
).encode(),
)
val pipe =
InMemoryQuicPipe(
client = client,
initialDcid = client.destinationConnectionId.bytes,
serverScid = serverScid,
tlsServer = tlsServer,
)
client.start()
pipe.drive(maxRounds = 16)
assertEquals(QuicConnection.Status.CONNECTED, client.status)
client to pipe
}
com.vitorpamplona.quic.connection
.newConnectedClient()
}
quic/src/commonTest/kotlin/com/vitorpamplona/quic/connection/StreamRetirementSoakTest.kt
+9 -44
View File
@@ -23,8 +23,6 @@ package com.vitorpamplona.quic.connection
import com.vitorpamplona.quic.frame.AckFrame
import com.vitorpamplona.quic.frame.StreamFrame
import com.vitorpamplona.quic.stream.StreamId
import com.vitorpamplona.quic.tls.InProcessTlsServer
import com.vitorpamplona.quic.tls.PermissiveCertificateValidator
import kotlinx.coroutines.flow.toList
import kotlinx.coroutines.runBlocking
import kotlinx.coroutines.withTimeoutOrNull
@@ -429,47 +427,14 @@ class StreamRetirementSoakTest {
return any
}
// moq-lite-shaped fixture: 4 K bidi caps + 64 K peer-uni caps so
// the soak harness can churn 10 K + streams without hitting
// either the peer cap or our advertised cap before the writer's
// periodic MAX_STREAMS_UNI extension fires.
private fun newConnectedClient(): Pair<QuicConnection, InMemoryQuicPipe> =
runBlocking {
val client =
QuicConnection(
serverName = "example.test",
config =
QuicConnectionConfig(
initialMaxStreamsBidi = 4096,
initialMaxStreamsUni = 65_536,
initialMaxData = 16L * 1024 * 1024,
initialMaxStreamDataBidiLocal = 64L * 1024,
initialMaxStreamDataBidiRemote = 64L * 1024,
initialMaxStreamDataUni = 64L * 1024,
),
tlsCertificateValidator = PermissiveCertificateValidator(),
)
val serverScid = ConnectionId.random(8)
val tlsServer =
InProcessTlsServer(
transportParameters =
TransportParameters(
initialMaxData = 16L * 1024 * 1024,
initialMaxStreamDataBidiLocal = 64L * 1024,
initialMaxStreamDataBidiRemote = 64L * 1024,
initialMaxStreamDataUni = 64L * 1024,
initialMaxStreamsBidi = 4096,
initialMaxStreamsUni = 65_536,
initialSourceConnectionId = serverScid.bytes,
originalDestinationConnectionId = client.destinationConnectionId.bytes,
).encode(),
)
val pipe =
InMemoryQuicPipe(
client = client,
initialDcid = client.destinationConnectionId.bytes,
serverScid = serverScid,
tlsServer = tlsServer,
)
client.start()
pipe.drive(maxRounds = 16)
assertEquals(QuicConnection.Status.CONNECTED, client.status)
client to pipe
}
com.vitorpamplona.quic.connection.newConnectedClient(
maxStreamsBidi = 4096,
maxStreamsUni = 65_536,
maxData = 16L * 1024 * 1024,
)
}
quic/src/jvmTest/kotlin/com/vitorpamplona/quic/connection/QuicHeapSoakTest.kt
+8 -44
View File
@@ -22,8 +22,6 @@ package com.vitorpamplona.quic.connection
import com.vitorpamplona.quic.frame.StreamFrame
import com.vitorpamplona.quic.stream.StreamId
import com.vitorpamplona.quic.tls.InProcessTlsServer
import com.vitorpamplona.quic.tls.PermissiveCertificateValidator
import kotlinx.coroutines.flow.toList
import kotlinx.coroutines.runBlocking
import kotlin.test.Test
@@ -215,47 +213,13 @@ class QuicHeapSoakTest {
}
}
// moq-lite-shaped fixture (matches StreamRetirementSoakTest's
// shape) — large peer-uni cap so the heap canary can churn 90 K
// streams in a 30-min run without bumping the cap.
private fun newConnectedClient(): Pair<QuicConnection, InMemoryQuicPipe> =
runBlocking {
val client =
QuicConnection(
serverName = "soak.test",
config =
QuicConnectionConfig(
initialMaxStreamsBidi = 4096,
initialMaxStreamsUni = 65_536,
initialMaxData = 16L * 1024 * 1024,
initialMaxStreamDataBidiLocal = 64L * 1024,
initialMaxStreamDataBidiRemote = 64L * 1024,
initialMaxStreamDataUni = 64L * 1024,
),
tlsCertificateValidator = PermissiveCertificateValidator(),
)
val serverScid = ConnectionId.random(8)
val tlsServer =
InProcessTlsServer(
transportParameters =
TransportParameters(
initialMaxData = 16L * 1024 * 1024,
initialMaxStreamDataBidiLocal = 64L * 1024,
initialMaxStreamDataBidiRemote = 64L * 1024,
initialMaxStreamDataUni = 64L * 1024,
initialMaxStreamsBidi = 4096,
initialMaxStreamsUni = 65_536,
initialSourceConnectionId = serverScid.bytes,
originalDestinationConnectionId = client.destinationConnectionId.bytes,
).encode(),
)
val pipe =
InMemoryQuicPipe(
client = client,
initialDcid = client.destinationConnectionId.bytes,
serverScid = serverScid,
tlsServer = tlsServer,
)
client.start()
pipe.drive(maxRounds = 16)
assertEquals(QuicConnection.Status.CONNECTED, client.status)
client to pipe
}
com.vitorpamplona.quic.connection.newConnectedClient(
maxStreamsBidi = 4096,
maxStreamsUni = 65_536,
maxData = 16L * 1024 * 1024,
)
}