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";