gfe-relnote: For server side QUIC connection, mark version_negotiated_ earlier. Protected by --gfe2_reloadable_flag_quic_version_negotiated_by_default_at_server.
PiperOrigin-RevId: 282365816
Change-Id: I9083e05d758f564befaab126e427f7ab434d2f6f
diff --git a/quic/core/http/quic_server_session_base_test.cc b/quic/core/http/quic_server_session_base_test.cc
index cd45c50..9e8c749 100644
--- a/quic/core/http/quic_server_session_base_test.cc
+++ b/quic/core/http/quic_server_session_base_test.cc
@@ -152,8 +152,10 @@
QuicCryptoServerConfig::ConfigOptions());
SetQuicReloadableFlag(quic_supports_tls_handshake, true);
session_->Initialize();
- QuicSessionPeer::GetMutableCryptoStream(session_.get())
- ->OnSuccessfulVersionNegotiation(supported_versions.front());
+ if (!GetQuicReloadableFlag(quic_version_negotiated_by_default_at_server)) {
+ QuicSessionPeer::GetMutableCryptoStream(session_.get())
+ ->OnSuccessfulVersionNegotiation(supported_versions.front());
+ }
QuicConfigPeer::SetReceivedInitialSessionFlowControlWindow(
session_->config(), kMinimumFlowControlSendWindow);
session_->OnConfigNegotiated();
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc
index c457ca4..a4efa01 100644
--- a/quic/core/quic_connection.cc
+++ b/quic/core/quic_connection.cc
@@ -336,7 +336,9 @@
treat_queued_packets_as_sent_(
GetQuicReloadableFlag(quic_treat_queued_packets_as_sent) ||
version().CanSendCoalescedPackets()),
- mtu_discovery_v2_(GetQuicReloadableFlag(quic_mtu_discovery_v2)) {
+ mtu_discovery_v2_(GetQuicReloadableFlag(quic_mtu_discovery_v2)),
+ quic_version_negotiated_by_default_at_server_(
+ GetQuicReloadableFlag(quic_version_negotiated_by_default_at_server)) {
QUIC_DLOG(INFO) << ENDPOINT << "Created connection with server connection ID "
<< server_connection_id
<< " and version: " << ParsedQuicVersionToString(version());
@@ -370,6 +372,13 @@
DCHECK(perspective_ == Perspective::IS_CLIENT ||
supported_versions.size() == 1);
InstallInitialCrypters(server_connection_id_);
+
+ if (quic_version_negotiated_by_default_at_server() &&
+ perspective_ == Perspective::IS_SERVER) {
+ QUIC_RELOADABLE_FLAG_COUNT(quic_version_negotiated_by_default_at_server);
+ version_negotiated_ = true;
+ framer_.InferPacketHeaderTypeFromVersion();
+ }
}
void QuicConnection::InstallInitialCrypters(QuicConnectionId connection_id) {
@@ -759,32 +768,41 @@
return false;
}
- if (!version_negotiated_ && perspective_ == Perspective::IS_SERVER) {
- if (!header.version_flag) {
- // Packets should have the version flag till version negotiation is
- // done.
- std::string error_details =
- QuicStrCat(ENDPOINT, "Packet ", header.packet_number.ToUint64(),
- " without version flag before version negotiated.");
- QUIC_DLOG(WARNING) << error_details;
- CloseConnection(QUIC_INVALID_VERSION, error_details,
- ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET);
- return false;
- } else {
- DCHECK_EQ(header.version, version());
- version_negotiated_ = true;
- framer_.InferPacketHeaderTypeFromVersion();
- visitor_->OnSuccessfulVersionNegotiation(version());
- if (debug_visitor_ != nullptr) {
- debug_visitor_->OnSuccessfulVersionNegotiation(version());
+ if (!quic_version_negotiated_by_default_at_server()) {
+ if (!version_negotiated_ && perspective_ == Perspective::IS_SERVER) {
+ if (!header.version_flag) {
+ // Packets should have the version flag till version negotiation is
+ // done.
+ std::string error_details =
+ QuicStrCat(ENDPOINT, "Packet ", header.packet_number.ToUint64(),
+ " without version flag before version negotiated.");
+ QUIC_DLOG(WARNING) << error_details;
+ CloseConnection(QUIC_INVALID_VERSION, error_details,
+ ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET);
+ return false;
+ } else {
+ DCHECK_EQ(header.version, version());
+ version_negotiated_ = true;
+ framer_.InferPacketHeaderTypeFromVersion();
+ visitor_->OnSuccessfulVersionNegotiation(version());
+ if (debug_visitor_ != nullptr) {
+ debug_visitor_->OnSuccessfulVersionNegotiation(version());
+ }
}
+ DCHECK(version_negotiated_);
}
- DCHECK(version_negotiated_);
}
return true;
}
+void QuicConnection::OnSuccessfulVersionNegotiation() {
+ visitor_->OnSuccessfulVersionNegotiation(version());
+ if (debug_visitor_ != nullptr) {
+ debug_visitor_->OnSuccessfulVersionNegotiation(version());
+ }
+}
+
void QuicConnection::OnDecryptedPacket(EncryptionLevel level) {
last_decrypted_packet_level_ = level;
last_packet_decrypted_ = true;
@@ -1959,10 +1977,7 @@
packet_creator_.StopSendingVersion();
}
version_negotiated_ = true;
- visitor_->OnSuccessfulVersionNegotiation(version());
- if (debug_visitor_ != nullptr) {
- debug_visitor_->OnSuccessfulVersionNegotiation(version());
- }
+ OnSuccessfulVersionNegotiation();
}
}
diff --git a/quic/core/quic_connection.h b/quic/core/quic_connection.h
index de26b3c..55173cc 100644
--- a/quic/core/quic_connection.h
+++ b/quic/core/quic_connection.h
@@ -895,6 +895,13 @@
return treat_queued_packets_as_sent_;
}
+ // Called when version is considered negotiated.
+ void OnSuccessfulVersionNegotiation();
+
+ bool quic_version_negotiated_by_default_at_server() const {
+ return quic_version_negotiated_by_default_at_server_;
+ }
+
protected:
// Calls cancel() on all the alarms owned by this connection.
void CancelAllAlarms();
@@ -1373,6 +1380,7 @@
QuicSentPacketManager sent_packet_manager_;
// Indicates whether connection version has been negotiated.
+ // Always true for server connections.
bool version_negotiated_;
// Tracks if the connection was created by the server or the client.
@@ -1524,6 +1532,9 @@
const bool mtu_discovery_v2_;
// Only used if quic_mtu_discovery_v2 is true.
QuicConnectionMtuDiscoverer mtu_discoverer_;
+
+ // Latched value of quic_version_negotiated_by_default_at_server.
+ const bool quic_version_negotiated_by_default_at_server_;
};
} // namespace quic
diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc
index 1fbd121..111f94f 100644
--- a/quic/core/quic_connection_test.cc
+++ b/quic/core/quic_connection_test.cc
@@ -1047,6 +1047,7 @@
EXPECT_CALL(visitor_, OnCongestionWindowChange(_)).Times(AnyNumber());
EXPECT_CALL(visitor_, OnPacketReceived(_, _, _)).Times(AnyNumber());
EXPECT_CALL(visitor_, OnForwardProgressConfirmed()).Times(AnyNumber());
+ EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_)).Times(AnyNumber());
EXPECT_CALL(*loss_algorithm_, GetLossTimeout())
.WillRepeatedly(Return(QuicTime::Zero()));
@@ -1551,6 +1552,10 @@
connection_.set_perspective(perspective);
if (perspective == Perspective::IS_SERVER) {
connection_.set_can_truncate_connection_ids(true);
+ QuicConnectionPeer::SetNegotiatedVersion(&connection_);
+ if (GetQuicReloadableFlag(quic_version_negotiated_by_default_at_server)) {
+ connection_.OnSuccessfulVersionNegotiation();
+ }
}
QuicFramerPeer::SetPerspective(&peer_framer_,
QuicUtils::InvertPerspective(perspective));
@@ -1613,7 +1618,6 @@
void MtuDiscoveryTestInit() {
set_perspective(Perspective::IS_SERVER);
- QuicConnectionPeer::SetNegotiatedVersion(&connection_);
QuicPacketCreatorPeer::SetSendVersionInPacket(creator_, false);
connection_.SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE);
peer_creator_.set_encryption_level(ENCRYPTION_FORWARD_SECURE);
@@ -1729,8 +1733,6 @@
}
TEST_P(QuicConnectionTest, SelfAddressChangeAtServer) {
- EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
-
set_perspective(Perspective::IS_SERVER);
QuicPacketCreatorPeer::SetSendVersionInPacket(creator_, false);
@@ -1760,8 +1762,6 @@
}
TEST_P(QuicConnectionTest, AllowSelfAddressChangeToMappedIpv4AddressAtServer) {
- EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
-
set_perspective(Perspective::IS_SERVER);
QuicPacketCreatorPeer::SetSendVersionInPacket(creator_, false);
@@ -1795,7 +1795,6 @@
}
TEST_P(QuicConnectionTest, ClientAddressChangeAndPacketReordered) {
- EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
set_perspective(Perspective::IS_SERVER);
QuicPacketCreatorPeer::SetSendVersionInPacket(creator_, false);
@@ -1834,7 +1833,6 @@
}
TEST_P(QuicConnectionTest, PeerAddressChangeAtServer) {
- EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
set_perspective(Perspective::IS_SERVER);
QuicPacketCreatorPeer::SetSendVersionInPacket(creator_, false);
EXPECT_EQ(Perspective::IS_SERVER, connection_.perspective());
@@ -1872,7 +1870,6 @@
}
TEST_P(QuicConnectionTest, EffectivePeerAddressChangeAtServer) {
- EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
set_perspective(Perspective::IS_SERVER);
QuicPacketCreatorPeer::SetSendVersionInPacket(creator_, false);
EXPECT_EQ(Perspective::IS_SERVER, connection_.perspective());
@@ -1958,7 +1955,6 @@
}
TEST_P(QuicConnectionTest, ReceivePaddedPingAtServer) {
- EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
set_perspective(Perspective::IS_SERVER);
QuicPacketCreatorPeer::SetSendVersionInPacket(creator_, false);
EXPECT_EQ(Perspective::IS_SERVER, connection_.perspective());
@@ -2083,7 +2079,6 @@
}
TEST_P(QuicConnectionTest, ReceiveConnectivityProbingAtServer) {
- EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
set_perspective(Perspective::IS_SERVER);
QuicPacketCreatorPeer::SetSendVersionInPacket(creator_, false);
EXPECT_EQ(Perspective::IS_SERVER, connection_.perspective());
@@ -2142,7 +2137,6 @@
}
TEST_P(QuicConnectionTest, ReceiveReorderedConnectivityProbingAtServer) {
- EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
set_perspective(Perspective::IS_SERVER);
QuicPacketCreatorPeer::SetSendVersionInPacket(creator_, false);
EXPECT_EQ(Perspective::IS_SERVER, connection_.perspective());
@@ -2199,7 +2193,6 @@
}
TEST_P(QuicConnectionTest, MigrateAfterProbingAtServer) {
- EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
set_perspective(Perspective::IS_SERVER);
QuicPacketCreatorPeer::SetSendVersionInPacket(creator_, false);
EXPECT_EQ(Perspective::IS_SERVER, connection_.perspective());
@@ -2400,8 +2393,6 @@
}
TEST_P(QuicConnectionTest, IncreaseServerMaxPacketSize) {
- EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
-
set_perspective(Perspective::IS_SERVER);
connection_.SetMaxPacketLength(1000);
@@ -2446,8 +2437,6 @@
}
TEST_P(QuicConnectionTest, IncreaseServerMaxPacketSizeWhileWriterLimited) {
- EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
-
const QuicByteCount lower_max_packet_size = 1240;
writer_->set_max_packet_size(lower_max_packet_size);
set_perspective(Perspective::IS_SERVER);
@@ -7870,7 +7859,6 @@
// Ack data.
clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(5));
- EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
EXPECT_CALL(*send_algorithm_, OnCongestionEvent(true, _, _, _, _));
QuicAckFrame frame =
InitAckFrame({{QuicPacketNumber(1u), QuicPacketNumber(2u)}});
@@ -7905,8 +7893,6 @@
}
TEST_P(QuicConnectionTest, ServerReceivesChloOnNonCryptoStream) {
- EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
-
set_perspective(Perspective::IS_SERVER);
QuicPacketCreatorPeer::SetSendVersionInPacket(creator_, false);
@@ -8855,7 +8841,6 @@
0u, QuicStringPiece()));
EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(1);
}
- EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
ProcessFramePacketWithAddresses(frame, kSelfAddress, kPeerAddress);
// Let connection process a Google QUIC packet.
@@ -9128,7 +9113,6 @@
if (!framer_.version().SupportsClientConnectionIds()) {
return;
}
- EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
set_perspective(Perspective::IS_SERVER);
QuicPacketHeader header = ConstructPacketHeader(1, ENCRYPTION_INITIAL);
header.source_connection_id = TestConnectionId(0x33);
@@ -9569,7 +9553,6 @@
if (!connection_.version().SupportsAntiAmplificationLimit()) {
return;
}
- EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
EXPECT_CALL(visitor_, OnCryptoFrame(_)).Times(AnyNumber());
set_perspective(Perspective::IS_SERVER);
diff --git a/quic/core/quic_crypto_server_stream_test.cc b/quic/core/quic_crypto_server_stream_test.cc
index 360c268..2f7055d 100644
--- a/quic/core/quic_crypto_server_stream_test.cc
+++ b/quic/core/quic_crypto_server_stream_test.cc
@@ -100,8 +100,10 @@
crypto_test_utils::SetupCryptoServerConfigForTest(
server_connection_->clock(), server_connection_->random_generator(),
&server_crypto_config_);
- server_session_->GetMutableCryptoStream()->OnSuccessfulVersionNegotiation(
- supported_versions_.front());
+ if (!GetQuicReloadableFlag(quic_version_negotiated_by_default_at_server)) {
+ server_session_->GetMutableCryptoStream()->OnSuccessfulVersionNegotiation(
+ supported_versions_.front());
+ }
}
QuicCryptoServerStream* server_stream() {
diff --git a/quic/core/quic_session.cc b/quic/core/quic_session.cc
index cba27f0..2870b37 100644
--- a/quic/core/quic_session.cc
+++ b/quic/core/quic_session.cc
@@ -114,6 +114,13 @@
connection_->SetDataProducer(this);
connection_->SetFromConfig(config_);
+ // On the server side, version negotiation has been done by the dispatcher,
+ // and the server session is created with the right version.
+ if (connection_->quic_version_negotiated_by_default_at_server() &&
+ perspective() == Perspective::IS_SERVER) {
+ connection_->OnSuccessfulVersionNegotiation();
+ }
+
if (QuicVersionUsesCryptoFrames(transport_version())) {
return;
}
diff --git a/quic/quic_transport/quic_transport_server_session_test.cc b/quic/quic_transport/quic_transport_server_session_test.cc
index 42c3eef..5e67748 100644
--- a/quic/quic_transport/quic_transport_server_session_test.cc
+++ b/quic/quic_transport/quic_transport_server_session_test.cc
@@ -72,7 +72,9 @@
session_->Initialize();
crypto_stream_ = static_cast<QuicCryptoServerStream*>(
session_->GetMutableCryptoStream());
- crypto_stream_->OnSuccessfulVersionNegotiation(GetVersions()[0]);
+ if (!GetQuicReloadableFlag(quic_version_negotiated_by_default_at_server)) {
+ crypto_stream_->OnSuccessfulVersionNegotiation(GetVersions()[0]);
+ }
}
void Connect() {
diff --git a/quic/test_tools/crypto_test_utils.cc b/quic/test_tools/crypto_test_utils.cc
index 83efd21..6716dec 100644
--- a/quic/test_tools/crypto_test_utils.cc
+++ b/quic/test_tools/crypto_test_utils.cc
@@ -235,8 +235,10 @@
server_conn, *server_quic_config, client_conn->supported_versions(),
crypto_config, &compressed_certs_cache);
server_session.Initialize();
- server_session.OnSuccessfulVersionNegotiation(
- client_conn->supported_versions().front());
+ if (!GetQuicReloadableFlag(quic_version_negotiated_by_default_at_server)) {
+ server_session.OnSuccessfulVersionNegotiation(
+ client_conn->supported_versions().front());
+ }
EXPECT_CALL(*server_session.helper(),
CanAcceptClientHello(testing::_, testing::_, testing::_,
testing::_, testing::_))
diff --git a/quic/test_tools/quic_connection_peer.cc b/quic/test_tools/quic_connection_peer.cc
index 197514e..d48ff05 100644
--- a/quic/test_tools/quic_connection_peer.cc
+++ b/quic/test_tools/quic_connection_peer.cc
@@ -296,6 +296,11 @@
// static
void QuicConnectionPeer::SetNegotiatedVersion(QuicConnection* connection) {
connection->version_negotiated_ = true;
+ if (connection->perspective() == Perspective::IS_SERVER &&
+ !QuicFramerPeer::infer_packet_header_type_from_version(
+ &connection->framer_)) {
+ connection->framer_.InferPacketHeaderTypeFromVersion();
+ }
}
// static
diff --git a/quic/test_tools/quic_framer_peer.h b/quic/test_tools/quic_framer_peer.h
index 661def5..d462555 100644
--- a/quic/test_tools/quic_framer_peer.h
+++ b/quic/test_tools/quic_framer_peer.h
@@ -187,6 +187,10 @@
uint64_t current_received_frame_type) {
framer->current_received_frame_type_ = current_received_frame_type;
}
+
+ static bool infer_packet_header_type_from_version(QuicFramer* framer) {
+ return framer->infer_packet_header_type_from_version_;
+ }
};
} // namespace test
diff --git a/quic/tools/quic_simple_server_session_test.cc b/quic/tools/quic_simple_server_session_test.cc
index 4a12c09..a548c6c 100644
--- a/quic/tools/quic_simple_server_session_test.cc
+++ b/quic/tools/quic_simple_server_session_test.cc
@@ -230,8 +230,10 @@
QuicRandom::GetInstance(), &clock,
QuicCryptoServerConfig::ConfigOptions());
session_->Initialize();
- QuicSessionPeer::GetMutableCryptoStream(session_.get())
- ->OnSuccessfulVersionNegotiation(supported_versions.front());
+ if (!GetQuicReloadableFlag(quic_version_negotiated_by_default_at_server)) {
+ QuicSessionPeer::GetMutableCryptoStream(session_.get())
+ ->OnSuccessfulVersionNegotiation(supported_versions.front());
+ }
if (VersionHasIetfQuicFrames(transport_version())) {
EXPECT_CALL(*connection_, SendControlFrame(_))
@@ -590,8 +592,10 @@
config_, connection_, &owner_, &stream_helper_, &crypto_config_,
&compressed_certs_cache_, &memory_cache_backend_);
session_->Initialize();
- QuicSessionPeer::GetMutableCryptoStream(session_.get())
- ->OnSuccessfulVersionNegotiation(supported_versions.front());
+ if (!GetQuicReloadableFlag(quic_version_negotiated_by_default_at_server)) {
+ QuicSessionPeer::GetMutableCryptoStream(session_.get())
+ ->OnSuccessfulVersionNegotiation(supported_versions.front());
+ }
// Needed to make new session flow control window and server push work.
if (VersionHasIetfQuicFrames(transport_version())) {
