diff --git a/quic/src/commonMain/kotlin/com/vitorpamplona/quic/connection/QuicConnectionDriver.kt b/quic/src/commonMain/kotlin/com/vitorpamplona/quic/connection/QuicConnectionDriver.kt index 8b6d4861e..9945eb507 100644 --- a/quic/src/commonMain/kotlin/com/vitorpamplona/quic/connection/QuicConnectionDriver.kt +++ b/quic/src/commonMain/kotlin/com/vitorpamplona/quic/connection/QuicConnectionDriver.kt @@ -508,12 +508,23 @@ class QuicConnectionDriver( * (or the fresh one — both are valid) and is at worst a no-op. */ internal suspend fun handlePtoFired(conn: QuicConnection) { + // RFC 9002 §6.2.2: the consecutive-PTO count advances by exactly + // ONE per PTO timer expiry, so the §6.2.1 backoff is + // `pto_base * 2^count` per firing. Pre-2026-05-08 we incremented + // THREE times per firing — here + inside [requeueInflightForProbe] + // + again from the send loop's between-probe re-requeue (RFC + // §6.2.4 two-packet probe budget) — turning the backoff into + // `2^(3*N)` — 1×, 8×, 64× per PTO instead of 1×, 2×, 4×. + // Quic-go's `amplificationlimit` testcase exposed this: after the + // sim drops client packets 2-7, we couldn't get a probe to the + // server within its 5 s amp-limited idle timeout because PTO #3 + // didn't fire until ~11 s post-handshake. + // // Increment FIRST so [requeueInflightForProbe]'s threshold check - // sees the post-increment value. The increment must happen exactly - // once per PTO event — not per call to [requeueInflightForProbe], - // because the send loop calls that helper again between the first - // and second probe (RFC 9002 §6.2.4) and that re-requeue is part - // of the SAME PTO event. + // (RFC 9000 §9 — trigger migration after PATH_PROBE_PTO_THRESHOLD + // consecutive PTOs) sees the post-increment value, matching the + // constant's natural reading: "after N consecutive PTOs with no + // progress, trigger migration on the Nth PTO firing". conn.consecutivePtoCount = (conn.consecutivePtoCount + 1).coerceAtMost(6) conn.pendingPing = true requeueInflightForProbe(conn) @@ -550,13 +561,15 @@ internal suspend fun requeueInflightForProbe(conn: QuicConnection) { if (conn.initial.sendProtection != null && !conn.initial.keysDiscarded) { conn.requeueAllInflightCrypto(EncryptionLevel.INITIAL) } - // The PTO count is incremented in [handlePtoFired] BEFORE this - // helper runs, so the threshold check below sees the post-increment - // value (matching the constant's natural reading: "after N - // consecutive PTOs with no progress, trigger migration on the Nth - // PTO firing"). The send loop's between-probe re-requeue calls - // this helper without bumping the count again — that's the same - // PTO event, not a new one. + // [conn.consecutivePtoCount] is incremented exactly once per PTO + // firing inside [handlePtoFired]; the path-validation threshold + // check below reads the already-incremented value. Pre-2026-05-08 + // this function ALSO incremented the count, and the send loop's + // between-probe re-requeue called this function again — turning + // one PTO into +3 to the count. The amplificationlimit interop + // testcase against quic-go flushed this: PTO #3 didn't fire until + // ~11 s post-handshake, well past the 5 s amp-limited idle + // timeout. See [handlePtoFired] kdoc for the full mechanism. // Once 1-RTT keys are installed, PTO must also retransmit application // data — STREAM bytes that were sent but never ACK'd. Without this, // a single corrupted/lost 1-RTT packet (especially the first one diff --git a/quic/src/commonTest/kotlin/com/vitorpamplona/quic/connection/PtoCryptoRetransmitTest.kt b/quic/src/commonTest/kotlin/com/vitorpamplona/quic/connection/PtoCryptoRetransmitTest.kt index 725f680c5..830b5697d 100644 --- a/quic/src/commonTest/kotlin/com/vitorpamplona/quic/connection/PtoCryptoRetransmitTest.kt +++ b/quic/src/commonTest/kotlin/com/vitorpamplona/quic/connection/PtoCryptoRetransmitTest.kt @@ -281,6 +281,67 @@ class PtoCryptoRetransmitTest { ) } + @Test + fun consecutivePtoCountAdvancesByOnePerPtoFiringNotPerProbePacket() = + runBlocking { + // RFC 9002 §6.2.2: the consecutive-PTO count advances by + // exactly ONE per PTO timer expiry (so the §6.2.1 backoff + // is `pto_base * 2^count` per firing). Pre-2026-05-08 the + // count incremented THREE times per firing — once in + // `handlePtoFired`, once in `requeueInflightForProbe`, + // and again from the send loop's between-probe re-requeue + // — making the effective backoff `2^(3*N)` (1×, 8×, 64× + // per PTO). Quic-go's `amplificationlimit` testcase + // exposed this: PTO #3 didn't fire until ~11 s + // post-handshake, well past quic-go's 5 s amp-limited + // idle timeout. See the explicit kdoc in + // [com.vitorpamplona.quic.connection.handlePtoFired]. + // + // This test drives the EXACT helpers the production send + // loop calls (handlePtoFired + the between-probe + // requeueInflightForProbe) and asserts the count grows + // by 1 per PTO firing, not per probe packet. + val client = newClientWithStartedHandshake() + drainOutbound(client, nowMillis = 1L) + assertEquals(0, client.consecutivePtoCount, "fresh client starts at 0") + + // PTO #1 — full simulation including budget consumption. + handlePtoFired(client) + assertEquals( + 1, + client.consecutivePtoCount, + "handlePtoFired alone must take count from 0 to 1", + ) + // First probe drained. + drainOutbound(client, nowMillis = 2L) + client.pendingProbePackets-- + // Send loop's between-probe re-requeue. Pre-fix this + // bumped the count to 2 inside the same PTO firing. + requeueInflightForProbe(client) + assertEquals( + 1, + client.consecutivePtoCount, + "between-probe re-requeue must NOT bump the count — same PTO firing", + ) + // Second probe drained. + drainOutbound(client, nowMillis = 3L) + client.pendingProbePackets-- + + // PTO #2 — count must advance to 2, not 4. + handlePtoFired(client) + assertEquals(2, client.consecutivePtoCount, "PTO #2 takes count from 1 to 2") + drainOutbound(client, nowMillis = 4L) + client.pendingProbePackets-- + requeueInflightForProbe(client) + assertEquals(2, client.consecutivePtoCount) + drainOutbound(client, nowMillis = 5L) + client.pendingProbePackets-- + + // PTO #3 — count → 3, not 6 (capped). + handlePtoFired(client) + assertEquals(3, client.consecutivePtoCount, "PTO #3 takes count from 2 to 3") + } + /** * Decode an Initial-level long-header packet from a freshly-drained * datagram and return its frames. We open with the client's own