Skip to content

Spinquic: MaxAckDelayMs=0 produces min_ack_delay > max_ack_delay in local transport params (assert in Debug, invalid TP in Release) #6212

Description

Bug

Setting MaxAckDelayMs = 0 crashes the connection. When it's 0, we advertise max_ack_delay = 0 in our transport parameters, but min_ack_delay is still derived from the timer resolution and stays non-zero. That gives us min_ack_delay > max_ack_delay, which isn't allowed.

  • Debug: asserts and aborts in QuicCryptoTlsEncodeTransportParameters (crypto_tls.c:868).
  • Release: no assert, so we send the bad params on the wire. The peer (including MsQuic, crypto_tls.c:1978) rejects them with a transport-parameter error and the connection fails.

The ack-frequency extension is explicit that min_ack_delay must be <= max_ack_delay, otherwise it's a TRANSPORT_PARAMETER_ERROR

Why 0 is valid

MaxAckDelayMs = 0 isn't a misuse — it means "don't delay ACKs, ack immediately":

  • We already accept 0 in settings validation (settings.c:491 only checks the upper bound).
  • The core treats 0 as "delayed ACKs off" — see ack_tracker.c:263 (and the comment at :250), and the timer is never armed for 0 (send.c:1551).

So an app can legitimately pass 0 via QUIC_SETTINGS, and it shouldn't crash or break the connection.

Repro

Set MaxAckDelayMs = 0 and start a connection (Debug build, default non-polling config).

Found by SpinQuic while adding settings-randomization coverage in #6204.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

Area: CoreRelated to the shared, core protocol logic

Type

Projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions