Account for worst-case header + tag size when deciding to coalesce

A header and tag could easily be larger than 40 bytes.

quinn-proto/src/connection/mod.rs

@@ -33,7 +33,7 @@ use crate::{
     token::ResetToken,
     transport_parameters::TransportParameters,
     Dir, EndpointConfig, Frame, Side, StreamId, Transmit, TransportError, TransportErrorCode,
-    VarInt, MAX_STREAM_COUNT, MIN_INITIAL_SIZE, TIMER_GRANULARITY,
+    VarInt, MAX_CID_SIZE, MAX_STREAM_COUNT, MIN_INITIAL_SIZE, TIMER_GRANULARITY,
 };
 
 mod ack_frequency;
@@ -608,7 +608,16 @@ impl Connection {
                 buf.len()
             };
 
-            if !coalesce || buf_capacity - buf_end < MIN_PACKET_SPACE {
+            let tag_len = if let Some(ref crypto) = self.spaces[space_id].crypto {
+                crypto.packet.local.tag_len()
+            } else if space_id == SpaceId::Data {
+                self.zero_rtt_crypto.as_ref().expect(
+                    "sending packets in the application data space requires known 0-RTT or 1-RTT keys",
+                ).packet.tag_len()
+            } else {
+                unreachable!("tried to send {:?} packet without keys", space_id)
+            };
+            if !coalesce || buf_capacity - buf_end < MIN_PACKET_SPACE + tag_len {
                 // We need to send 1 more datagram and extend the buffer for that.
 
                 // Is 1 more datagram allowed?
@@ -3739,8 +3748,24 @@ fn get_max_ack_delay(params: &TransportParameters) -> Duration {
 
 // Prevents overflow and improves behavior in extreme circumstances
 const MAX_BACKOFF_EXPONENT: u32 = 16;
-// Minimal remaining size to allow packet coalescing
-const MIN_PACKET_SPACE: usize = 40;
+
+/// Minimal remaining size to allow packet coalescing, excluding cryptographic tag
+///
+/// This must be at least as large as the header for a well-formed empty packet to be coalesced,
+/// plus some space for frames. We only care about handshake headers because short header packets
+/// necessarily have smaller headers, and initial packets are only ever the first packet in a
+/// datagram (because we coalesce in ascending packet space order and the only reason to split a
+/// packet is when packet space changes).
+const MIN_PACKET_SPACE: usize = MAX_HANDSHAKE_OR_0RTT_HEADER_SIZE + 32;
+
+/// Largest amount of space that could be occupied by a Handshake or 0-RTT packet's header
+///
+/// Excludes packet-type-specific fields such as packet number or Initial token
+// https://www.rfc-editor.org/rfc/rfc9000.html#name-0-rtt: flags + version + dcid len + dcid +
+// scid len + scid + length + pn
+const MAX_HANDSHAKE_OR_0RTT_HEADER_SIZE: usize =
+    1 + 4 + 1 + MAX_CID_SIZE + 1 + MAX_CID_SIZE + VarInt::from_u32(u16::MAX as u32).size() + 4;
+
 /// The maximum amount of datagrams that are sent in a single transmit
 ///
 /// This can be lower than the maximum platform capabilities, to avoid excessive

quinn-proto/src/connection/packet_builder.rs

@@ -138,12 +138,10 @@ impl PacketBuilder {
                 crypto.packet.local.tag_len(),
             )
         } else if space_id == SpaceId::Data {
-            let zero_rtt = conn.zero_rtt_crypto.as_ref().expect(
-                "sending packets in the application data space requires known 0-RTT or 1-RTT keys",
-            );
+            let zero_rtt = conn.zero_rtt_crypto.as_ref().unwrap();
             (zero_rtt.header.sample_size(), zero_rtt.packet.tag_len())
         } else {
-            unreachable!("tried to send {:?} packet without keys", space_id);
+            unreachable!();
         };
 
         // Each packet must be large enough for header protection sampling, i.e. the combined
@@ -159,6 +157,7 @@ impl PacketBuilder {
             partial_encode.start + dst_cid.len() + 6,
         );
         let max_size = buffer_capacity - tag_len;
+        debug_assert!(max_size >= min_size);
 
         Some(Self {
             datagram_start,