Close QUIC connection if it fails to serialize a coalesced packet.
Protected by FLAGS_quic_reloadable_flag_quic_close_connection_if_fail_to_serialzie_coalesced_packet.
PiperOrigin-RevId: 434638558
diff --git a/quic/core/quic_coalesced_packet.cc b/quic/core/quic_coalesced_packet.cc
index e8ce931..dcfcb09 100644
--- a/quic/core/quic_coalesced_packet.cc
+++ b/quic/core/quic_coalesced_packet.cc
@@ -181,4 +181,17 @@
return info;
}
+std::vector<size_t> QuicCoalescedPacket::packet_lengths() const {
+ std::vector<size_t> lengths;
+ for (const auto& packet : encrypted_buffers_) {
+ if (lengths.empty()) {
+ lengths.push_back(
+ initial_packet_ == nullptr ? 0 : initial_packet_->encrypted_length);
+ } else {
+ lengths.push_back(packet.length());
+ }
+ }
+ return lengths;
+}
+
} // namespace quic
diff --git a/quic/core/quic_coalesced_packet.h b/quic/core/quic_coalesced_packet.h
index 9b8295c..2e4165d 100644
--- a/quic/core/quic_coalesced_packet.h
+++ b/quic/core/quic_coalesced_packet.h
@@ -9,6 +9,10 @@
namespace quic {
+namespace test {
+class QuicCoalescedPacketPeer;
+}
+
// QuicCoalescedPacket is used to buffer multiple packets which can be coalesced
// into the same UDP datagram.
class QUIC_EXPORT_PRIVATE QuicCoalescedPacket {
@@ -61,7 +65,11 @@
QuicPacketLength max_packet_length() const { return max_packet_length_; }
+ std::vector<size_t> packet_lengths() const;
+
private:
+ friend class test::QuicCoalescedPacketPeer;
+
// self/peer addresses are set when trying to coalesce the first packet.
// Packets with different self/peer addresses cannot be coalesced.
QuicSocketAddress self_address_;
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc
index 4715925..0ceafd9 100644
--- a/quic/core/quic_connection.cc
+++ b/quic/core/quic_connection.cc
@@ -3391,6 +3391,10 @@
packet_creator_.max_packet_length())) {
// Failed to coalesce packet, flush current coalesced packet.
if (!FlushCoalescedPacket()) {
+ QUIC_BUG_IF(quic_connection_connected_after_flush_coalesced_failure,
+ connected_)
+ << "QUIC connection is still connected after failing to flush "
+ "coalesced packet.";
// Failed to flush coalesced packet, write error has been handled.
return false;
}
@@ -5905,6 +5909,15 @@
const size_t length = packet_creator_.SerializeCoalescedPacket(
coalesced_packet_, buffer, coalesced_packet_.max_packet_length());
if (length == 0) {
+ if (GetQuicReloadableFlag(
+ quic_close_connection_if_fail_to_serialzie_coalesced_packet) &&
+ connected_) {
+ QUIC_RELOADABLE_FLAG_COUNT(
+ quic_close_connection_if_fail_to_serialzie_coalesced_packet);
+ CloseConnection(QUIC_FAILED_TO_SERIALIZE_PACKET,
+ "Failed to serialize coalesced packet.",
+ ConnectionCloseBehavior::SILENT_CLOSE);
+ }
return false;
}
if (debug_visitor_ != nullptr) {
diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc
index db7ae69..d0ee223 100644
--- a/quic/core/quic_connection_test.cc
+++ b/quic/core/quic_connection_test.cc
@@ -13,6 +13,7 @@
#include "absl/base/macros.h"
#include "absl/strings/str_cat.h"
+#include "absl/strings/str_join.h"
#include "absl/strings/string_view.h"
#include "quic/core/congestion_control/loss_detection_interface.h"
#include "quic/core/congestion_control/send_algorithm_interface.h"
@@ -42,6 +43,7 @@
#include "quic/platform/api/quic_test.h"
#include "quic/test_tools/mock_clock.h"
#include "quic/test_tools/mock_random.h"
+#include "quic/test_tools/quic_coalesced_packet_peer.h"
#include "quic/test_tools/quic_config_peer.h"
#include "quic/test_tools/quic_connection_peer.h"
#include "quic/test_tools/quic_framer_peer.h"
@@ -10266,6 +10268,81 @@
EXPECT_NE(nullptr, writer_->coalesced_packet());
}
+TEST_P(QuicConnectionTest, FailToCoalescePacket) {
+ // EXPECT_QUIC_BUG tests are expensive so only run one instance of them.
+ if (!IsDefaultTestConfiguration() ||
+ !connection_.version().CanSendCoalescedPackets()) {
+ return;
+ }
+
+ set_perspective(Perspective::IS_SERVER);
+ use_tagging_decrypter();
+
+ EXPECT_CALL(visitor_, OnHandshakePacketSent());
+
+ if (GetQuicReloadableFlag(
+ quic_close_connection_if_fail_to_serialzie_coalesced_packet)) {
+ EXPECT_CALL(visitor_,
+ OnConnectionClosed(_, ConnectionCloseSource::FROM_SELF))
+ .WillOnce(Invoke(this, &QuicConnectionTest::SaveConnectionCloseFrame));
+ }
+
+ ProcessDataPacketAtLevel(1, !kHasStopWaiting, ENCRYPTION_INITIAL);
+ auto test_body = [&] {
+ QuicConnection::ScopedPacketFlusher flusher(&connection_);
+ connection_.SetEncrypter(ENCRYPTION_INITIAL,
+ std::make_unique<TaggingEncrypter>(0x01));
+ connection_.SetDefaultEncryptionLevel(ENCRYPTION_INITIAL);
+ connection_.SendCryptoDataWithString("foo", 0);
+ // Verify this packet is on hold.
+ EXPECT_EQ(0u, writer_->packets_write_attempts());
+
+ connection_.SetEncrypter(ENCRYPTION_HANDSHAKE,
+ std::make_unique<TaggingEncrypter>(0x02));
+ connection_.SetDefaultEncryptionLevel(ENCRYPTION_HANDSHAKE);
+ connection_.SendCryptoDataWithString("bar", 3);
+ EXPECT_EQ(0u, writer_->packets_write_attempts());
+
+ connection_.SetEncrypter(ENCRYPTION_FORWARD_SECURE,
+ std::make_unique<TaggingEncrypter>(0x03));
+ connection_.SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE);
+ SendStreamDataToPeer(2, "baz", 3, NO_FIN, nullptr);
+
+ creator_->Flush();
+
+ auto& coalesced_packet =
+ QuicConnectionPeer::GetCoalescedPacket(&connection_);
+ QuicPacketLength coalesced_packet_max_length =
+ coalesced_packet.max_packet_length();
+ QuicCoalescedPacketPeer::SetMaxPacketLength(coalesced_packet,
+ coalesced_packet.length());
+
+ // Make the coalescer's FORWARD_SECURE packet longer.
+ *QuicCoalescedPacketPeer::GetMutableEncryptedBuffer(
+ coalesced_packet, ENCRYPTION_FORWARD_SECURE) += "!!! TEST !!!";
+
+ QUIC_LOG(INFO) << "Reduced coalesced_packet_max_length from "
+ << coalesced_packet_max_length << " to "
+ << coalesced_packet.max_packet_length()
+ << ", coalesced_packet.length:" << coalesced_packet.length()
+ << ", coalesced_packet.packet_lengths:"
+ << absl::StrJoin(coalesced_packet.packet_lengths(), ":");
+ };
+
+ EXPECT_QUIC_BUG(test_body(), "SerializeCoalescedPacket failed.");
+
+ if (GetQuicReloadableFlag(
+ quic_close_connection_if_fail_to_serialzie_coalesced_packet)) {
+ EXPECT_FALSE(connection_.connected());
+ EXPECT_THAT(saved_connection_close_frame_.quic_error_code,
+ IsError(QUIC_FAILED_TO_SERIALIZE_PACKET));
+ EXPECT_EQ(saved_connection_close_frame_.error_details,
+ "Failed to serialize coalesced packet.");
+ } else {
+ EXPECT_TRUE(connection_.connected());
+ }
+}
+
TEST_P(QuicConnectionTest, LegacyVersionEncapsulation) {
connection_.EnableLegacyVersionEncapsulation("test.example.org");
diff --git a/quic/core/quic_crypto_stream_test.cc b/quic/core/quic_crypto_stream_test.cc
index fffcb09..3760c7d 100644
--- a/quic/core/quic_crypto_stream_test.cc
+++ b/quic/core/quic_crypto_stream_test.cc
@@ -107,6 +107,9 @@
&alarm_factory_,
Perspective::IS_CLIENT)),
session_(connection_, /*create_mock_crypto_stream=*/false) {
+ EXPECT_CALL(*static_cast<MockPacketWriter*>(connection_->writer()),
+ WritePacket(_, _, _, _, _))
+ .WillRepeatedly(Return(WriteResult(WRITE_STATUS_OK, 0)));
stream_ = new MockQuicCryptoStream(&session_);
session_.SetCryptoStream(stream_);
session_.Initialize();
diff --git a/quic/core/quic_flags_list.h b/quic/core/quic_flags_list.h
index 33f9bc8..0fe1980 100644
--- a/quic/core/quic_flags_list.h
+++ b/quic/core/quic_flags_list.h
@@ -23,6 +23,8 @@
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_add_cached_network_parameters_to_address_token2, true)
// If true, 1) QUIC connections will use a lower minimum for trusted initial rtt, 2) When TRTT is received, QUIC server sessions will mark the initial rtt from CachedNetworkParameters as trusted.
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_use_lower_min_for_trusted_irtt, false)
+// If true, QUIC connection will be closed if it fails to serialize a coalesced packet.
+QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_close_connection_if_fail_to_serialzie_coalesced_packet, true)
// If true, QUIC will default enable MTU discovery at server, with a target of 1450 bytes.
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_enable_mtu_discovery_at_server, false)
// If true, QuicGsoBatchWriter will support release time if it is available and the process has the permission to do so.
diff --git a/quic/core/quic_packet_creator.cc b/quic/core/quic_packet_creator.cc
index 4065e7e..95149b1 100644
--- a/quic/core/quic_packet_creator.cc
+++ b/quic/core/quic_packet_creator.cc
@@ -14,6 +14,7 @@
#include "absl/base/macros.h"
#include "absl/base/optimization.h"
#include "absl/strings/str_cat.h"
+#include "absl/strings/str_join.h"
#include "absl/strings/string_view.h"
#include "absl/types/optional.h"
#include "quic/core/crypto/crypto_protocol.h"
@@ -1122,6 +1123,7 @@
QUIC_BUG_IF(quic_bug_12398_15, coalesced.length() == 0)
<< ENDPOINT << "Attempt to serialize empty coalesced packet";
size_t packet_length = 0;
+ size_t initial_length = 0;
if (coalesced.initial_packet() != nullptr) {
// Padding coalesced packet containing initial packet to full.
size_t padding_size = coalesced.max_packet_length() - coalesced.length();
@@ -1132,7 +1134,7 @@
// Do not pad server initial connection close packet.
padding_size = 0;
}
- size_t initial_length = ReserializeInitialPacketInCoalescedPacket(
+ initial_length = ReserializeInitialPacketInCoalescedPacket(
*coalesced.initial_packet(), padding_size, buffer, buffer_len);
if (initial_length == 0) {
QUIC_BUG(quic_bug_10752_19)
@@ -1147,6 +1149,14 @@
}
size_t length_copied = 0;
if (!coalesced.CopyEncryptedBuffers(buffer, buffer_len, &length_copied)) {
+ QUIC_BUG(quic_serialize_coalesced_packet_copy_failure)
+ << "SerializeCoalescedPacket failed. buffer_len:" << buffer_len
+ << ", initial_length:" << initial_length
+ << ", length_copied:" << length_copied
+ << ", coalesced.length:" << coalesced.length()
+ << ", coalesced.max_packet_length:" << coalesced.max_packet_length()
+ << ", coalesced.packet_lengths:"
+ << absl::StrJoin(coalesced.packet_lengths(), ":");
return 0;
}
packet_length += length_copied;
diff --git a/quic/test_tools/quic_coalesced_packet_peer.cc b/quic/test_tools/quic_coalesced_packet_peer.cc
new file mode 100644
index 0000000..efd1900
--- /dev/null
+++ b/quic/test_tools/quic_coalesced_packet_peer.cc
@@ -0,0 +1,23 @@
+// Copyright (c) 2022 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "quic/test_tools/quic_coalesced_packet_peer.h"
+
+namespace quic {
+namespace test {
+
+// static
+void QuicCoalescedPacketPeer::SetMaxPacketLength(
+ QuicCoalescedPacket& coalesced_packet, QuicPacketLength length) {
+ coalesced_packet.max_packet_length_ = length;
+}
+
+// static
+std::string* QuicCoalescedPacketPeer::GetMutableEncryptedBuffer(
+ QuicCoalescedPacket& coalesced_packet, EncryptionLevel encryption_level) {
+ return &coalesced_packet.encrypted_buffers_[encryption_level];
+}
+
+} // namespace test
+} // namespace quic
diff --git a/quic/test_tools/quic_coalesced_packet_peer.h b/quic/test_tools/quic_coalesced_packet_peer.h
new file mode 100644
index 0000000..908cf09
--- /dev/null
+++ b/quic/test_tools/quic_coalesced_packet_peer.h
@@ -0,0 +1,26 @@
+// Copyright (c) 2022 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef QUICHE_QUIC_TEST_TOOLS_QUIC_COALESCED_PACKET_PEER_H_
+#define QUICHE_QUIC_TEST_TOOLS_QUIC_COALESCED_PACKET_PEER_H_
+
+#include "quic/core/quic_coalesced_packet.h"
+#include "quic/core/quic_types.h"
+
+namespace quic {
+namespace test {
+
+class QuicCoalescedPacketPeer {
+ public:
+ static void SetMaxPacketLength(QuicCoalescedPacket& coalesced_packet,
+ QuicPacketLength length);
+
+ static std::string* GetMutableEncryptedBuffer(
+ QuicCoalescedPacket& coalesced_packet, EncryptionLevel encryption_level);
+};
+
+} // namespace test
+} // namespace quic
+
+#endif // QUICHE_QUIC_TEST_TOOLS_QUIC_COALESCED_PACKET_PEER_H_
diff --git a/quic/test_tools/quic_connection_peer.cc b/quic/test_tools/quic_connection_peer.cc
index 207582f..5e60a84 100644
--- a/quic/test_tools/quic_connection_peer.cc
+++ b/quic/test_tools/quic_connection_peer.cc
@@ -535,5 +535,16 @@
connection->last_decrypted_packet_level_ = level;
}
+// static
+QuicCoalescedPacket& QuicConnectionPeer::GetCoalescedPacket(
+ QuicConnection* connection) {
+ return connection->coalesced_packet_;
+}
+
+// static
+void QuicConnectionPeer::FlushCoalescedPacket(QuicConnection* connection) {
+ connection->FlushCoalescedPacket();
+}
+
} // namespace test
} // namespace quic
diff --git a/quic/test_tools/quic_connection_peer.h b/quic/test_tools/quic_connection_peer.h
index 62ed60e..ff536c2 100644
--- a/quic/test_tools/quic_connection_peer.h
+++ b/quic/test_tools/quic_connection_peer.h
@@ -220,6 +220,10 @@
static void SetLastDecryptedLevel(QuicConnection* connection,
EncryptionLevel level);
+
+ static QuicCoalescedPacket& GetCoalescedPacket(QuicConnection* connection);
+
+ static void FlushCoalescedPacket(QuicConnection* connection);
};
} // namespace test
