Fix coalesced packet processing

Make sure we correctly process all coalesced packets without modifying the collection while we're iterating on it.

This issue was found by clusterfuzz:
https://bugs.chromium.org/p/chromium/issues/detail?id=990001

I've confirmed that the new test fails with the old code and passes with the fix, and that the fuzzer no longer crashes.

gfe-relnote: fix coalesced packet processing, protected by disabled v99 flag
PiperOrigin-RevId: 261433275
Change-Id: Iea1edf70fc84873fc7fe2f05c749759b2c5a6c9b

quic/core/quic_connection.cc

diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc
index 0b648e2..0c33604 100644
--- a/quic/core/quic_connection.cc
+++ b/quic/core/quic_connection.cc
@@ -2620,15 +2620,12 @@
 
 void QuicConnection::MaybeProcessCoalescedPackets() {
   bool processed = false;
-  for (const auto& packet : coalesced_packets_) {
-    if (!connected_) {
-      return;
-    }
+  while (connected_ && !coalesced_packets_.empty()) {
+    std::unique_ptr<QuicEncryptedPacket> packet =
+        std::move(coalesced_packets_.front());
+    coalesced_packets_.pop_front();
 
-    // }
-    // while (connected_ && !coalesced_packets_.empty()) {
     QUIC_DVLOG(1) << ENDPOINT << "Processing coalesced packet";
-    // QuicEncryptedPacket* packet = coalesced_packets_.front().get();
     if (framer_.ProcessPacket(*packet)) {
       processed = true;
     } else {
@@ -2644,9 +2641,7 @@
         }
       }
     }
-    // coalesced_packets_.pop_front();
   }
-  coalesced_packets_.clear();
   if (processed) {
     MaybeProcessUndecryptablePackets();
   }

quic/core/quic_connection_test.cc

diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc
index a3ab06f..773db66 100644
--- a/quic/core/quic_connection_test.cc
+++ b/quic/core/quic_connection_test.cc
@@ -8562,6 +8562,53 @@
   EXPECT_FALSE(ack_alarm->IsSet());
 }
 
+// Verify that a packet containing three coalesced packets is parsed correctly.
+TEST_P(QuicConnectionTest, CoalescedPacket) {
+  if (!QuicVersionHasLongHeaderLengths(connection_.transport_version())) {
+    // Coalesced packets can only be encoded using long header lengths.
+    return;
+  }
+  EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
+  EXPECT_TRUE(connection_.connected());
+  if (QuicVersionUsesCryptoFrames(connection_.transport_version())) {
+    EXPECT_CALL(visitor_, OnCryptoFrame(_)).Times(3);
+  } else {
+    EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(3);
+  }
+
+  uint64_t packet_numbers[3] = {1, 2, 3};
+  EncryptionLevel encryption_levels[3] = {
+      ENCRYPTION_INITIAL, ENCRYPTION_INITIAL, ENCRYPTION_FORWARD_SECURE};
+  char buffer[kMaxOutgoingPacketSize] = {};
+  size_t total_encrypted_length = 0;
+  for (int i = 0; i < 3; i++) {
+    QuicPacketHeader header =
+        ConstructPacketHeader(packet_numbers[i], encryption_levels[i]);
+    QuicFrames frames;
+    if (QuicVersionUsesCryptoFrames(connection_.transport_version())) {
+      frames.push_back(QuicFrame(&crypto_frame_));
+    } else {
+      frames.push_back(QuicFrame(frame1_));
+    }
+    std::unique_ptr<QuicPacket> packet = ConstructPacket(header, frames);
+    peer_creator_.set_encryption_level(encryption_levels[i]);
+    size_t encrypted_length = peer_framer_.EncryptPayload(
+        encryption_levels[i], QuicPacketNumber(packet_numbers[i]), *packet,
+        buffer + total_encrypted_length,
+        sizeof(buffer) - total_encrypted_length);
+    EXPECT_GT(encrypted_length, 0u);
+    total_encrypted_length += encrypted_length;
+  }
+  connection_.ProcessUdpPacket(
+      kSelfAddress, kPeerAddress,
+      QuicReceivedPacket(buffer, total_encrypted_length, clock_.Now(), false));
+  if (connection_.GetSendAlarm()->IsSet()) {
+    connection_.GetSendAlarm()->Fire();
+  }
+
+  EXPECT_TRUE(connection_.connected());
+}
+
 }  // namespace
 }  // namespace test
 }  // namespace quic