In quic, include minplaintextpacketsize when deterine whether removing soft limit for crypto frames. protected by enabled gfe2_reloadable_flag_quic_fix_min_crypto_frame_size.
PiperOrigin-RevId: 317743102
Change-Id: I43d56a2b3daea603cac4d2746f58426610875b0f
diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc
index affd89f..d0f38a5 100644
--- a/quic/core/quic_connection_test.cc
+++ b/quic/core/quic_connection_test.cc
@@ -11345,6 +11345,44 @@
}
}
+// Regresstion test for b/156232673.
+TEST_P(QuicConnectionTest, CoalescePacketOfLowerEncryptionLevel) {
+ if (!connection_.version().CanSendCoalescedPackets()) {
+ return;
+ }
+ if (GetQuicReloadableFlag(quic_fix_min_crypto_frame_size)) {
+ EXPECT_CALL(visitor_, OnHandshakePacketSent()).Times(1);
+ } else {
+ EXPECT_CALL(visitor_, OnHandshakePacketSent()).Times(0);
+ }
+ {
+ QuicConnection::ScopedPacketFlusher flusher(&connection_);
+ use_tagging_decrypter();
+ connection_.SetEncrypter(ENCRYPTION_HANDSHAKE,
+ std::make_unique<TaggingEncrypter>(0x01));
+ connection_.SetEncrypter(ENCRYPTION_FORWARD_SECURE,
+ std::make_unique<TaggingEncrypter>(0x02));
+ connection_.SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE);
+ SendStreamDataToPeer(2, string(1286, 'a'), 0, NO_FIN, nullptr);
+ connection_.SetDefaultEncryptionLevel(ENCRYPTION_HANDSHAKE);
+ // Try to coalesce a HANDSHAKE packet after 1-RTT packet.
+ if (GetQuicReloadableFlag(quic_fix_min_crypto_frame_size)) {
+ // Verify soft max packet length gets resumed and handshake packet gets
+ // successfully sent.
+ connection_.SendCryptoDataWithString("a", 0, ENCRYPTION_HANDSHAKE);
+ } else {
+ // Problematic: creator thinks there is space to consume 1-byte, however,
+ // extra paddings make the serialization fail because of
+ // MinPlaintextPacketSize.
+ EXPECT_CALL(visitor_,
+ OnConnectionClosed(_, ConnectionCloseSource::FROM_SELF));
+ EXPECT_QUIC_BUG(
+ connection_.SendCryptoDataWithString("a", 0, ENCRYPTION_HANDSHAKE),
+ "AppendPaddingFrame of 3 failed");
+ }
+ }
+}
+
} // namespace
} // namespace test
} // namespace quic
diff --git a/quic/core/quic_packet_creator.cc b/quic/core/quic_packet_creator.cc
index 82b4a27..7672a37 100644
--- a/quic/core/quic_packet_creator.cc
+++ b/quic/core/quic_packet_creator.cc
@@ -198,6 +198,8 @@
}
if (framer_->GetMaxPlaintextSize(length) <
PacketHeaderSize() + MinPlaintextPacketSize(framer_->version())) {
+ // Please note: this would not guarantee to fit next packet if the size of
+ // packet header increases (e.g., encryption level changes).
QUIC_DLOG(INFO) << length << " is too small to fit packet header";
return;
}
@@ -423,8 +425,17 @@
QuicFrame* frame) {
size_t min_frame_size =
QuicFramer::GetMinCryptoFrameSize(write_length, offset);
- if (BytesFree() <= min_frame_size &&
- (!RemoveSoftMaxPacketLength() || BytesFree() <= min_frame_size)) {
+ size_t min_plaintext_bytes = min_frame_size;
+ if (fix_min_crypto_frame_size_ && queued_frames_.empty()) {
+ // TODO(fayang): to make this more general, we need to move the checking
+ // when trying to add a frame when GetSerializedFrameLength. Remove soft
+ // limit if current packet needs extra padding and the space is too small.
+ QUIC_RELOADABLE_FLAG_COUNT(quic_fix_min_crypto_frame_size);
+ min_plaintext_bytes =
+ std::max(min_frame_size, MinPlaintextPacketSize(framer_->version()));
+ }
+ if (BytesFree() <= min_plaintext_bytes &&
+ (!RemoveSoftMaxPacketLength() || BytesFree() <= min_plaintext_bytes)) {
return false;
}
size_t max_write_length = BytesFree() - min_frame_size;
diff --git a/quic/core/quic_packet_creator.h b/quic/core/quic_packet_creator.h
index c25d8f6..743e46e 100644
--- a/quic/core/quic_packet_creator.h
+++ b/quic/core/quic_packet_creator.h
@@ -601,6 +601,9 @@
const bool avoid_leak_writer_buffer_ =
GetQuicReloadableFlag(quic_avoid_leak_writer_buffer);
+
+ const bool fix_min_crypto_frame_size_ =
+ GetQuicReloadableFlag(quic_fix_min_crypto_frame_size);
};
} // namespace quic
