mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-08-10 16:33:27 +00:00
43d0b8b2bc4e34dad38bd8566b95c806ead09de0
101
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  Vitor PamplonaandClaude Opus 4.7
|
1d503dc6ac |
Adds android.util.Log mock for quic tests
Mirrors the quartz commonTest stub so :quic:testAndroidHostTest no longer
throws RuntimeException("Stub!") through PlatformLog.android on the
MAX_STREAMS_UNI emission path.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
||
|
Vitor PamplonaandClaude Opus 4.7
|
0920aba2b8 |
fix(quic): issue spare source CIDs to peer post-handshake-confirmed
RFC 9000 §5.1.1 + §19.15: a client SHOULD advertise spare source
CIDs to the peer via NEW_CONNECTION_ID frames so the peer has
DCIDs available for path migration / NAT rebind. Strict server
stacks (quic-go, picoquic, msquic, mvfst) refuse to validate a new
client path until they have a spare DCID — server log shows
"skipping validation of new path … since no connection ID is
available". Quinn migrates on src-port-change alone, which is why
rebind-port / rebind-addr passed against quinn pre-fix and failed
against the others.
Adds:
- IssuedSourceConnectionIdEntry (data class) — sequence + CID +
stateless-reset token tuple.
- QuicConnection.issuedSourceConnectionIds (LinkedHashMap) — pool
of currently-active issued SCIDs, seeded with seq=0 (the
initial CID, no token because the stateless_reset_token TP is
server-only per RFC 9000 §18.2).
- QuicConnection.issueOwnConnectionIdsLocked(count) — drives the
initial issuance once handshake is confirmed; appends recovery
tokens to pendingOwnNewConnectionIdEmits for the writer to emit.
- QuicConnection.issueOwnReplacementSourceCidLocked() — top-up
after each peer RETIRE_CONNECTION_ID so the pool stays at
full capacity for repeated migrations.
- QuicConnectionWriter — once handshakeConfirmed flips, calls
issueOwnConnectionIdsLocked with `peer.activeConnectionIdLimit
- 1` (cap 7); drains pendingOwnNewConnectionIdEmits as fresh
NEW_CONNECTION_ID frames; the existing pendingNewConnectionId
map continues to handle loss-recovery retransmits.
- applyPeerRetireConnectionIdLocked — three cases: in-pool seq
is freed and replaced; below-next-seq missing seq is benign
retransmit; above-next-seq is PROTOCOL_VIOLATION (peer
retiring something we never advertised).
Verified against the live interop runner:
- quic-go rebind-port: 2/2 (was 0/N — Task 2). Now passes in ~64s.
- quic-go rebind-addr: 2/2 (was 0/N). Now passes in ~125-138s.
- picoquic rebind-port: 2/2.
- picoquic rebind-addr: 1/2 (flaky — separate investigation;
one run the connection migrates correctly, the other times
out at 110s; not blocking the rebind-port / quic-go win).
- picoquic connectionmigration: 0/2 still failing (the runner's
"active migration" testcase exercises a more sophisticated
migration shape — TBD; tracked separately).
Tests: IssuedSourceConnectionIdTest (5 cases pinning the
post-handshake emission, peer-RETIRE replacement, retransmit
tolerance, and protocol-violation closure for above-next-seq retires).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
||
|
Vitor PamplonaandClaude Opus 4.7
|
3a5c010993 |
fix(quic): consecutivePtoCount must advance once per PTO firing
RFC 9002 §6.2.2 says the consecutive-PTO count goes up by exactly
ONE per PTO timer expiry, so the §6.2.1 backoff is `pto_base * 2^count`
per firing. Pre-fix, the count incremented THREE times per firing:
1. inside `handlePtoFired` itself,
2. inside `requeueInflightForProbe` (which `handlePtoFired` calls),
3. again from the send loop's between-probe re-requeue (RFC §6.2.4
two-packet probe budget calls `requeueInflightForProbe` a
second time).
That made the effective backoff `2^(3*N)` per firing — 1×, 8×, 64×
of pto_base. With pto_base ≈ 150 ms post-handshake, PTO #3 didn't
fire until ~11 s post-handshake, well past the 5 s amp-limited
idle timeout strict servers (quic-go) enforce.
Quic-go's `amplificationlimit` testcase exposed this. The sim
drops client packets 2–7. Our PTO #2 second probe is packet #8
— the first one that gets through. With correct 1× increment
per PTO, packet #8 reaches the server in ~900 ms; pre-fix it
arrived at ~12 s, after quic-go had already destroyed the
connection ("Amplification window limited" → "Destroying
connection: timeout: no recent network activity"). Same root
cause for `handshakeloss` flakiness against quic-go (multiconnect
under 30% loss can't recover within the 30 s per-iteration
budget when PTO ramps to 64× by iteration 3).
Fix:
- Move the count increment to the top of `handlePtoFired`,
before it calls `requeueInflightForProbe`. The threshold
check inside the latter (RFC 9000 §9 — trigger
PATH_PROBE_PTO_THRESHOLD migration) reads the
post-increment value, preserving the prior 2-PTO-firing
trigger semantics.
- Remove the count increment from `requeueInflightForProbe`.
The send loop's between-probe re-requeue stays a no-op for
the count (same PTO firing).
Tests: PtoCryptoRetransmitTest.consecutivePtoCountAdvancesByOnePerPtoFiringNotPerProbePacket
pins the contract (full handlePtoFired → drain → re-requeue →
drain → handlePtoFired sequence; asserts count after each step).
Verified end-to-end against the live interop runner:
- amplificationlimit vs quic-go: 5/5 (was 0/3 — consistent
30 s timeout). Now passes in ~7 s.
- handshakeloss vs quic-go: 5/5 (was 2/3 — flaky multiconnect
iteration timeouts). Now consistently ~30 s.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
||
|
Vitor PamplonaandClaude Opus 4.7
|
e8991fab69 |
fix(quic): gate client-initiated key update + path migration on HANDSHAKE_DONE
Pre-fix, [QuicConnection.initiateKeyUpdate] only checked that 1-RTT
keys were installed — which fires when the TLS handshake derives
Finished, well before HANDSHAKE_DONE arrives. RFC 9001 §6.1 + §4.1.2
require the CLIENT to consider the handshake confirmed (HANDSHAKE_DONE
received) before rolling KEY_PHASE; quinn / quic-go / picoquic
correctly close the connection with PROTOCOL_VIOLATION ("illegal
packet: key update error") when an early update arrives.
The interop runner's keyupdate testcase against quinn flushed this:
the client polled `status == CONNECTED` (which flips on TLS Finished
via `onHandshakeComplete`) and called `initiateKeyUpdate()` before
HANDSHAKE_DONE landed. ~33% repro rate; the rest of the runs HANDSHAKE_DONE
happened to arrive in the gap between the status check and the
update.
Fix:
- Add `QuicConnection.handshakeConfirmed: Boolean` flipped only by
the parser when a HANDSHAKE_DONE frame is processed.
- Add `awaitHandshakeConfirmed()` for callers that need the
spec-confirmed state.
- Gate `initiateKeyUpdate()` on `handshakeConfirmed` (returns
false otherwise — same shape as the existing app-keys-not-yet
branch).
- Tighten `triggerPathMigrationLocked` to also gate on
`handshakeConfirmed` (RFC 9000 §9.1: same confirmation
requirement). Pre-fix it gated on `handshakeComplete`, which
flips at TLS-Finished too.
- InteropClient's keyupdate flow now waits on
`awaitHandshakeConfirmed` (with a 2s upper bound) rather than
polling `status == CONNECTED`.
The test fixture in ConnectedClientFixture.newConnectedClient now
delivers HANDSHAKE_DONE by default — most tests want the
production "fully ready" state. New `deliverHandshakeDone = false`
parameter for tests that need to exercise the pre-confirmation
window (KeyUpdateClientInitiatedTest).
Tests:
- KeyUpdateClientInitiatedTest:
initiateKeyUpdateBeforeHandshakeDoneIsRejectedAndDoesNotRotateKeys
initiateKeyUpdateAfterHandshakeDoneRotatesBothDirections
awaitHandshakeConfirmedSuspendsUntilHandshakeDone
triggerPathMigrationBeforeHandshakeDoneReturnsNotConnected
Verified against the live interop runner — 5/5 against quinn,
3/3 against quic-go, 3/3 against picoquic (pre-fix: ~33% pass rate
against quinn).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
||
|
Vitor PamplonaandClaude Opus 4.7
|
707fcee5b6 |
fix(quic): rotate DCID off failed CID on path validation timeout
Pre-fix, PathValidator.checkValidationTimeout queued a
RETIRE_CONNECTION_ID for the failed sequence number while leaving
QuicConnection.destinationConnectionId stamping that same CID. The
strict servers (quic-go / picoquic / msquic / mvfst) processed the
RETIRE, dropped their routing entry, then closed us with
PROTOCOL_VIOLATION the next time a packet arrived stamped with
the just-retired CID — visible in qlog as two RETIRE_CONNECTION_ID
frames within ~500 ms followed by "retired connection ID N, which
was used as the Destination Connection ID on this packet".
Reproducer: setting proactiveDcidRotationMillis=4_000L on
QuicConnection drove the trigger every 4 s; rebind-port vs quic-go
then failed at ~10 s with the close above. Reverted in this commit;
the experiment fields are gone.
The fix splits the timeout outcome into three cases:
- NotTimedOut — budget hasn't elapsed.
- RecoveredOnSpare — rotated active to the lowest spare CID;
queued RETIRE for the failed seq; the
QuicConnection wrapper synchronously
updates destinationConnectionId so the
next outbound stamps the new CID, atomic
under streamsLock with the validator
state change.
- StuckOnFailedCid — no spare available; KEEP the failed seq
active and DO NOT queue retire. The
writer keeps stamping the unvalidated
CID; if the path is genuinely dead the
connection idles out, and once a
NEW_CONNECTION_ID arrives the next
trigger rotates cleanly.
After the fix, rebind-port vs quic-go fails at the 60 s test
timeout with server-side trigger=idle_timeout (the Task 2 issue —
strict servers gate on fresh DCID at new src port, which we can't
synchronize with the sim's rebind cadence) instead of the
PROTOCOL_VIOLATION close.
Tests:
- PathValidatorTest:
validationTimeoutAfter3PtoTransitionsToFailedAndRetiresFailedCidWithSpare
timeoutWithoutSpareKeepsFailedSeqActiveAndDoesNotRetire
twoConsecutiveFailedValidationsRetireAllAbandonedSequencesWithSpare
- ClientPathMigrationTest:
backToBackSuccessfulMigrationsRetireExactlyOnePerRotationAndStampNewDcid
validationTimeoutWithSpareRotatesDcidAndRetiresFailedSeq
validationTimeoutWithoutSpareKeepsActiveCidAndDoesNotRetire
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
||
|
Claude
|
6cc1bf6433 |
fix(quic): RFC 9001 §4.8 TLS alert mapping + RFC 9000 §22 CRYPTO_BUFFER_EXCEEDED
The remaining 🟦 cosmetic items from the 2026-05-09 audit. Pre-fix the connection's `closeErrorCode` was effectively unused — TLS handshake failures bubbled up as generic `QuicCodecException` and observers had to grep the reason string for the spec category. Now closeErrorCode carries the RFC 9000 §20.1 transport code or the RFC 9001 §4.8 TLS-alert-mapped CRYPTO_ERROR. Implementation: * `TlsAlertException(alertCode, message)` carrier raised by the TLS layer for the well-defined cases: - HelloRetryRequest received (handshake_failure = 40) - TLS 1.3 not negotiated (protocol_version = 70) - Unsupported cipher (illegal_parameter = 47) - ALPN mismatch (no_application_protocol = 120) - Cert chain validation failure (bad_certificate = 42) - CertificateVerify signature failure (decrypt_error = 51) - Server Finished MAC mismatch (decrypt_error = 51) - TLS KeyUpdate over QUIC (unexpected_message = 10) — RFC 9001 §6 mandates this specific code; fixes the misleading prior message that said "rotation not implemented" (we DO implement QUIC's own KEY_PHASE-bit rotation). The QUIC parser maps `0x100 + alertCode` per RFC 9001 §4.8 and stamps `closeErrorCode`. * `QuicTransportError` constants object covering the RFC 9000 §20.1 table (NO_ERROR through NO_VIABLE_PATH including the previously- flagged CRYPTO_BUFFER_EXCEEDED 0x0d, KEY_UPDATE_ERROR 0x0e, AEAD_LIMIT_REACHED 0x0f). * `markClosedExternally(reason, errorCode = NO_ERROR)` overload — backward compatible; lets call sites pin the spec code on `closeErrorCode` for qlog observability without changing the wire-emit semantics (still no CONNECTION_CLOSE frame, same as before — that's a separate, larger refactor). * RFC 9000 §22 CRYPTO_BUFFER_EXCEEDED enforcement — pre-insert per-level cap of 64 KiB on inbound CRYPTO data. Generous enough for an RSA-4096 cert chain with intermediates; bounds the worst-case heap a misbehaving peer can pin to 192 KiB across all 3 encryption levels. Fires before the receive buffer actually allocates. * INVALID_TOKEN — N/A for client role (only servers validate Retry tokens). KEY_UPDATE_ERROR — exposed as a constant; no current failure path maps to it (PN regression on a key-update packet is spec-allowed to handle silently per RFC 9001 §6.1, which we do). Tests: `ErrorCodeMappingTest` (6 cases) covers TlsAlertException construction + offset, alert code bounds, markClosedExternally preservation, CRYPTO_BUFFER_EXCEEDED close, sanity-check that small CRYPTO frames don't trip the cap, and §20.1 numeric values. `HelloRetryRequestTest` updated to expect TlsAlertException(40) instead of generic QuicCodecException. Plan updated to mark the §4.8 + §22 (CRYPTO_BUFFER_EXCEEDED) items resolved; KEY_UPDATE_ERROR documented as TLS-side covered + QUIC-side spec-allowed-silent; INVALID_TOKEN documented as N/A for client. https://claude.ai/code/session_01XGmmaVuy2wsSZQx2AqN2ik |
||
|
Claude
|
cdb9d87a2e |
fix(quic): RFC 9114 §6.2.1 critical-stream auto-close + WiFi-handoff stateless-reset tokens
Two related improvements for the audio-rooms moq-lite path on mobile.
** RFC 9114 §6.2.1 + RFC 9204 §4.2 critical-stream closure **
When the peer closes a critical unidirectional stream — control (0x00),
QPACK encoder (0x02), or QPACK decoder (0x03) — the spec MANDATES we
treat it as a connection error of type H3_CLOSED_CRITICAL_STREAM.
Pre-fix the demux only set `peerH3ProtocolError` flag on protocol
violations, with no autonomous close on clean FIN; the application
was expected to poll. For audio rooms running on lossy mobile paths,
relay-side control-stream drops left users staring at silent audio.
* `Http3ErrorCode` constants for the §8.1 error-code table.
* `WtPeerStreamDemux.drainControlStream` calls `connection.close(...)`
on either clean FIN (H3_CLOSED_CRITICAL_STREAM) or QuicCodecException
raised by the frame reader (specific code via
[http3ErrorCodeForMessage] map: H3_MISSING_SETTINGS,
H3_FRAME_UNEXPECTED, H3_SETTINGS_ERROR, H3_ID_ERROR, etc.).
* New `drainCriticalStream` helper fires the same close on QPACK
encoder / decoder FIN (RFC 9204 §4.2).
* Closure intent latched on `criticalStreamClosureCode` /
`criticalStreamClosureReason` so tests can verify the path runs
without wiring a real driver.
** RFC 9000 §10.3 stateless-reset tokens for migrated CIDs **
The 2026-05-09 stateless-reset detection covered tokens in the
unused-CID pool but lost them the moment a CID was rotated to active
via `tryStartValidation` (WiFi handoff) or `forceRotateToHigherSequence`
(peer-forced retire). On the new path the migrated token wouldn't
match — relay crash mid-handoff would silently hang until idle timeout.
* `PathValidator.knownStatelessResetTokens` — append-only lifetime
store, populated in `recordPeerNewConnectionId`. Survives rotation.
* `QuicConnection.isStatelessReset` walks the lifetime store instead
of the unused pool.
* §10.3.1 explicitly permits keeping tokens after retirement; cost
is ~16 bytes per peer-issued CID.
Tests:
* `CriticalStreamClosureTest` (5 cases) — control FIN, QPACK FIN,
MISSING_SETTINGS, FRAME_UNEXPECTED, idempotency.
* `StatelessResetDetectionTest` extended with 2 cases —
`token_persists_after_path_migration_for_wifi_handoff` and
`token_persists_through_force_rotation_for_acid_reissue`.
Plan updated: 🟦 H3_CLOSED_CRITICAL_STREAM resolved; the
"limitation: tokens for actively-used DCIDs we migrated to" caveat is
removed from the §10.3 entry and from known-limitations item #5.
https://claude.ai/code/session_01XGmmaVuy2wsSZQx2AqN2ik
|
||
|
Claude
|
d4ffe0474f |
fix(quic): RFC 9000 §10.2.2 DRAINING state for peer CONNECTION_CLOSE
Peer's CONNECTION_CLOSE now transitions the connection through DRAINING
for 3 * PTO before flipping to CLOSED, instead of going straight to
CLOSED. While DRAINING:
- the writer emits no packets (drainOutbound returns null);
- the parser drops late inbound silently at the top of feedDatagramInner;
- close()-awaiters unblock immediately via closingDrainSignal so the
transition isn't gated on the 3*PTO grace.
After 3 * PTO the driver's send-loop timer fires
markClosedExternally("draining period elapsed") which transitions to
CLOSED. The §10.2.2 grace period gives the peer's last retransmits a
chance to converge before we discard state.
Implementation:
* `Status.DRAINING` enum value with kdoc tying it to §10.2.2.
* `QuicConnection.drainingDeadlineMs` + `enterDraining(reason, nowMs)`
(sets status, computes `now + max(3*pto, MIN_DRAINING_PERIOD_MS)`,
completes closingDrainSignal, fires qlog) + `isDrainingExpired`.
* Parser's CONNECTION_CLOSE handler routes through `enterDraining`
instead of `markClosedExternally`.
* Driver folds `drainingDeadlineMs` into its send-loop sleep
`withTimeoutOrNull` and transitions to CLOSED on expiry.
* Writer short-circuits drainOutbound for DRAINING (returns null —
spec MUST NOT send).
* Parser drops late inbound at the top of feedDatagramInner.
Updated `FrameRoutingTest.connection_close_from_peer_short_circuits_remaining_frames`
to expect DRAINING (was CLOSED). Other status=CLOSED assertions in the
test suite cover OUR-side closes (markClosedExternally on protocol
violations / TRANSPORT_PARAMETER_ERROR / FLOW_CONTROL_ERROR) which
still go directly to CLOSED.
Test: `DrainingStateTest` (7 cases) covers the peer-close → DRAINING
transition, late-inbound drop, deadline math, the
MIN_DRAINING_PERIOD_MS floor, retransmit no-op semantics, and the
fresh-connection invariant.
Plan updated to mark all 🟡 Medium items resolved. The 2026-05-09
audit's complete spec-compliance close-out: 2 of 2 High + 6 of 6
Medium fixed in seven commits.
https://claude.ai/code/session_01XGmmaVuy2wsSZQx2AqN2ik
|
||
|
Claude
|
50ea477426 |
fix(quic): RFC 9000 §10.3 stateless-reset detection
A peer that has lost connection state (crash, restart, route change)
signals so by sending a "Stateless Reset" datagram — bytes shaped like
a short-header packet whose trailing 16 bytes equal a
`stateless_reset_token` previously communicated by the peer.
Pre-fix the tokens were stored (via NEW_CONNECTION_ID into the
[PathValidator] pool, and via the peer's `statelessResetToken`
transport parameter) but never matched against arriving datagrams —
a peer's stateless reset looked indistinguishable from noise and the
connection lingered until idle timeout, or worse, an attacker could
spam look-like-noise packets toward our integrity counter.
Implementation:
* `QuicConnection.isStatelessReset(datagram)` — short-header-form
+ size + trailing-16-bytes comparison. Matches against the peer's
advertised token AND every unused entry in `pathValidator`'s
pool. Constant-time per-token compare per §10.3.1 to avoid
leaking token bits via timing.
* `PathValidator.unusedTokenForSequence(seq)` — accessor for the
above to walk the pool.
* Parser pre-empts AEAD/HP parsing: `feedDatagramInner` checks every
short-header-form datagram before the loop. False-positive rate
of a real short-header packet (whose trailing 16 bytes are the
AEAD tag) matching a known token is 2^-128, negligible in
practice. Defense-in-depth: the AEAD-failure branch in
`feedShortHeaderPacket` retains a redundant check for
second-packet-of-coalesced edge cases.
* On match, `markClosedExternally("stateless reset received from
peer")` transitions silently to CLOSED — no CONNECTION_CLOSE
emission per §10.3.1.
Limitation: tokens for an actively-used DCID we migrated to via
`PathValidator.tryStartValidation` are lost when the entry leaves
the unused pool. Acceptable for the audio-rooms scope (no migration);
documented in the kdoc.
Test: `StatelessResetDetectionTest` (5 cases) covers token-match
silent-close, unknown-trailer no-close, pool-stored token match,
long-header form rejected, too-short datagram rejected.
https://claude.ai/code/session_01XGmmaVuy2wsSZQx2AqN2ik
|
||
|
Claude
|
4ec192347e |
fix(quic): RFC 9001 §6.6 AEAD invocation limit tracking
Per-key AEAD usage limits per RFC 9001 §6.6 / §B.1:
* Confidentiality limit (encrypt count per send key):
AES-128-GCM = 2^23, ChaCha20-Poly1305 = 2^62. Reaching this means
AEAD security is no longer assured; the endpoint MUST initiate a
key update or close.
* Integrity limit (forged-packet count per receive key):
AES-128-GCM = 2^52, ChaCha20-Poly1305 = 2^36. Reaching this means
an attacker has been grinding for AEAD key recovery; MUST close
with AEAD_LIMIT_REACHED.
Pre-fix neither limit was tracked. Long-running AES-128-GCM sessions
(~2hrs at 1000pkt/s) could roll past the confidentiality limit; an
attacker spamming forged packets had no failure ceiling.
Implementation:
* `Aead.confidentialityLimit` / `Aead.integrityLimit` properties
surface the spec values per cipher; AES-128-GCM and
ChaCha20-Poly1305 (commonMain singletons + jvmAndroid JCA classes)
return the §B.1 numbers.
* `QuicConnection.aeadEncryptCount` / `aeadDecryptFailureCount` —
per-key counters, reset on every 1-RTT key rotation
(`commitKeyUpdate` and `initiateKeyUpdate`).
* Writer increments encrypt count after each application-level
build. At half the limit it soft-triggers `initiateKeyUpdate`
(latched via `aeadKeyUpdateRequested` so the in-flight rotation
isn't re-issued); at the limit it closes with AEAD_LIMIT_REACHED
if rotation hasn't completed.
* Parser increments decrypt-failure count on every 1-RTT AEAD
auth-tag failure; closes when the count hits the integrity limit.
Initial / Handshake levels are out of scope — their keys' lifetime is
too short to approach the limit.
Test: `AeadInvocationLimitTest` (6 cases) covers spec-value
verification, counter increment on application send, counter reset on
rotation, confidentiality-limit close, integrity-limit close (via a
real ciphertext-tampered datagram from the in-process pipe).
https://claude.ai/code/session_01XGmmaVuy2wsSZQx2AqN2ik
|
||
|
Claude
|
be96d4b28f |
fix(quic): RFC 9000 §3.2 stream state — STREAM after RESET_STREAM rejected
Once the peer has sent RESET_STREAM the receive side enters the "Reset Recvd" terminal state per §3.2; any subsequent STREAM frame on that id is a peer protocol violation and MUST close the connection with STREAM_STATE_ERROR. Pre-fix the parser silently absorbed the bytes — a peer that violated the spec would leave us with a phantom mid-reset stream the peer believed was already dead, with reset/byte bookkeeping diverging. Implementation: a per-stream `peerResetReceived: Boolean` flag set in the RESET_STREAM handler and checked at the top of the STREAM handler. The flag is independent of our local-side `resetState` (which tracks OUR RESET emission). Test: `StreamAfterResetTest` (4 cases) covers STREAM-after-reset closure, FIN-bearing STREAM-after-reset, the legal FIN-then-RESET shape (no false-positive), and per-stream isolation (reset on stream 3 doesn't poison stream 7). https://claude.ai/code/session_01XGmmaVuy2wsSZQx2AqN2ik |
||
|
Claude
|
6e054583a9 |
fix(quic): RFC 9000 §4.1 connection-level inbound flow-control enforcement
Pre-fix the receiver enforced ONLY per-stream `receiveLimit`. The
aggregate cap (`initial_max_data` / latest MAX_DATA) was advertised and
respected on the SEND side but never checked on the RECEIVE side — a
peer that opened many streams and pushed each up to its per-stream cap
could collectively exceed `initial_max_data` without us closing.
Implementation:
* `QuicStream.receiveHighestOffset`: per-stream high-water mark of
"largest stream offset received" — the spec quantity that gets
summed across streams for the connection-level check.
* `QuicConnection.connectionInboundOffsetSum`: running sum across
all streams of `receiveHighestOffset`. Updated in the parser at
STREAM and RESET_STREAM frame ingest time.
* Parser closes with FLOW_CONTROL_ERROR whenever the running sum
exceeds `advertisedMaxData`.
* RESET_STREAM final-size accounting: per §4.5 the finalSize counts
toward connection-level flow control even though no STREAM frame
delivered those bytes — a peer that resets a stream at a finalSize
larger than it had previously sent gets the extra bytes added to
the running sum at RESET arrival.
Different from the writer's MAX_DATA threshold logic (which uses the
cheaper contiguous-end approximation): the receiver-side enforcement
needs the strict spec quantity to close before bookkeeping diverges.
Test: `ConnectionLevelFlowControlTest` (5 cases) covers below-cap pass,
over-cap close, retransmit-doesn't-double-count, RESET_STREAM
finalSize counts toward limit, fresh-handshake invariants.
https://claude.ai/code/session_01XGmmaVuy2wsSZQx2AqN2ik
|
||
|
Claude
|
657306392d |
fix(quic): RFC 9000 §18.2 transport-param bounds + §13.2.5 ack-delay-exponent + §7.2.4.1 reserved SETTINGS + RFC 9221 §3 outbound DATAGRAM size
Five spec-compliance fixes from the 2026-05-09 audit, all in the same
"hostile-peer hardening" tier.
* RFC 9000 §18.2 — `applyPeerTransportParameters` now closes the
connection with TRANSPORT_PARAMETER_ERROR when the peer advertises
`max_udp_payload_size < 1200`, `ack_delay_exponent > 20`, or
`active_connection_id_limit < 2`. Pre-fix the values were decoded
but never bounds-checked.
* RFC 9000 §13.2.5 — the parser's ACK-delay decoding now uses the
PEER's advertised `ack_delay_exponent` (with the §18.2 default of 3
as the pre-handshake fallback) instead of our own config value.
Defensive coercion to 0..20 is preserved so a bypass of the bounds
check above can't desync the RTT estimator.
* RFC 9114 §7.2.4.1 — `Http3Settings.decodeBody` now rejects the
HTTP/2-reserved SETTINGS ids 0x02, 0x03, 0x04, 0x05 with
H3_SETTINGS_ERROR. Pre-fix they fell through to the generic
1<<32 cap and were accepted.
* RFC 9221 §3 — the writer now drops outbound DATAGRAM frames when
the peer didn't advertise `max_datagram_frame_size` (or advertised
0), or when the encoded frame (type byte + length varint + payload)
would exceed the peer's advertised value. Pre-fix the writer
emitted DATAGRAM regardless and let spec-conformant peers close us
with PROTOCOL_VIOLATION.
* Added `TransportParameterDefaults` for the §18.2 defaults
(ack_delay_exponent=3, max_ack_delay=25ms, active_connection_id_limit=2)
so callers don't hard-code the magic numbers.
Tests: `TransportParameterBoundsTest` (8 cases) drives a real handshake
through the in-process pipe and asserts CLOSED on each violation +
CONNECTED at the boundary; `Http3ReservedSettingsTest` (5 cases) covers
the four reserved ids and the adjacent legal ids. Plan tier dropped
from 🟡 Medium to resolved for all five items.
https://claude.ai/code/session_01XGmmaVuy2wsSZQx2AqN2ik
|
||
|
Claude
|
a80f4ea19d |
fix(quic): RFC 9000 §17.2/§17.3 fixed-bit + §10.1 idle-timeout enforcement
The two High-severity gaps from the 2026-05-09 four-RFC compliance audit.
Fixed-bit (RFC 9000 §17.2 long-header, §17.3 short-header): the spec
says fixed-bit (0x40) MUST be 1 in every v1 packet; receivers MUST
discard packets where it's 0. Pre-fix the parsers checked only the
form-bit (0x80) and accepted any fixed-bit value — a peer or off-path
attacker could route packets through AEAD that aren't valid v1 packets.
Both `parseAndDecrypt` paths and the `peekKeyPhase` shortcut now
silently drop fixed-bit=0. The bit isn't header-protected (HP only
XORs the low 5 bits), so the check runs on the raw wire byte before
HP unmask + AEAD. Test: `FixedBitValidationTest` (3 cases).
Idle timeout (RFC 9000 §10.1): `max_idle_timeout` was decoded into
config and advertised via the ClientHello transport-params extension,
but never enforced — a black-holed connection lived indefinitely.
This commit:
* adds `QuicConnection.lastActivityMs`, bumped on (a) successful
inbound packet decrypt and (b) outbound ack-eliciting send per
§10.1.1
* adds `effectiveIdleTimeoutMs()` returning the min of local and
peer advertisements (skipping any side that advertised 0), with
the §10.1 `3 * PTO` floor applied
* folds the idle deadline into the driver's send-loop
`withTimeoutOrNull` so an idle connection wakes exactly at expiry
* silently transitions to CLOSED via `markClosedExternally` per
§10.2.1 ("the connection enters the closing state silently —
discarding the connection state without sending a CONNECTION_CLOSE")
Test: `IdleTimeoutTest` (8 cases) covers the min-of-two computation,
0-means-disabled handling, the 3*PTO floor, deadline math, and
postpone-on-activity. Plan updated to mark both items resolved.
https://claude.ai/code/session_01XGmmaVuy2wsSZQx2AqN2ik
|
||
|
Claude
|
28c1a355da |
fix(quic): RFC 9002 §6.1.2 loss-detection timer + PSK rejection signal
Round 9 — the two largest items remaining from the audit.
* RFC 9002 §6.1.2 timer-driven loss detection. [detectAndRemoveLost]
now returns a [Result] data class carrying both the list of lost
packets and the absolute monotonic deadline at which the next
earliest sub-largest in-flight packet will cross the time threshold.
The parser stores that deadline on each [LevelState.nextLossTimeMs]
per encryption level, and the driver's send loop now sleeps for
`min(ptoDeadline, minNextLossTimeAcrossLevels) - now`. On expiry,
the driver distinguishes:
* Loss-timer wake → run [detectAndRemoveLost] across all levels;
declare time-threshold-lost packets and dispatch their tokens.
No probe budget, no exponential backoff.
* PTO wake → existing [handlePtoFired] path (probe + backoff).
Pre-fix tail loss waited for the full PTO (often 5x the time
threshold) before retransmitting because we had no event between
ACK arrivals to fire loss detection. Now `9/8 * max_rtt` is the
ceiling.
* TLS PSK rejection: instead of the prior generic [QuicCodecException]
("server rejected PSK; full-handshake fallback not implemented"),
raise a typed [PskRejectedException] subclass. Application reconnect
logic can selectively catch this and retry the handshake without
cached resumption state — a path that's correct by construction
(fresh ClientHello, no PSK history, no transcript-rebuild
complexity). [QuicCodecException] is now `open` so the subclass
can extend it.
In-place fallback (clear early secret, rebuild transcript without
PSK extension, replay derivation) remains deferred — the subtle
transcript-hash discrepancies it could introduce would be much
harder to debug than a hard failure that the application
intentionally turns into a retry.
All 269 :quic:jvmTest tests pass. BUILD SUCCESSFUL in 47s.
https://claude.ai/code/session_01AhGvbMV8uPRse3TmAGaddM
|
||
|
Claude
|
f987b3dfe2 |
fix(quic): SETTINGS validation, sensitive headers, peer-stream count, drain alloc
Round 7 — closing out the audit's spec/correctness/perf quick wins.
* WtPeerStreamDemux: validate peer SETTINGS includes ENABLE_WEBTRANSPORT=1
AND ENABLE_CONNECT_PROTOCOL=1 (draft-ietf-webtrans-http3 §3 +
RFC 8441). Pre-fix we accepted any SETTINGS and proceeded to
Extended CONNECT, which the server then rejects with no
diagnostic for the application. Now surfaces as a typed protocol
error on peerH3ProtocolError so the QUIC layer closes deliberately.
* QpackEncoder: set N=1 (never-indexed) on literal field lines for
authorization / cookie / set-cookie / proxy-authorization per
RFC 9204 §4.5.4. Pre-fix sensitive credentials could be cached
by intermediate QPACK encoder caches.
* QuicConnection.getOrCreatePeerStreamLocked: track peerInitiated*Count
via max(current, peerIndex+1) instead of += 1. The counter now
derives from the stream id's index field (RFC 9000 §2.1) and is
idempotent against retransmits-after-eviction. Pre-fix a peer
retransmit on a stream id that aged out of the retired-IDs FIFO
bumped the counter again, eventually triggering spurious
MAX_STREAMS_* emissions.
* applyPeerRetireConnectionIdLocked: reclassify the close-on-seq=0
case as PROTOCOL_VIOLATION (peer fault) instead of INTERNAL_ERROR
(our fault). The diagnostic was misleading — the peer IS
misbehaving (asking us to retire our only SCID with no
replacement available), not us.
* QuicConnectionWriter.drainOutbound: skip the
`streamsView.filter { !it.isClosed }` allocation when no streams
are closed. Quick `any` scan first; only allocate the filtered
list when at least one stream is actually closed. Saves an
N-sized ArrayList per drain in the common case (~50 drains/sec
× N up to ~2000 streams under multiplex load).
All 269 :quic:jvmTest tests pass. BUILD SUCCESSFUL in 40s.
https://claude.ai/code/session_01AhGvbMV8uPRse3TmAGaddM
|
||
|
Claude
|
5421b81569 |
perf(quic): QPACK Huffman decode — no boxing, IntArray + binary search
Pre-fix QpackHuffman.decode allocated two boxed Integers per output character: one for the `candidate` Int passed into `HashMap<Int, Int>.get` and one for the wrapper-Integer return value. Output also went through `ArrayList<Byte>`, boxing every emitted byte as a `java.lang.Byte` (~16 bytes per output byte on a 64-bit JVM). On a typical HTTP/3 response with ~30 header values, that's hundreds of throwaway wrapper objects per request — pure GC churn on the hot path. The new layout keeps two parallel `IntArray`s per code-length: codes[len] (sorted ascending) and syms[len] (the matching symbol indices). Lookup is a primitive `IntArray.binarySearch(candidate)` — no boxing, the array stays in JIT-friendly contiguous memory, and the per-length arrays are tiny (a few entries each, since the Huffman table is sparse at any given length). Output uses a growable `ByteArray` with a manual position index rather than `ArrayList<Byte>`. Pre-grow to 2× input size as a rough upper bound — ASCII headers compress to ~62% with HPACK Huffman, so we rarely need to grow. Also fixes a latent bug: the new init loop covers lengths 5..30 (previously 5..29), restoring decoding for symbols 10 (LF), 13 (CR), 22 (DC2) which all use 30-bit codes per RFC 7541 Appendix B. Pre-rewrite the HashMap path included these via `for (sym in 0..255)` walking the full symbol table; the IntArray rewrite needed an explicit length range and accidentally cut at 29. Added a unit test exercising hand-encoded length-30 inputs to lock the fix in. Behavior verified against RFC 7541 Appendix C test vectors and the new length-30 round-trip. All 269 :quic:jvmTest tests pass. https://claude.ai/code/session_01AhGvbMV8uPRse3TmAGaddM |
||
|
Claude
|
392df0384b |
fix(quic): HTTP/3 stream-context validation + ReceiveBuffer perf cliff
Round 3 of the audit follow-ups.
* Http3FrameReader gains a [StreamContext] parameter that enforces
RFC 9114 §7.2 per-stream rules:
- CONTROL: first frame MUST be SETTINGS (else H3_MISSING_SETTINGS);
duplicate SETTINGS, DATA, HEADERS, PUSH_PROMISE all forbidden.
- REQUEST: SETTINGS / GOAWAY / MAX_PUSH_ID / CANCEL_PUSH forbidden.
- PUSH: similar set including PUSH_PROMISE.
- Reserved types 0x02 / 0x06 / 0x08 / 0x09 explicitly rejected.
- WT_BIDI_DATA / WT_UNI_DATA: reader is the wrong tool, throw.
- UNCHECKED preserves prior test behaviour and is the default.
WtPeerStreamDemux's CONTROL drain now constructs the reader with
StreamContext.CONTROL, so a buggy server can no longer slip a DATA
frame into our SETTINGS expectations and silently confuse the
parser. The validation throws QuicCodecException, which the
drainControlStream catch records on a new peerH3ProtocolError
field — the QUIC layer / application reads it to close with the
proper diagnostic instead of having the route() catch swallow it.
* ReceiveBuffer no longer coalesces overlapping segments on insert.
Pre-fix every reorder fill allocated a fresh merged ByteArray of
size (hi - lo) and copyInto'd each existing segment — under a 200-
chunk reorder burst that was O(N²) bytes. The new layout keeps
segments as a sorted, non-overlapping list (binary-searched on
insert) and only allocates at readContiguous time, where it walks
consecutive segments and concats them in a single pass. Adjacent
segments are not eagerly merged — the read-side concat is bounded
by the contiguous prefix the consumer is about to drain anyway.
bufferedAhead becomes O(1) (cached counter) instead of O(N) sum.
* New tests cover the per-context rejection paths (CONTROL-stream
first-frame check, DATA-on-control, SETTINGS-on-request, all four
reserved frame types).
All 269 :quic:jvmTest tests pass. BUILD SUCCESSFUL.
https://claude.ai/code/session_01AhGvbMV8uPRse3TmAGaddM
|
||
|
Vitor PamplonaandClaude Opus 4.7
|
0c4bf031f1 |
fix(quic): RFC 9002 §6.2.4 — emit two ack-eliciting packets per PTO probe
Single-packet probes need 6 PTO doublings (~19s) to land one datagram through the `amplificationlimit` interop scenario's 6-drop window. quic-go and msquic kill the connection at ~10s of silence regardless of our handshake-timeout budget, so we never recovered against them (diagnosed in the parent investigation; the 10s→30s timeout bump in 0a892b0d4b only fixed picoquic). RFC 9002 §6.2.4 allows up to 2 ack-eliciting packets per PTO. Adding the second probe halves recovery to ~3 PTO rounds (~5s) and lands within strict server tolerances. Wiring: - New `QuicConnection.pendingProbePackets`, set to 2 by handlePtoFired. - Extracted `requeueInflightForProbe` from handlePtoFired so the send loop can re-requeue inflight CRYPTO / STREAM bytes between probes. - Send loop decrements the budget after each probe-bearing send; if the budget is still positive, re-requeues AND re-arms `pendingPing` so the no-data fallback (post-handshake idle) still emits a second PING. Without the `pendingPing` re-arm, only the first probe fires when CRYPTO is fully ACK'd — `pendingPing` is one-shot in collectHandshakeLevelFrames. Verified end-to-end: - amplificationlimit: ✕→✓ vs quic-go (35s→7s); ✓ no-regression vs picoquic (19s→7s) and quinn (15s→7s); msquic now reports server- side UNSUPPORTED (was failing). Recovery times across the board drop ~3x because handshake-loss recovery is ~3 PTO rounds instead of ~6. - handshake / transfer / multiplexing / handshakeloss all green vs quic-go, quinn, picoquic, msquic — no regression on the core matrix. Tests: - New `ptoEmitsTwoProbePacketsPerRfc9002` in PtoCryptoRetransmitTest invokes the EXACT helpers the send loop uses (handlePtoFired then requeueInflightForProbe between drains) and asserts two distinct Initial datagrams with the same CRYPTO bytes at offset 0 on distinct PNs. Verified the test fails when budget is reverted to 1. - Existing PTO + recovery tests stay green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
Claude
|
9ad4dbc356 |
fix(quic): replace deprecated lock with split locks in QlogObserverTest
The post-handshake status check uses lifecycleLock (status is guarded by lifecycleLock per the lock-split refactor); the malformed-datagram test uses streamsLock since feedDatagram requires streamsLock. |
||
|
Claude
|
07ba23a71c |
fix(quic): second-pass audit fixes for path validation
Three correctness bugs surfaced by post-fix re-audit, plus minor
cleanups.
- Bug A (validation hang): checkPathValidationTimeoutLocked was
only called from handlePtoFired. PATH_CHALLENGE is ack-eliciting
so the peer ACKs it; that ACK resets consecutivePtoCount, which
means the PTO timer that used to host the budget check stops
firing. Validation could hang indefinitely on a peer that ACKs
but doesn't reply with PATH_RESPONSE. Fix: drive the budget
check from drainOutbound (every send-loop wake).
- Bug B (stale retire): under abrupt-migration semantics the
prior CID is abandoned the moment we rotate. Queuing the retire
only inside applyPathResponse meant two consecutive failed
validations would leave the original seq=0 unretired forever.
Fix: queue priorSeq in tryStartValidation; advance
activeCidSequence at trigger time so it tracks the on-wire DCID.
- Bug C (spec MUST violation): RFC 9000 §5.1.2 requires server-
forced retirement of the active CID when the peer's
retire_prior_to advances past it. Previously the parser silently
accepted the offer and we kept stamping a now-retired CID.
Fix: new PathValidator.forceRotateToHigherSequence; called from
applyPeerNewConnectionIdLocked after a successful Stored result.
No PATH_CHALLENGE needed (same path, just different CID).
Closes connection with CONNECTION_ID_LIMIT_ERROR if the pool
is empty when forced rotation is needed.
Concurrency:
- Add @Volatile to consecutivePtoCount. The driver kdoc claimed
it already was; it wasn't. The send-loop reads it lockless for
backoff calculation while three writers mutate it (driver PTO
fire, parser ACK reset, applyPeerPathResponseLocked reset).
Cleanup:
- Drop redundant destinationConnectionId re-stamp in
applyPeerPathResponseLocked (already rotated at challenge time).
- Fix PathMigrationResult kdoc to acknowledge that NotConnected
is produced only by the connection-level wrapper.
- Update applyPeerNewConnectionIdLocked kdoc with the §5.1.2
forced-rotation contract.
Tests:
- PathValidatorTest:
+ triggerRetiresPriorSequenceImmediately (Bug B)
+ twoConsecutiveFailedValidationsRetireAllAbandonedSequences
(Bug B regression — would have caught the original miss)
+ forceRotateRunsWhenWatermarkPassesActiveCid (Bug C)
+ forceRotateNoOpWhenWatermarkBelowActive (Bug C edge)
+ forceRotateRotatesAgainWhenNewerOfferAdvancesWatermark (Bug C cascading)
- ClientPathMigrationTest:
+ newConnectionIdWithRetirePriorToPastActiveForcesRotationOnSamePath
(Bug C wire-level)
+ Updated fullMigrationRoundTrip to assert RETIRE rides in the
same packet as PATH_CHALLENGE under abrupt-migration semantics.
+ Updated pathResponseWithMismatchingPayloadKeepsValidatingAndDcid
to reflect activeCidSequence advances at trigger time.
All :quic:jvmTest (39 tests in path-validation suite) and
:nestsClient:jvmTest pass.
https://claude.ai/code/session_01PVVhSQXvw4K4oQ46FzpgaT
|
||
|
Claude
|
9b9ede2e1e |
fix(quic): audit fixes for client path validation + DCID rotation
Addresses seven bugs surfaced by post-landing audit of the path
validation feature.
Spec fixes (RFC 9000 §9):
- Bug 1: PATH_CHALLENGE was going out on the OLD DCID because the
writer reads conn.destinationConnectionId per packet and the
rotation only happened on PATH_RESPONSE arrival. Now rotate the
DCID inside triggerPathMigrationLocked (abrupt-migration model
appropriate for the "old path looks dead" trigger condition).
Fixes the headline feature — without this the challenge cannot
actually exercise the new path.
- Bug 2: 3 * PTO timeout dropped the failed CID without queuing a
RETIRE_CONNECTION_ID. The peer kept the routing entry forever.
checkValidationTimeout now queues the failed sequence per §5.1.2.
- Bug 3: RETIRE_CONNECTION_ID for seq 0 was silently honored. We
have no replacement SCID to give the peer (we don't issue our
own NEW_CONNECTION_ID frames), so the connection is unusable.
Close with INTERNAL_ERROR instead.
- Bug 4: triggerPathMigration had no handshake-confirmed gate;
§9.1 forbids migration before handshake confirmation. Returns
new PathMigrationResult.NotConnected when status != CONNECTED.
Implementation fixes:
- Bug 5: driver was calling Clock.System.now() directly instead
of conn.nowMillis(), breaking virtual-clock tests.
- Bug 6: PTO threshold check ran BEFORE the consecutive-PTO
counter increment, so threshold=2 actually required 3 PTOs.
Increment first; threshold semantics now match the constant.
- Bug 7: applyPeerPathResponseLocked didn't reset
consecutivePtoCount on successful validation; the next sleep
inherited a stale exponential-backoff multiplier even though
the peer just proved liveness.
Code quality:
- Rename ValidationOutcome.Validated.newConnectionIdBytes →
connectionId; PathValidationState.Validating.newCidBytes →
newConnectionId. The "Bytes" suffix was redundant.
- Drop unused PathValidator(initialActiveCidSequence) parameter.
- Drop dead coerceAtLeast(2) in pool size calculation.
- Make pendingChallenges and pendingRetireSequences internal.
- Fix stale KDoc references (activatePendingValidatedCid,
forceRetireActiveIfNeeded, "retirePriorTo decreased" — none
survived the §19.15 clamp fix).
- PathValidator.RecordResult: drop RetirePriorToRegressed
enum value (clamped, never returned).
- Surface qlogObserver.onConnectionIdRetired in both the
success and timeout paths.
Tests:
- ClientPathMigrationTest: existing fullMigrationRoundTrip
test now asserts DCID rotates AT challenge time, not on
PATH_RESPONSE.
- New retireConnectionIdForSequenceZeroClosesConnection.
- New pathResponseSuccessResetsConsecutivePtoCount.
- New triggerPathMigrationBeforeHandshakeReturnsNotConnected.
- PathValidatorTest:
validationTimeoutAfter3PtoTransitionsToFailedAndRetiresFailedCid
now asserts the failed sequence is queued for retire.
- retirePriorToRegressionIsRejected → renamed to
retirePriorToRegressionIsClampedNotRejected.
All :quic:jvmTest and :nestsClient:jvmTest pass.
https://claude.ai/code/session_01PVVhSQXvw4K4oQ46FzpgaT
|
||
|
Claude
|
435c49bae9 |
feat(quic): client-initiated path validation + DCID rotation (RFC 9000 §9)
Implements the client side of connection migration so a path that
stops receiving ACKs (NAT rebind, route flap, dead peer) can be
recovered without a fresh handshake:
1. NEW_CONNECTION_ID frames from the server are stored in a
PathValidator pool (was: parsed and dropped).
2. After PATH_PROBE_PTO_THRESHOLD consecutive PTOs, the driver
calls triggerPathMigrationLocked(); the validator picks an
unused CID and queues a PATH_CHALLENGE with a CSPRNG payload.
3. The writer drains the challenge into the next outbound 1-RTT
packet using the new DCID; a RecoveryToken.PathChallenge is
attached so loss recovery can re-queue on packet drop.
4. Inbound PATH_RESPONSE that byte-equals the outstanding payload
promotes destinationConnectionId to the new bytes and queues
RETIRE_CONNECTION_ID for the prior sequence.
5. RFC 9000 §8.2.4: validation is abandoned after 3 * PTO;
timeout transitions to PathValidationState.Failed for retry.
Spec coverage:
- §5.1.1 initial DCID is sequence 0
- §5.1.2 retire_prior_to enforcement (clamping per §19.15
reordering rule, force-retire of cached entries below
watermark)
- §8.2.2 byte-equal payload match
- §8.2.4 3 * PTO abandonment
- §19.15 frame-encoding error checks (retire_prior_to >
sequence_number, invalid CID/token length)
- §19.16 RETIRE_CONNECTION_ID frame codec + protocol-violation
close on retire of an unissued sequence
Observability: QlogObserver gains onPathValidationStarted /
Succeeded / Failed and onConnectionIdActivated / Retired hooks
for qvis sequence diagrams.
Tests: PathValidatorTest (state-machine unit) +
ClientPathMigrationTest (full round-trip through InMemoryQuicPipe:
NEW_CONNECTION_ID -> trigger -> PATH_CHALLENGE -> PATH_RESPONSE ->
DCID rotated + RETIRE_CONNECTION_ID emitted). Existing
PathValidationTest (peer-initiated PATH_CHALLENGE echo) continues
to pass unchanged.
https://claude.ai/code/session_01PVVhSQXvw4K4oQ46FzpgaT
|
||
|
Vitor PamplonaandClaude Opus 4.7
|
5b8bd021b0 |
fix(quic): exempt retired stream ids from client-initiated squatting guard
The audit-4 #5 guard ran before the existing phantom-stream check, so an msquic-style aggressive STREAM retransmit on a stream we'd opened and retired (peer's loss-detector refire racing our FIN-ACK) closed the connection with STREAM_STATE_ERROR. Observed in the parallel `transfer` interop test where retransmits on retired streams 0/4 truncated whichever URL was still mid-receive (5 MB → 2.2 MB). Add `!isStreamIdRetiredLocked` to the guard so legitimate retransmits fall through to the existing silent-drop branch. Genuine squatting on never-opened CLIENT_* ids still closes — the existing FrameRoutingTest case stays green because id 0 is never put into the retired ring. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
Claude
|
71e14fe639 |
chore(quic): audit cleanup — drop redundant copy, rename queue, extract test fixture
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 |
||
|
Claude
|
afe3aaf020 |
feat(quic): RFC 9000 §8.2 server-initiated path validation (PATH_CHALLENGE / PATH_RESPONSE)
Soak target #4 — minimum viable path validation. Lands the spec-required peer-initiated case so a server probing the path (e.g. after a NAT rebind, or post-CID rotation) sees a matching PATH_RESPONSE and doesn't declare the path dead. Pre-fix the parser decoded PATH_CHALLENGE / PATH_RESPONSE bytes but threw the result away — a peer's challenge went silently into the void. After ~3 RTT of no response, a strict peer would mark the path dead and tear the connection down (visible to audio-rooms users as a sudden cut on a phone that briefly switched cells). Implementation: - Add PathChallengeFrame / PathResponseFrame data classes; wire decode and encode (was decode-and-discard previously). - Add `pendingPathResponses` queue on QuicConnection (bounded at MAX_PENDING_PATH_RESPONSES = 64 to defend against challenge flood; excess silently dropped — peer retries on PTO). - Parser handler queues a response on inbound PATH_CHALLENGE. - Writer drains the queue in buildApplicationPacket. RFC 9000 §13.3 doesn't list PATH_RESPONSE as ack-eliciting-and- retransmittable; if a response is lost, the peer's next PATH_CHALLENGE re-queues it and we respond again. Out of scope for this landing (multi-day each, parked unless production evidence requires): - Client-initiated migration: requires UdpSocket replacement, new-CID acquisition tracking, validating new path BEFORE moving traffic to it. - Anti-amplification on unvalidated paths (RFC 9000 §8.1). Tests (PathValidationTest, 6 cases): - PATH_CHALLENGE / PATH_RESPONSE codec round-trip + 8-byte length validation. - End-to-end: peer PATH_CHALLENGE → client PATH_RESPONSE with byte-equal payload. - Multi-challenge fan-in: 3 challenges → 3 distinct responses (in any order; matched by content). - Flood cap: 256 challenges → ≤ MAX_PENDING_PATH_RESPONSES responses, connection stays CONNECTED. https://claude.ai/code/session_018KPKWRg5baX5Anf7zfEyec |
||
|
Claude
|
be8f2e08d3 |
feat(quic+amethyst): close-under-load + key-update PN gate + loss harness depth + foreground recycle
Working through the punch list from the prior "what's left?" status. #2 close-under-load (CloseUnderLoadTest, 3 cases) ================================================== Pins three races between connection close and active stream traffic that the existing idle-driver close test doesn't cover: - closeWhileBulkStreamRetirementIsRunning — server ACKs 100 in-flight client-bidi streams in one shot, retire pass + close fire concurrently. Asserts CLOSED status and no Flow leak. - closeWhileAppCoroutinesAreOpeningStreamsDoesNotDeadlock — pins the lock-ordering invariant: streamsLock (openers) and lifecycleLock (close) don't fight. - closeWhilePeerStreamsAreInFlight — close fires mid-stream of 50 server-uni group streams (half FIN'd, half not). Every incoming Flow terminates promptly with whatever bytes the parser had already delivered. #3 PN-gate / try-previous-fall-through-to-next for key updates ============================================================== Closes the KNOWN-LIMITATION I documented in the prior round. QuicConnectionParser previously routed mismatched-KEY_PHASE packets unconditionally to previousReceiveProtection if non-null, which silently dropped consecutive-rotation packets (KEY_PHASE wraps back to its prior value, prior keys are now wrong, AEAD fails, connection wedges). Fix follows neqo's shape: try previous keys; on AEAD failure fall through to next-phase derivation. Two AEAD attempts on a mismatched-phase packet are cheap; KEY_PHASE mismatch is rare. The previously-disabled twoConsecutiveRotationsCommitCorrectly test now passes. #4 loss harness depth (MoqLiteLossHarnessTest, 3 added cases) ============================================================= First-pass harness from the previous round was a single 5%-loss moq-lite shape. Added: - listenerToleratesPacketReorderingOnGroupStreams — random permutation of 50 group-stream datagrams, asserts 100% delivery. Pins the reorder contract for moq-lite. - listenerSurvivesExtremeTwentyPercentLoss — 200 streams at 20% loss, asserts ≥ 60% delivery and connection stays CONNECTED. Catches catastrophic-collapse regressions in flow-control / ACK-tracker / retired-id ring under stress. - reliableBidiStreamRecoversFromMidStreamPacketLoss — drops the middle two of four STREAM frames on a reliable bidi stream, retransmits, asserts the consumer surfaces the full contiguous payload. Pins the reliability contract distinct from the best-effort moq-lite path. #1 foreground-resume recycle (AppForegroundRecycleHook, 5 tests) ================================================================ Closes the user-visible production gap. ReconnectingNestsListener already orchestrates retry on terminal state and observes NestNetworkChangeBus for network-handover recycles. The missing piece was a foregrounding signal: when Android reclaims the app's UDP socket FD after backgrounding (typical at ~30 s+, network itself still up so the connectivity callback doesn't fire), the QUIC connection sits dead until the next send-loop throw — which landed last round. This hook publishes a NestNetworkChangeBus event when the app returns to foreground after spending ≥ 5 s in background. The pre-existing wiring observes that event and calls recycleSession() on every active listener / speaker. Pure-state core (AppForegroundCounter) is testable without Robolectric; JUnit-4 unit tests pin the threshold logic, multi-activity counter behaviour (e.g. PIP), and consecutive-cycle correctness. Wired into Amethyst.Application.onCreate via registerActivityLifecycleCallbacks. https://claude.ai/code/session_018KPKWRg5baX5Anf7zfEyec |
||
|
Claude
|
6706c111b5 |
feat(quic): peer-initiated key-update verification + send-loop death surfaces as CLOSED
#2 KEY UPDATE VERIFICATION (soak target #2) Added KeyUpdatePeerInitiatedTest pinning the RFC 9001 §6 peer-initiated 1-RTT key update path against InMemoryQuicPipe: - peerInitiatedRotationCommitsAndMirrorsOnSend — single rotation flips currentReceiveKeyPhase, mirrors currentSendKeyPhase, retains pre-rotation keys as previousReceiveProtection, installs new receive+send protections; connection stays CONNECTED. - reorderedPacketOnPriorKeysStillDecryptsAfterRotation — packet sent before peer rotated but arriving after the rotation triggering packet decrypts via previousReceiveProtection (RFC 9001 §6.1 reorder window). - postRotationOutboundPacketCarriesNewKeyPhaseAndDecryptsForPeer — writer stamps currentSendKeyPhase into the short header AND encrypts with the rolled-forward send keys. Test infrastructure: InMemoryQuicPipe grows rotateServerApplicationKeys (walks the same HKDF-Expand-Label "quic ku" dance the production peer would) plus buildServerApplicationDatagramWithPriorKeys (re-emits via the stashed pre-rotation TX, exercising the reorder-window path). Documented limitation: consecutive rotations within the reorder window mis-route via previousReceiveProtection. The spec-correct fix is to gate previousReceiveProtection on a packet-number threshold (neqo / picoquic shape); for the audio-rooms 3-hour scenario, a single rotation is the realistic case so this is a follow-on rather than a blocker. No test asserts the broken behaviour. #3 RECONNECT-ON-FOREGROUND (soak target #3) ReconnectingNestsListener already has all the orchestration (exponential-backoff retry, JWT-refresh recycle, recycleSession() hook for platform network-change events). What was missing at the QUIC level: when the OS reclaims the UDP socket FD while the app is backgrounded, socket.send() throws and the bare exception escapes the SupervisorJob silently. The connection sits in HANDSHAKING / CONNECTED indefinitely and the orchestrator's terminal-state listener never fires — the room screen shows "live" while audio is dead. Wrapped sendLoop in try/catch mirroring the existing readLoop's finally block: any uncaught Throwable (CancellationException excepted, since close() is already driving teardown) calls markClosedExternally with the cause. Also wired markClosedExternally to record closeReason on first-call so observability surfaces the human-readable cause through to NestsListenerState.Failed.reason. Pinned by socketDeathMidSessionFlipsConnectionToClosed — runs the driver, tears the UDP socket out from under it, asserts status flips to CLOSED within 5 s and the close reason mentions the loop death. Pre-fix this would loop forever waiting for status to move. Tests: - KeyUpdatePeerInitiatedTest (3 cases) - QuicConnectionDriverLifecycleTest::socketDeathMidSessionFlipsConnectionToClosed https://claude.ai/code/session_018KPKWRg5baX5Anf7zfEyec |
||
|
Claude
|
c65eef6927 |
feat(quic): heap-sampling soak test, FD-leak canary, phantom-stream guard, loss harness
Follow-up to b8c6e080 addressing the gaps I called out in the "is this the best we can do?" reply. 1. **Heap-sampling soak test** (`QuicHeapSoakTest`). Long-form, default- skipped via `-PquicSoakSeconds=N` propagated by `quic/build.gradle.kts` to the jvmTest task. Without the property the test early-returns with a printed SKIPPED line, so `./gradlew test` stays fast for CI. With the property, drives moq-lite-shaped peer-uni churn at ~50 streams/s for N seconds, samples `totalMemory - freeMemory` six times across the run, and fails if the post-warmup → final delta exceeds 10 MB (the acceptance threshold from the audio-rooms soak prompt). Production use: `-PquicSoakSeconds=1800` for the 30-minute soak. 2. **FD-leak canary** added to `QuicConnectionDriverLifecycleTest`. On Linux, samples `/proc/self/fd` size before / after the 100-session loop; banded at +16 entries for ambient JVM noise. macOS / Windows silently no-op because /proc isn't there. Catches socket / pipe leaks the thread-count check would miss. 3. **Phantom-stream guard.** Added `retiredStreamIdSet` (capped FIFO ring at 4 096 entries, ~80 s of moq-lite churn) plus `isStreamIdRetiredLocked` on the connection. Parser checks before `getOrCreatePeerStreamLocked` and drops STREAM frames the peer retransmits on already-retired streams. Eliminates the "duplicate ACK lost → peer retransmits FIN → we mint a phantom QuicStream" edge case I papered over in the previous commit. Pinned by `phantomGuardDropsRetransmitOnRetiredPeerStream`. 4. **moq-lite loss harness** (`MoqLiteLossHarnessTest`). First pass at soak target #5: drive 50 best-effort group streams with 5% uniform packet loss, assert the listener surfaces ≥ 90% with payloads intact and the connection stays CONNECTED. Out of scope here: reorder injection, latency-under-loss measurement, full end-to-end with a real moq-lite publisher. Tests: - `QuicHeapSoakTest` — gated, validates 10MB heap acceptance band. - `QuicConnectionDriverLifecycleTest::repeatedSessionLifecycleDoesNotLeakThreads` — now also enforces /proc/self/fd bound. - `StreamRetirementSoakTest::phantomGuardDropsRetransmitOnRetiredPeerStream` — pins the duplicate-frame drop semantics. - `MoqLiteLossHarnessTest` — 2 cases (lossy + lossRate=0 baseline). https://claude.ai/code/session_018KPKWRg5baX5Anf7zfEyec |
||
|
Claude
|
bfc983bff7 |
feat(quic): retire fully-settled streams to keep tracker bounded under audio-room churn
Soak target #1 from the audio-rooms hardening pass: moq-lite over QUIC mints one peer-uni stream per Opus frame, so a 3-hour broadcast at ~50 frames/sec accumulated ~540 000 stream entries in `QuicConnection.streamsList` / `streams` for the lifetime of the session. The two structures were append-only — closed streams were filtered out of the writer's iteration but never removed — and the heap grew monotonically. Adds `QuicStream.isFullyRetired` plus `retireFullyDoneStreamsLocked` on the connection. The writer drains the retire pass at the top of `buildApplicationPacket`, dropping streams whose send side has peer-acked FIN/RESET and whose receive side has both FIN'd and fully drained into the application's incoming Channel. The cumulative receive high-water folds into `retiredStreamsRecvBytes` so the connection-level MAX_DATA accounting in `appendFlowControlUpdates` keeps advertising the lifetime total — without that seed, retiring K bytes would silently regress the peer's send credit. Also adds soak target #6 coverage: `QuicConnectionDriver` now exposes `driverJob` / `closeTeardownJob` for test assertion, and the new `QuicConnectionDriverLifecycleTest` cycles 100 sessions against a localhost UDP blackhole to pin idempotent close + bounded thread growth. Tests: - `StreamRetirementSoakTest` (4 cases): local-uni FIN+ACK retirement, peer-uni listener-path retirement, MAX_DATA accounting preservation across retire, and a 10 000-stream churn harness that asserts the working set stays bounded. - `QuicConnectionDriverLifecycleTest` (2 cases): close idempotency and 100-session thread-leak canary. https://claude.ai/code/session_018KPKWRg5baX5Anf7zfEyec |
||
|
Vitor PamplonaandClaude Opus 4.7
|
d1567e4a53 |
fix(quic): faster PTO with INITIAL_RTT=100ms and unified ptoBaseMs path
multiconnect handshakeloss / handshakecorruption tail-fail under the runner's 30% packet drop / bit-flip scenarios because each PTO retransmit chance gives ~49% one-side success (0.7² for both directions clear). With INITIAL_RTT=333ms the first PTO fires at 999 ms and doubling tops out at ~5 attempts in 30s — across 50 sequential connections, ~5% probability some iteration runs out of retransmits before the per-iter budget. Two coupled changes: 1. INITIAL_RTT_MS 333→100. RFC 9002 §6.2.2 spec-allowed (the standard default but explicitly configurable). Matches Chrome and Firefox/neqo. Pre-sample PTO is now 300 ms instead of 999 ms; doubling fits ~8 retransmit attempts in 30s instead of 5, pushing per-iter loss-recovery success past 99% under 30% drop. Spurious retransmits on slow paths are harmless (peer dedupes by packet number) and smoothed_rtt converges in one round-trip. 2. QuicConnectionDriver always uses lossDetection.ptoBaseMs() for the PTO timer, including before the first RTT sample. Pre-fix the driver hardcoded 1000ms as a "handshake-timeout safety floor" that ignored INITIAL_RTT_MS entirely — the PTO was always 1s pre-handshake regardless of the constant. Now both pre- and post-sample regimes go through the same calculation. max_ack_delay is gated to APPLICATION space (RFC 9002 §6.2.1) so pre-handshake PTOs aren't padded with the peer's quoted delay. Two pre-existing tests (PtoTest, QuicLossDetectionTest) hard-coded expected PTO durations derived from the old 333 ms constant; updated them to express the relationships in terms of INITIAL_RTT_MS so future tweaks don't desync. Result: 21/21 against aioquic, picoquic, quic-go (handshake, multiplexing, longrtt, transferloss, transfercorruption, handshakeloss, handshakecorruption all pass on each peer). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
Vitor PamplonaandClaude Opus 4.7
|
b622d0c936 |
feat(quic): RFC 9001 §6 1-RTT key update
quic-go initiates a 1-RTT key update partway through every transferloss
or transfercorruption test (KEY_PHASE bit flips 0→1 around server pn=100
by default). Pre-fix our parser used the OLD application keys for every
post-update packet, AEAD-failed all of them, never sent another ACK,
the server fell into PTO mode, and throughput collapsed (~24kbps over
60s vs the 10Mbps the path supports).
The fix is end-to-end:
- ShortHeaderPacket.peekKeyPhase: HP-unmasks just the first byte to
surface the key-phase bit BEFORE running AEAD. The parser uses this
to pick the right keys instead of paying for a doomed AEAD.
- QuicConnection: tracks the live application secrets (server- and
client-side) and current send/receive key phase, plus a
previousReceiveProtection slot for RFC §6.1 reorder-window decryption.
deriveNextPhaseReceiveKeys derives the next phase via
HKDF-Expand-Label("quic ku", "", Hash.length) without committing;
commitKeyUpdate installs them only after AEAD has succeeded, then
rolls the send side forward in lockstep so our next outbound
carries the matching KEY_PHASE bit (peer needs that to confirm the
rotation completed). HP key is NOT rotated, per spec.
- QuicConnectionParser.feedShortHeaderPacket: three-way dispatch on
the peeked bit — matches current → live keys; matches retained
previous → previous keys (reordered packet); else → derive
next-phase, attempt AEAD, commit on success.
- QuicConnectionWriter: ShortHeaderPlaintextPacket(... keyPhase =
conn.currentSendKeyPhase) at both 1-RTT build sites (steady-state
and CONNECTION_CLOSE).
We don't drive key updates ourselves — only echo the peer's. Avoids
the bookkeeping for RFC 9001 §6.6 packet-count limits and the safety
benefits of voluntary rotation aren't load-bearing at our connection
scale.
Tests: peekKeyPhase round-trip + long-header rejection;
2-byte-pn round-trip when largestReceived is far behind (the original
suspected-but-not-actual cause before the key-phase reveal).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
||
|
Claude
|
9cd8d0a6ee |
diag(quic): writer-side per-drain stats behind DEBUG=1 build flag
The qlog confirmed the writer emits 1 STREAM frame per packet on the
live wire, while MultiplexingCoalescingTest + MultiplexingAioquicTpsTest
both show ~9 streams per packet under synchronous drain. So the bug is
in the live driver flow — concurrent send loop + parser feed +
real-socket interleaving — and not in buildApplicationPacket itself.
To localize: add an opt-in trace at the END of buildApplicationPacket
that dumps per-drain state when QUIC_INTEROP_DEBUG=1:
[writer.app] frames=N stream_frames=K streamsView=M active=A
packetBudget_remaining=R connBudget_initial=C
- frames vs stream_frames tells us if non-stream frames (ACK,
MAX_DATA, MAX_STREAM_DATA) are bloating the packet
- active vs streamsView tells us if isClosed filter dropped streams
- packetBudget_remaining tells us if we hit the 64-byte break early
- connBudget_initial tells us if conn flow control was zero
Wired three pieces:
1. WriterDebug.kt — a single @Volatile boolean owned by commonMain,
`writerDebugEnabled`. Off by default.
2. InteropClient.main flips it to true if QUIC_INTEROP_DEBUG=1 is set
in the env.
3. Dockerfile + Makefile accept --build-arg DEBUG=1 (or `make build
DEBUG=1`) to bake the env var into the image.
Usage:
cd quic/interop
make build DEBUG=1
cd ../..
./quic/interop/run-matrix.sh -s aioquic -t multiplexing
cat ../quic-interop-runner/logs/run-*/aioquic_amethyst/multiplexing/output.txt | grep '^\[writer'
When off, cost is one volatile read in the writer hot path — negligible.
https://claude.ai/code/session_01HcvfQq1ttPV9PkRoJb4nyT
|
||
|
Claude
|
4616234c82 |
diag(quic-interop): dump first 10 packets fully + add live-driver-flow synth test
Two adds for the multiplex investigation.
(1) inspect-multiplexing.sh: previous histograms aggregate over the
whole run. Add full-frame-array dump of the first 10 packet_sent
AND first 10 packet_received events so we can see whether the
FIRST chunk burst (~13 streams in one packet) or dribbled (1 per).
(2) MultiplexingAioquicTpsTest: synchronous drain test using EXACTLY
the TPs aioquic gave us in the failing run (initial_max_data=1MB,
initial_max_stream_data_bidi_remote=1MB, initial_max_streams_bidi=
128) and ~80-byte HEADERS-frame-sized payloads. PASSES with 7
packets / 9.1 streams per packet — proving the writer's coalescing
is fine under aioquic's flow-control budget. So the bug is NOT in
the writer; it's in the live driver flow that
MultiplexingCoalescingTest doesn't exercise (concurrent send loop
+ parser + real socket).
The first-10 dump from inspect should localize this further:
- if first packet has 13 stream frames → writer burst, bug is
server-side timing or driver-loop scheduling
- if first packet has 1 stream frame → writer producing 1 per call
in production for some condition my synchronous test doesn't
cover
https://claude.ai/code/session_01HcvfQq1ttPV9PkRoJb4nyT
|
||
|
Claude
|
6bcee12669 |
audit-r2(quic): tighten batched-open API tests + docs
Round-2 audit of the openBidiStreamsBatch / openUniStreamsBatch landing. (1) Tests overpromised. The previous "holds streamsLock for the whole batch" test only verified the API didn't crash and stream ids were unique. A future regression that released the lock between opens (the 2026-05-06 bug shape) would not break it. Added `assertTrue(client.streamsLock.isLocked)` inside both batch init lambdas. Now the test name matches what's verified. (2) `*Batch` docstrings didn't warn that `init` runs under streamsLock. A naive caller might do encoding / IO inside, defeating the lock-hold-time goal that motivated pre-encoding outside. Added the warning + the canonical caller shape (encode outside, enqueue inside) to both function docs. (3) Empty-batch corner: an empty `items` list was still acquiring the lock and entering withLock. Added an `if (items.isEmpty()) return emptyList()` short-circuit. New test pins the contract — `init` must not run for an empty batch. Test count: 6 → 7. All green; no production-API change. https://claude.ai/code/session_01HcvfQq1ttPV9PkRoJb4nyT |
||
|
Claude
|
a0a604b8e7 |
refactor(quic): kill dead levelLock + add bug-resistant batched-open API
Audit follow-up. Two cleanups consolidated into one commit since they
share the goal of "make the lock contract obvious from the API".
(1) Remove LevelState.levelLock entirely.
The lock-split refactor introduced a per-level Mutex with a docstring
claiming the writer/parser would acquire it around encode + sentPackets
record + ACK observation. Neither actually does. SendBuffer's internal
synchronized(this) is what serializes cryptoSend mutations against
takeChunk and markAcked. handlePtoFired's levelLock acquisition was
the only production usage and it serialized only against itself.
Removed:
- LevelState.levelLock
- handlePtoFired's withLock wrapper (now a non-suspend fun)
- All docstring references to a third lock domain
The pre-existing sentPackets HashMap race (writer mutates under
streamsLock, parser reads without sync) is unchanged — out of scope
for this PR. Acquisition order is now `lifecycleLock → streamsLock`,
flat.
(2) Add openBidiStreamsBatch + openUniStreamsBatch — the bug-resistant
high-level API for the prepareRequests / moq audio-rooms patterns.
The previous shape required callers to manually do
`streamsLock.withLock { repeat(N) { openBidiStreamLocked() ... } }`.
That contract regressed twice on this very branch: once held the wrong
lock (lifecycleLock alias), once skipped the wrap entirely. Both shapes
silently emitted one STREAM per packet under multiplex load.
The new API encapsulates the lock + the per-item init lambda:
conn.openBidiStreamsBatch(items) { stream, item ->
stream.send.enqueue(encode(item))
stream.send.finish()
Handle(stream)
}
Callers physically cannot hold the wrong lock. Migrated:
- Http3GetClient.prepareRequests
- HqInteropGetClient.prepareRequests
Also added `openUniStreamLocked` + `openUniStreamsBatch` symmetric to
the bidi versions. moq audio-rooms eventually wants to open many uni
streams in burst; the same one-stream-per-packet bug lurks if every
open serializes through its own lock acquisition.
`openBidiStreamLocked` / `openUniStreamLocked` remain public (with
their `check(streamsLock.isLocked)` guards) for the rare custom-batch
callers that need to mix bidi+uni opens under a single hold. Most
callers should use the *Batch variants going forward.
Test coverage extended:
- openBidiStreamsBatch happy path
- openUniStreamLocked throws without streamsLock
- openUniStreamsBatch happy path
Six tests in BatchedOpenLockContractTest now pin the contract.
https://claude.ai/code/session_01HcvfQq1ttPV9PkRoJb4nyT
|
||
|
Claude
|
07dd572423 |
perf+test(quic): audit follow-ups for the multiplex lock fix
Audit of recent multiplex / PTO commits surfaced three concrete improvements: 1. **Pre-encode requests outside streamsLock** in both prepareRequests impls. QPACK encoding (Http3GetClient) and string formatting (HqInteropGetClient) are non-trivial under multiplex load — 1999 paths means we were holding streamsLock across all 32 chunks × ~2KB of QPACK encoding per chunk = ~64 KB of CPU work serialised against the send loop. Now done outside the lock. 2. **Fix O(N²) byte merging in MultiplexingRoundTripTest.** Previous shape allocated a new array per STREAM frame; for real-size payloads this would dominate test runtime. Switched to a per- stream MutableList<ByteArray> joined once at the end. 3. **Pin coalescing end-to-end in MultiplexingRoundTripTest.** Pre-existing test verified per-stream content arrived but said nothing about the wire shape. Added totalDatagrams ≤ 12 assertion so the matrix's "one stream per datagram" failure mode would break the test instead of passing silently. Audit also surfaced two non-actionable items, flagged for future cleanup but not changed in this commit: - LevelState.levelLock docstring claims writer/parser acquire it around sentPackets mutations; in practice neither does. The handlePtoFired call that takes levelLock currently serialises only against itself; SendBuffer's internal synchronized is what prevents the actual cryptoSend race. Kept the call (matches the documented design intent and future-proofs against the writer actually taking levelLock) but the docstring is stale. - openBidiStreamLocked's check(streamsLock.isLocked) catches "no lock" and "wrong lock" callers but not "another coroutine holds streamsLock and I'm calling without holding it" — kotlinx Mutex doesn't expose owner-aware checks without an explicit owner arg we don't pass. Acceptable since the bug we just fixed and any future regression in the same shape are caught. https://claude.ai/code/session_01HcvfQq1ttPV9PkRoJb4nyT |
||
|
Claude
|
991b1a1da3 |
fix(quic-interop): prepareRequests must hold streamsLock, not lifecycleLock
aioquic interop multiplexing 2026-05-06 qlog post-mortem:
- 2898 packets sent in 60s, each carrying ONE STREAM frame
- server log: streams created/discarded strictly serially, ~30-40ms apart
- 1421/2000 files completed before the runner's 60s timeout
- shape ≈ 1 RTT per stream — wire was emitting one stream per datagram
Cause: Http3GetClient.prepareRequests + HqInteropGetClient.prepareRequests
both did `conn.lock.withLock { ... openBidiStreamLocked() }`. Post the
lock-split refactor `conn.lock` is the deprecated alias for lifecycleLock.
The writer's drainOutbound takes streamsLock — not lifecycleLock — so the
send loop interleaved between every two openBidiStreamLocked calls,
draining one stream's data per pass.
Fix:
1. Both prepareRequests impls now use conn.streamsLock.withLock.
2. openBidiStreamLocked now `check`s streamsLock.isLocked at entry
so this can never silently regress again — calling it without the
lock (or with the wrong lock) throws IllegalStateException with
a message naming streamsLock as the lock to acquire.
3. New BatchedOpenLockContractTest pins the contract:
- calling openBidiStreamLocked WITHOUT any lock throws
- calling openBidiStreamLocked while holding lifecycleLock throws
(the exact regression shape)
- calling openBidiStreamLocked while holding streamsLock works
(happy path)
The runtime check is the regression-proof part: future callers physically
cannot hold the wrong lock without the test (and prod) blowing up at the
first call site.
https://claude.ai/code/session_01HcvfQq1ttPV9PkRoJb4nyT
|
||
|
Claude
|
46926a712b |
test(quic): in-process matrix-shape multiplexing round-trip
The runner's `multiplexing` testcase opens N parallel bidi streams,
each downloading one file, and asserts every file's content lands
on the right stream. Existing tests cover pieces of that:
- MultiplexingThroughputTest: opens 1000 streams in <2s — measures
lock contention but never moves bytes server-side.
- MultiplexingCoalescingTest: pins that 64 streams coalesce into ≤6
packets — encoder contract, no end-to-end.
- MultiStreamFinDeliveryTest: server pushes responses to 50 streams,
client surfaces every FIN — but the CLIENT never sends a STREAM
frame in that test, so any regression in the writer's request-
side multiplex path is invisible.
This test runs the full request → response loop:
1. Open 64 parallel bidi streams
2. Each enqueues a tiny request + FIN
3. Drain client outbound → decrypt → assert all 64 STREAM frames
made it across, with their request bytes intact
4. Server sends one response per stream + FIN
5. Per-stream incoming.toList() must yield the expected response
Failures call out the specific stream id, so a regression points
at "stream X dropped its FIN" instead of a generic timeout.
64 streams keeps wall-clock under a second; the bug class the test
guards against (per-stream loss / mis-routing) fires identically at
64 and 1999.
https://claude.ai/code/session_01HcvfQq1ttPV9PkRoJb4nyT
|
||
|
Claude
|
b579c766a4 |
test(quic): exercise the actual driver PTO path, not a simulation
The existing PtoCryptoRetransmitTest simulated the driver inline: it set pendingPing=true and called requeueAllInflightCrypto by hand, then asserted the next drain emitted CRYPTO. That checked the helpers worked but never noticed when the DRIVER stopped calling requeueAllInflightCrypto — which is exactly the regression that bit us in commits |
||
|
Claude
|
d920bf8fd0 | Merge branch 'worktree-agent-acb67f8575e4086eb' into claude/research-quic-libraries-hH1Dc | ||
|
Claude
|
ef4bb99988 |
refactor(quic): split conn.lock into streamsLock + per-level lock + lifecycleLock
The single connection-wide `QuicConnection.lock` mutex serialised every
critical path: the read loop's `feedDatagram`, the send loop's
`drainOutbound`, and every public mutator (`openBidiStream`,
`streamById`, `flowControlSnapshot`, ...). The multiplexing testcase
opens hundreds of bidi streams in parallel and was capped at ~25
streams/sec by lock contention against the I/O loops.
Phase 1 of the lock split (see
`quic/plans/2026-05-08-lock-split-design.md`) introduces three
domain-specific mutexes:
- `streamsLock` — streams registry, datagram queues, stream-id
counters, connection-level flow-control bookkeeping, pending-
retransmit maps for control frames
- `LevelState.levelLock` (one per encryption level) — per-level
pnSpace / sentPackets / ackTracker / CRYPTO buffers
- `lifecycleLock` — status transitions, close reason/error code
Acquisition order: `lifecycleLock < streamsLock < levelLock`.
Per-stream `synchronized(this)` blocks inside SendBuffer/ReceiveBuffer
remain at the leaf — never acquire any QuicConnection mutex while
holding a per-stream lock.
The legacy `lock: Mutex` field is preserved as a deprecated alias of
`lifecycleLock` for source-compatibility with external test harnesses;
new code MUST use the appropriate domain lock.
Highlights:
- `feedDatagram` / `drainOutbound` now require the caller to hold
`streamsLock`; the driver wraps each call. Phase 1 keeps the whole
feed/drain inside `streamsLock` for safety; phase 2 (deferred) will
split frame-collection from encrypt + sentPackets-record so app
coroutines can intersperse during the encrypt window.
- `pendingPing`, `peerTransportParameters`, `status`,
`handshakeComplete` are now @Volatile so observers read them
without a lock.
- `markClosedExternally` no longer needs any lock (status is
@Volatile, signals are channel-thread-safe).
- Driver's PTO bookkeeping uses the volatile fields directly — no
lock needed.
- Tests that manually acquired `conn.lock` to call
`getOrCreatePeerStreamLocked` / `onTokensAcked` / `onTokensLost`
now acquire `streamsLock` (the domain those routines mutate).
- New `MultiplexingThroughputTest` locks in the contract: 1000
parallel `openBidiStream` calls must complete in <2 s.
Test plan:
- `:quic:jvmTest` — 294 tests pass (293 prior + 1 new throughput).
- `MultiplexingThroughputTest`: 1000 bidi streams in 52 ms
(~19,000 streams/sec on the in-memory pipe), well above the
250+/sec target.
- `:nestsClient:compileKotlinJvm` — clean, no API breaks.
- `./gradlew :quic:spotlessApply` — clean.
https://claude.ai/code/session_01HcvfQq1ttPV9PkRoJb4nyT
|
||
|
Claude
|
7ed3d55b31 |
test(quic): pin multiplexing coalescing contract
Two unit tests for the multiplexing throughput problem we just fixed
in the InteropClient (commit
|
||
|
Claude
|
17b80270d9 |
fix(quic): preserve Initial PN namespace across Retry (RFC 9001 §5.7)
aioquic's retry test result, surfaced via qlog: Check of downloaded files succeeded. Client reset the packet number. Check failed for PN 0 Our applyRetry called LevelState.resetForVersionNegotiation, which creates a fresh PacketNumberSpaceState() — resetting PN to 0. The qlog confirmed: PN=0 sent at t=388 (pre-Retry ClientHello), then PN=0 again at t=1468 (post-Retry retried ClientHello). Same PN reused across the boundary. RFC 9001 §5.7 + RFC 9000 §17.2.5: the Initial PN namespace CONTINUES across the Retry boundary. The new Initial keys are derived from the new DCID, but PN doesn't reset. Reusing a PN under different keys makes the runner's pcap-decryption check fail (it's also a security concern in the general case, hence the strict spec rule). Fix: new LevelState.resetForRetry that's identical to resetForVersionNegotiation EXCEPT it preserves pnSpace. applyRetry calls resetForRetry. Two regression tests updated to assert the post-Retry Initial uses PN=1 (continues from PN=0 of the pre-Retry attempt) rather than PN=0 (the buggy reset behavior). For Version Negotiation the original semantics still apply (RFC 9000 §6.2: client treats VN as if the original Initial was never sent; PN reset to 0 is correct). This should bring the retry testcase from ✕(S) to ✓(S) against servers that exercise the Retry path. The handshake / transfer already succeeded over the Retry per the qlog (the server's check "Check of downloaded files succeeded." passed); only the PN-reuse flag was failing the test. https://claude.ai/code/session_01HcvfQq1ttPV9PkRoJb4nyT |
||
|
Claude
|
0107bbbac9 | Merge branch 'worktree-agent-a5e247c7b4025a83c' into claude/research-quic-libraries-hH1Dc | ||
|
Claude
|
cfc305feb3 |
feat(quic): add qlog observer infrastructure for interop diagnostics
Hooks every QUIC protocol decision (packets sent / received / dropped, TLS key updates, transport params, ALPN, loss detection, PTO, close) into a [QlogObserver] interface. Production callers default to [QlogObserver.NoOp] (zero allocation, single virtual call); the :quic interop runner wires a [QlogWriter] writing JSON-NDJSON (qlog 0.3 / JSON-SEQ format) to `<QLOGDIR>/client.sqlog`, consumable by qvis (https://qvis.quictools.info/) and Wireshark. Goal: every interop-runner test failure produces a qlog file the operator can drop into qvis to see exactly what we did differently from the spec. Hooked call sites: - QuicConnection.start → connection_started, parameters_set(local), version_information - QuicConnection.close → connection_closed(local) - QuicConnection.markClosedExternally → connection_closed(remote) - QuicConnection.applyPeerTransportParameters → parameters_set(remote) - QuicConnection.tlsListener (handshake/app keys) → security:key_updated - QuicConnection.tlsListener (handshake done) → alpn_information - QuicConnectionWriter.buildLongHeaderFromFrames → packet_sent (initial / handshake) - QuicConnectionWriter.buildApplicationPacket → packet_sent (1-rtt) - QuicConnectionWriter.buildBestLevelPacket → packet_sent (close-path) - QuicConnectionParser.feedLongHeaderPacket → packet_received / packet_dropped - QuicConnectionParser.feedShortHeaderPacket → packet_received / packet_dropped - QuicConnectionParser AckFrame loss-detect → recovery:packet_lost - QuicConnectionDriver.sendLoop PTO branch → recovery:loss_timer_updated (pto) |
||
|
Claude
|
d0bc998cd2 | Merge branch 'worktree-agent-a4e96f738ceb4bbd5' into claude/research-quic-libraries-hH1Dc | ||
|
Claude
|
aff2ee182b |
fix(quic): add explicit peer-uni-stream drainer to avoid H3 multiplex tear-down
Variant (B) from the three-way fix menu in the multiplexing-interop investigation: keep `:quic` strict about per-stream backpressure (the audit-4 #3 "INTERNAL_ERROR: stream … consumer overflowed" tear-down stays the contract for app-data overflow on bidi streams) but expose an explicit, opt-in helper for peer-initiated UNI streams that the application has decided it does not need to interpret. Root cause confirmed in QuicConnectionParser.kt:290: when the server opens its three RFC 9114 §6.2.1 peer-uni streams (CONTROL + QPACK_ENCODER + QPACK_DECODER) and the H3 client does not consume them, the parser routes their bytes into each stream's bounded incomingChannel (capacity 64). Once the QPACK encoder issues dynamic-table inserts beyond 64 chunks the next chunk overflows trySend, sets QuicStream.overflowed, and the parser maps that to markClosedExternally — the entire connection dies. Notes on scope: - The `Http3GetClient` and `:quic-interop` runner mentioned in the investigation prompt do NOT exist on the `main` worktree this branch starts from. The fix here is therefore `:quic`-only: the public `awaitIncomingPeerStream` API was already sufficient for an integrator to write the accept loop themselves; this commit wraps the common case in `drainPeerInitiatedUniStreamsIntoBlackHole` and updates the doc on `awaitIncomingPeerStream` so the next integrator landing the H3 GET client doesn't hit the same trap. - Variant (C) — silent default drain in `:quic` itself — was deliberately rejected: defaults that swallow application bytes are the misconfiguration we want type-system-or-API-explicit. The new helper requires the caller to pass a CoroutineScope, so opt-in is unmistakable in any callsite. Regression test coverage in PeerUniStreamDrainTest: - pre_fix_no_consumer_overflows_and_tears_down_connection — pushes 65 chunks (capacity + 1) on a SERVER_UNI stream with no consumer; asserts the connection transitions to CLOSED. Pins the existing backpressure contract. - drainPeerInitiatedUniStreamsIntoBlackHole_keeps_connection_alive — same setup but with the new helper running on a side scope; pushes 256 chunks (4× capacity) and asserts the connection stays CONNECTED. With the helper sabotaged, this test fails at line 119 with status=CLOSED, confirming it actually exercises the fix. Full quic test suite: 295 tests, 0 failures, 0 errors. https://claude.ai/code/session_01HcvfQq1ttPV9PkRoJb4nyT |
||
|
Claude
|
04e30465d9 | Merge branch 'worktree-agent-a9d8336181fce16eb' into claude/research-quic-libraries-hH1Dc | ||
|
Claude
|
350387f7e0 |
feat(quic): handle Version Negotiation packets per RFC 9000 §6
Adds the client-side VN flow needed for the interop runner's `versionnegotiation` testcase: - `QuicConnection` accepts an `initialVersion` constructor parameter (default `QuicVersion.V1`) and exposes a mutable `currentVersion` the writer stamps into outbound long-headers. `start()` now caches the ClientHello bytes for VN-driven re-emission. - `applyVersionNegotiation(supportedVersions)` validates per §6.2 (anti-downgrade: reject if list contains the offered version), picks v1 from the offered set, regenerates DCID, re-derives Initial keys against the new DCID, resets the Initial level via `LevelState.resetForVersionNegotiation`, re-enqueues the cached ClientHello, and latches `vnConsumed` so a second VN is dropped. Failure to find a mutually supported version closes the connection with `QuicVersionNegotiationException`. - `QuicConnectionParser.feedDatagram` detects `version == 0` long headers BEFORE peekHeader (whose layout assumes v1) and dispatches to a new `feedVersionNegotiationPacket` that parses the §17.2.1 shape and validates the echoed DCID. - `QuicConnectionWriter` reads `conn.currentVersion` instead of the hardcoded `QuicVersion.V1`. - `QuicVersion.FORCE_VERSION_NEGOTIATION = 0x1a2a3a4a` for the interop runner. - `InteropRunner` honors `TESTCASE=versionnegotiation` (or `-DinteropTestcase=`) and offers the force-VN version. Regression coverage in `VersionNegotiationTest`: - happy path: VN switches `currentVersion` to v1, regenerates DCID, resets PN, and the next drain emits a v1 Initial on the wire. - downgrade defense: VN listing the offered version is dropped. - unsupported list: VN whose versions we can't speak fails the handshake and closes the connection. - second VN: post-consumption VN is ignored. - DCID mismatch: spoofed VN with wrong echoed DCID is dropped. - backward compatibility: default `initialVersion` keeps v1 behavior for existing callers. https://claude.ai/code/session_01HcvfQq1ttPV9PkRoJb4nyT |