Remove server in connection version negotiation code.
gfe-relnote: Deprecate gfe2_restart_flag_quic_no_server_conn_ver_negotiation2.
PiperOrigin-RevId: 254442620
Change-Id: I490aa71424a9cfb0ee36d1d6a202080f41eeec2f
diff --git a/quic/core/chlo_extractor.cc b/quic/core/chlo_extractor.cc
index 201c6df..4b38ad2 100644
--- a/quic/core/chlo_extractor.cc
+++ b/quic/core/chlo_extractor.cc
@@ -31,8 +31,7 @@
// QuicFramerVisitorInterface implementation
void OnError(QuicFramer* /*framer*/) override {}
- bool OnProtocolVersionMismatch(ParsedQuicVersion version,
- PacketHeaderFormat form) override;
+ bool OnProtocolVersionMismatch(ParsedQuicVersion version) override;
void OnPacket() override {}
void OnPublicResetPacket(const QuicPublicResetPacket& /*packet*/) override {}
void OnVersionNegotiationPacket(
@@ -106,8 +105,7 @@
chlo_contains_tags_(false),
connection_id_(EmptyQuicConnectionId()) {}
-bool ChloFramerVisitor::OnProtocolVersionMismatch(ParsedQuicVersion version,
- PacketHeaderFormat /*form*/) {
+bool ChloFramerVisitor::OnProtocolVersionMismatch(ParsedQuicVersion version) {
if (!framer_->IsSupportedVersion(version)) {
return false;
}
diff --git a/quic/core/http/end_to_end_test.cc b/quic/core/http/end_to_end_test.cc
index 93eceb0..96b7ee3 100644
--- a/quic/core/http/end_to_end_test.cc
+++ b/quic/core/http/end_to_end_test.cc
@@ -215,7 +215,6 @@
override_client_connection_id_(nullptr),
expected_server_connection_id_length_(kQuicDefaultConnectionIdLength) {
SetQuicFlag(FLAGS_quic_supports_tls_handshake, true);
- SetQuicRestartFlag(quic_no_server_conn_ver_negotiation2, true);
client_supported_versions_ = GetParam().client_supported_versions;
server_supported_versions_ = GetParam().server_supported_versions;
negotiated_version_ = GetParam().negotiated_version;
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc
index 485b4c9..c11d0c0 100644
--- a/quic/core/quic_connection.cc
+++ b/quic/core/quic_connection.cc
@@ -298,7 +298,7 @@
&stats_,
GetQuicReloadableFlag(quic_default_to_bbr) ? kBBR : kCubicBytes,
kNack),
- version_negotiation_state_(START_NEGOTIATION),
+ version_negotiated_(false),
perspective_(perspective),
connected_(true),
can_truncate_connection_ids_(perspective == Perspective::IS_SERVER),
@@ -322,12 +322,7 @@
processing_ack_frame_(false),
supports_release_time_(false),
release_time_into_future_(QuicTime::Delta::Zero()),
- no_version_negotiation_(supported_versions.size() == 1),
retry_has_been_parsed_(false) {
- if (perspective_ == Perspective::IS_SERVER &&
- supported_versions.size() == 1) {
- QUIC_RESTART_FLAG_COUNT(quic_no_server_conn_ver_negotiation2);
- }
QUIC_DLOG(INFO) << ENDPOINT << "Created connection with server connection ID "
<< server_connection_id
<< " and version: " << ParsedQuicVersionToString(version());
@@ -356,8 +351,7 @@
uber_received_packet_manager_.set_max_ack_ranges(255);
MaybeEnableSessionDecidesWhatToWrite();
MaybeEnableMultiplePacketNumberSpacesSupport();
- DCHECK(!GetQuicRestartFlag(quic_no_server_conn_ver_negotiation2) ||
- perspective_ == Perspective::IS_CLIENT ||
+ DCHECK(perspective_ == Perspective::IS_CLIENT ||
supported_versions.size() == 1);
InstallInitialCrypters();
}
@@ -537,8 +531,7 @@
}
bool QuicConnection::OnProtocolVersionMismatch(
- ParsedQuicVersion received_version,
- PacketHeaderFormat form) {
+ ParsedQuicVersion received_version) {
QUIC_DLOG(INFO) << ENDPOINT << "Received packet with mismatched version "
<< ParsedQuicVersionToString(received_version);
if (perspective_ == Perspective::IS_CLIENT) {
@@ -546,64 +539,11 @@
QUIC_BUG << ENDPOINT << error_details;
CloseConnection(QUIC_INTERNAL_ERROR, error_details,
ConnectionCloseBehavior::SILENT_CLOSE);
- return false;
- }
- if (no_version_negotiation_) {
- // Drop old packets that were sent by the client before the version was
- // negotiated.
- return false;
- }
- DCHECK_NE(version(), received_version);
-
- if (debug_visitor_ != nullptr) {
- debug_visitor_->OnProtocolVersionMismatch(received_version);
}
- switch (version_negotiation_state_) {
- case START_NEGOTIATION:
- if (!framer_.IsSupportedVersion(received_version)) {
- SendVersionNegotiationPacket(form != GOOGLE_QUIC_PACKET);
- version_negotiation_state_ = NEGOTIATION_IN_PROGRESS;
- return false;
- }
- break;
-
- case NEGOTIATION_IN_PROGRESS:
- if (!framer_.IsSupportedVersion(received_version)) {
- SendVersionNegotiationPacket(form != GOOGLE_QUIC_PACKET);
- return false;
- }
- break;
-
- case NEGOTIATED_VERSION:
- // Might be old packets that were sent by the client before the version
- // was negotiated. Drop these.
- return false;
-
- default:
- DCHECK(false);
- }
-
- // Store the new version.
- framer_.set_version(received_version);
- framer_.InferPacketHeaderTypeFromVersion();
-
- version_negotiation_state_ = NEGOTIATED_VERSION;
- visitor_->OnSuccessfulVersionNegotiation(received_version);
- if (debug_visitor_ != nullptr) {
- debug_visitor_->OnSuccessfulVersionNegotiation(received_version);
- }
- QUIC_DLOG(INFO) << ENDPOINT << "version negotiated "
- << ParsedQuicVersionToString(received_version);
-
- MaybeEnableSessionDecidesWhatToWrite();
- no_stop_waiting_frames_ =
- VersionHasIetfInvariantHeader(received_version.transport_version);
-
- // TODO(satyamshekhar): Store the packet number of this packet and close the
- // connection if we ever received a packet with incorrect version and whose
- // packet number is greater.
- return true;
+ // Server drops old packets that were sent by the client before the version
+ // was negotiated.
+ return false;
}
// Handles version negotiation for client connection.
@@ -626,7 +566,7 @@
debug_visitor_->OnVersionNegotiationPacket(packet);
}
- if (version_negotiation_state_ != START_NEGOTIATION) {
+ if (version_negotiated_) {
// Possibly a duplicate version negotiation packet.
return;
}
@@ -781,8 +721,7 @@
return false;
}
- if (version_negotiation_state_ != NEGOTIATED_VERSION &&
- perspective_ == Perspective::IS_SERVER) {
+ if (!version_negotiated_ && perspective_ == Perspective::IS_SERVER) {
if (!header.version_flag) {
// Packets should have the version flag till version negotiation is
// done.
@@ -795,14 +734,14 @@
return false;
} else {
DCHECK_EQ(header.version, version());
- version_negotiation_state_ = NEGOTIATED_VERSION;
+ version_negotiated_ = true;
framer_.InferPacketHeaderTypeFromVersion();
visitor_->OnSuccessfulVersionNegotiation(version());
if (debug_visitor_ != nullptr) {
debug_visitor_->OnSuccessfulVersionNegotiation(version());
}
}
- DCHECK_EQ(NEGOTIATED_VERSION, version_negotiation_state_);
+ DCHECK(version_negotiated_);
}
return true;
@@ -1894,7 +1833,7 @@
return false;
}
- if (version_negotiation_state_ != NEGOTIATED_VERSION) {
+ if (!version_negotiated_) {
if (perspective_ == Perspective::IS_CLIENT) {
DCHECK(!header.version_flag || header.form != GOOGLE_QUIC_PACKET);
if (!VersionHasIetfInvariantHeader(framer_.transport_version())) {
@@ -1904,7 +1843,7 @@
// forward secure.
packet_generator_.StopSendingVersion();
}
- version_negotiation_state_ = NEGOTIATED_VERSION;
+ version_negotiated_ = true;
visitor_->OnSuccessfulVersionNegotiation(version());
if (debug_visitor_ != nullptr) {
debug_visitor_->OnSuccessfulVersionNegotiation(version());
diff --git a/quic/core/quic_connection.h b/quic/core/quic_connection.h
index 947209e..9bb00b2 100644
--- a/quic/core/quic_connection.h
+++ b/quic/core/quic_connection.h
@@ -464,8 +464,7 @@
// From QuicFramerVisitorInterface
void OnError(QuicFramer* framer) override;
- bool OnProtocolVersionMismatch(ParsedQuicVersion received_version,
- PacketHeaderFormat form) override;
+ bool OnProtocolVersionMismatch(ParsedQuicVersion received_version) override;
void OnPacket() override;
void OnPublicResetPacket(const QuicPublicResetPacket& packet) override;
void OnVersionNegotiationPacket(
@@ -1310,21 +1309,8 @@
// to send packets.
QuicSentPacketManager sent_packet_manager_;
- // The state of connection in version negotiation finite state machine.
- enum QuicVersionNegotiationState {
- START_NEGOTIATION = 0,
- // Server-side this implies we've sent a version negotiation packet and are
- // waiting on the client to select a compatible version. Client-side this
- // implies we've gotten a version negotiation packet, are retransmitting the
- // initial packets with a supported version and are waiting for our first
- // packet from the server.
- NEGOTIATION_IN_PROGRESS,
- // This indicates this endpoint has received a packet from the peer with a
- // version this endpoint supports. Version negotiation is complete, and the
- // version number will no longer be sent with future packets.
- NEGOTIATED_VERSION
- };
- QuicVersionNegotiationState version_negotiation_state_;
+ // Indicates whether connection version has been negotiated.
+ bool version_negotiated_;
// Tracks if the connection was created by the server or the client.
Perspective perspective_;
@@ -1418,11 +1404,6 @@
// Time this connection can release packets into the future.
QuicTime::Delta release_time_into_future_;
- // Indicates whether server connection does version negotiation. Server
- // connection does not support version negotiation if a single version is
- // provided in constructor.
- const bool no_version_negotiation_;
-
// Payload of most recently transmitted IETF QUIC connectivity
// probe packet (the PATH_CHALLENGE payload). This implementation transmits
// only one PATH_CHALLENGE per connectivity probe, so only one
diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc
index 5201b5a..c7f6e07 100644
--- a/quic/core/quic_connection_test.cc
+++ b/quic/core/quic_connection_test.cc
@@ -743,7 +743,6 @@
void SetSupportedVersions(const ParsedQuicVersionVector& versions) {
QuicConnectionPeer::GetFramer(this)->SetSupportedVersions(versions);
- QuicConnectionPeer::SetNoVersionNegotiation(this, versions.size() == 1);
writer()->SetSupportedVersions(versions);
}
diff --git a/quic/core/quic_framer.cc b/quic/core/quic_framer.cc
index 83ea19b..5b061d7 100644
--- a/quic/core/quic_framer.cc
+++ b/quic/core/quic_framer.cc
@@ -1513,7 +1513,7 @@
if (header.version_flag && header.version != version_) {
if (perspective_ == Perspective::IS_SERVER) {
- if (!visitor_->OnProtocolVersionMismatch(header.version, header.form)) {
+ if (!visitor_->OnProtocolVersionMismatch(header.version)) {
RecordDroppedPacketReason(DroppedPacketReason::VERSION_MISMATCH);
return true;
}
diff --git a/quic/core/quic_framer.h b/quic/core/quic_framer.h
index 18420f1..3a3faa1 100644
--- a/quic/core/quic_framer.h
+++ b/quic/core/quic_framer.h
@@ -76,8 +76,8 @@
// |quic_version_|. The visitor should return true after it updates the
// version of the |framer_| to |received_version| or false to stop processing
// this packet.
- virtual bool OnProtocolVersionMismatch(ParsedQuicVersion received_version,
- PacketHeaderFormat form) = 0;
+ virtual bool OnProtocolVersionMismatch(
+ ParsedQuicVersion received_version) = 0;
// Called when a new packet has been received, before it
// has been validated or processed.
diff --git a/quic/core/quic_framer_test.cc b/quic/core/quic_framer_test.cc
index c2aea06..26e756f 100644
--- a/quic/core/quic_framer_test.cc
+++ b/quic/core/quic_framer_test.cc
@@ -213,8 +213,7 @@
retry_token_ = QuicMakeUnique<std::string>(std::string(retry_token));
}
- bool OnProtocolVersionMismatch(ParsedQuicVersion received_version,
- PacketHeaderFormat /*form*/) override {
+ bool OnProtocolVersionMismatch(ParsedQuicVersion received_version) override {
QUIC_DLOG(INFO) << "QuicFramer Version Mismatch, version: "
<< received_version;
++version_mismatch_;
diff --git a/quic/core/quic_ietf_framer_test.cc b/quic/core/quic_ietf_framer_test.cc
index e00ced6..23ec1cb 100644
--- a/quic/core/quic_ietf_framer_test.cc
+++ b/quic/core/quic_ietf_framer_test.cc
@@ -100,8 +100,8 @@
QuicConnectionId /*new_connection_id*/,
QuicStringPiece /*retry_token*/) override {}
- bool OnProtocolVersionMismatch(ParsedQuicVersion /*received_version*/,
- PacketHeaderFormat /*form*/) override {
+ bool OnProtocolVersionMismatch(
+ ParsedQuicVersion /*received_version*/) override {
return true;
}
diff --git a/quic/core/quic_versions.cc b/quic/core/quic_versions.cc
index 349b373..23b6094 100644
--- a/quic/core/quic_versions.cc
+++ b/quic/core/quic_versions.cc
@@ -433,7 +433,6 @@
SetQuicReloadableFlag(quic_eliminate_static_stream_map_3, true);
SetQuicReloadableFlag(quic_tolerate_reneging, true);
SetQuicReloadableFlag(quic_simplify_stop_waiting, true);
- SetQuicRestartFlag(quic_no_server_conn_ver_negotiation2, true);
SetQuicRestartFlag(quic_do_not_override_connection_id, true);
SetQuicRestartFlag(quic_use_allocated_connection_ids, true);
}
diff --git a/quic/test_tools/quic_connection_peer.cc b/quic/test_tools/quic_connection_peer.cc
index 6492a68..66d1a51 100644
--- a/quic/test_tools/quic_connection_peer.cc
+++ b/quic/test_tools/quic_connection_peer.cc
@@ -304,7 +304,7 @@
// static
void QuicConnectionPeer::SetNegotiatedVersion(QuicConnection* connection) {
- connection->version_negotiation_state_ = QuicConnection::NEGOTIATED_VERSION;
+ connection->version_negotiated_ = true;
}
// static
@@ -316,13 +316,6 @@
}
// static
-void QuicConnectionPeer::SetNoVersionNegotiation(QuicConnection* connection,
- bool no_version_negotiation) {
- *const_cast<bool*>(&connection->no_version_negotiation_) =
- no_version_negotiation;
-}
-
-// static
bool QuicConnectionPeer::SupportsReleaseTime(QuicConnection* connection) {
return connection->supports_release_time_;
}
diff --git a/quic/test_tools/quic_connection_peer.h b/quic/test_tools/quic_connection_peer.h
index 3cd8520..d0797f5 100644
--- a/quic/test_tools/quic_connection_peer.h
+++ b/quic/test_tools/quic_connection_peer.h
@@ -128,8 +128,6 @@
static void SetMaxConsecutiveNumPacketsWithNoRetransmittableFrames(
QuicConnection* connection,
size_t new_value);
- static void SetNoVersionNegotiation(QuicConnection* connection,
- bool no_version_negotiation);
static bool SupportsReleaseTime(QuicConnection* connection);
static QuicConnection::PacketContent GetCurrentPacketContent(
QuicConnection* connection);
diff --git a/quic/test_tools/quic_test_utils.cc b/quic/test_tools/quic_test_utils.cc
index 4ff4554..6ffcb8c 100644
--- a/quic/test_tools/quic_test_utils.cc
+++ b/quic/test_tools/quic_test_utils.cc
@@ -192,7 +192,7 @@
MockFramerVisitor::MockFramerVisitor() {
// By default, we want to accept packets.
- ON_CALL(*this, OnProtocolVersionMismatch(_, _))
+ ON_CALL(*this, OnProtocolVersionMismatch(_))
.WillByDefault(testing::Return(false));
// By default, we want to accept packets.
@@ -232,8 +232,8 @@
MockFramerVisitor::~MockFramerVisitor() {}
-bool NoOpFramerVisitor::OnProtocolVersionMismatch(ParsedQuicVersion /*version*/,
- PacketHeaderFormat /*form*/) {
+bool NoOpFramerVisitor::OnProtocolVersionMismatch(
+ ParsedQuicVersion /*version*/) {
return false;
}
@@ -479,8 +479,7 @@
}
bool MockQuicConnection::OnProtocolVersionMismatch(
- ParsedQuicVersion /*version*/,
- PacketHeaderFormat /*form*/) {
+ ParsedQuicVersion /*version*/) {
return false;
}
diff --git a/quic/test_tools/quic_test_utils.h b/quic/test_tools/quic_test_utils.h
index 221dd6f..db103de 100644
--- a/quic/test_tools/quic_test_utils.h
+++ b/quic/test_tools/quic_test_utils.h
@@ -241,8 +241,7 @@
MOCK_METHOD1(OnError, void(QuicFramer* framer));
// The constructor sets this up to return false by default.
- MOCK_METHOD2(OnProtocolVersionMismatch,
- bool(ParsedQuicVersion version, PacketHeaderFormat form));
+ MOCK_METHOD1(OnProtocolVersionMismatch, bool(ParsedQuicVersion version));
MOCK_METHOD0(OnPacket, void());
MOCK_METHOD1(OnPublicResetPacket, void(const QuicPublicResetPacket& header));
MOCK_METHOD1(OnVersionNegotiationPacket,
@@ -306,8 +305,7 @@
void OnRetryPacket(QuicConnectionId /*original_connection_id*/,
QuicConnectionId /*new_connection_id*/,
QuicStringPiece /*retry_token*/) override {}
- bool OnProtocolVersionMismatch(ParsedQuicVersion version,
- PacketHeaderFormat form) override;
+ bool OnProtocolVersionMismatch(ParsedQuicVersion version) override;
bool OnUnauthenticatedHeader(const QuicPacketHeader& header) override;
bool OnUnauthenticatedPublicHeader(const QuicPacketHeader& header) override;
void OnDecryptedPacket(EncryptionLevel /*level*/) override {}
@@ -530,8 +528,7 @@
QuicConnection::ProcessUdpPacket(self_address, peer_address, packet);
}
- bool OnProtocolVersionMismatch(ParsedQuicVersion version,
- PacketHeaderFormat form) override;
+ bool OnProtocolVersionMismatch(ParsedQuicVersion version) override;
bool ReallySendControlFrame(const QuicFrame& frame) {
return QuicConnection::SendControlFrame(frame);
diff --git a/quic/test_tools/simple_quic_framer.cc b/quic/test_tools/simple_quic_framer.cc
index 07988cc..547f6d2 100644
--- a/quic/test_tools/simple_quic_framer.cc
+++ b/quic/test_tools/simple_quic_framer.cc
@@ -24,8 +24,7 @@
void OnError(QuicFramer* framer) override { error_ = framer->error(); }
- bool OnProtocolVersionMismatch(ParsedQuicVersion /*version*/,
- PacketHeaderFormat /*form*/) override {
+ bool OnProtocolVersionMismatch(ParsedQuicVersion /*version*/) override {
return false;
}
diff --git a/quic/tools/quic_packet_printer_bin.cc b/quic/tools/quic_packet_printer_bin.cc
index 8b148f9..a87f10e 100644
--- a/quic/tools/quic_packet_printer_bin.cc
+++ b/quic/tools/quic_packet_printer_bin.cc
@@ -48,8 +48,7 @@
std::cerr << "OnError: " << QuicErrorCodeToString(framer->error())
<< " detail: " << framer->detailed_error() << "\n";
}
- bool OnProtocolVersionMismatch(ParsedQuicVersion received_version,
- PacketHeaderFormat /*form*/) override {
+ bool OnProtocolVersionMismatch(ParsedQuicVersion received_version) override {
framer_->set_version(received_version);
std::cerr << "OnProtocolVersionMismatch: "
<< ParsedQuicVersionToString(received_version) << "\n";
