Fix TlsChloExtractor move assignment operator
The default move assignement operator does not
update the delegates on various members, which
means that callbacks will be sent to the wrong
instance. This CL fixes that and adds a
regression test.
gfe-relnote: n/a, unused code
PiperOrigin-RevId: 308172454
Change-Id: Ie2784af64071b21995207b9ce2341443b4181d26
diff --git a/quic/core/tls_chlo_extractor.cc b/quic/core/tls_chlo_extractor.cc
index 5221ee1..7c10d2c 100644
--- a/quic/core/tls_chlo_extractor.cc
+++ b/quic/core/tls_chlo_extractor.cc
@@ -26,6 +26,34 @@
state_(State::kInitial),
parsed_crypto_frame_in_this_packet_(false) {}
+TlsChloExtractor::TlsChloExtractor(TlsChloExtractor&& other)
+ : TlsChloExtractor() {
+ *this = std::move(other);
+}
+
+TlsChloExtractor& TlsChloExtractor::operator=(TlsChloExtractor&& other) {
+ framer_ = std::move(other.framer_);
+ if (framer_) {
+ framer_->set_visitor(this);
+ }
+ crypto_stream_sequencer_ = std::move(other.crypto_stream_sequencer_);
+ crypto_stream_sequencer_.set_stream(this);
+ ssl_ = std::move(other.ssl_);
+ if (ssl_) {
+ std::pair<SSL_CTX*, int> shared_handles = GetSharedSslHandles();
+ int ex_data_index = shared_handles.second;
+ const int rv = SSL_set_ex_data(ssl_.get(), ex_data_index, this);
+ CHECK_EQ(rv, 1) << "Internal allocation failure in SSL_set_ex_data";
+ }
+ state_ = other.state_;
+ error_details_ = std::move(other.error_details_);
+ parsed_crypto_frame_in_this_packet_ =
+ other.parsed_crypto_frame_in_this_packet_;
+ alpns_ = std::move(other.alpns_);
+ server_name_ = std::move(other.server_name_);
+ return *this;
+}
+
void TlsChloExtractor::IngestPacket(const ParsedQuicVersion& version,
const QuicReceivedPacket& packet) {
if (state_ == State::kUnrecoverableFailure) {
@@ -318,13 +346,7 @@
ssl_ = bssl::UniquePtr<SSL>(SSL_new(ssl_ctx));
const int rv = SSL_set_ex_data(ssl_.get(), ex_data_index, this);
- if (rv != 1) {
- std::string error_details =
- quiche::QuicheStrCat("SSL_set_ex_data(", ex_data_index, ") failed");
- QUIC_BUG << error_details;
- HandleUnrecoverableError(error_details);
- return;
- }
+ CHECK_EQ(rv, 1) << "Internal allocation failure in SSL_set_ex_data";
SSL_set_accept_state(ssl_.get());
}
@@ -364,4 +386,10 @@
return quiche::QuicheStrCat("Unknown(", static_cast<int>(state), ")");
}
+std::ostream& operator<<(std::ostream& os,
+ const TlsChloExtractor::State& state) {
+ os << TlsChloExtractor::StateToString(state);
+ return os;
+}
+
} // namespace quic
diff --git a/quic/core/tls_chlo_extractor.h b/quic/core/tls_chlo_extractor.h
index af47144..1762566 100644
--- a/quic/core/tls_chlo_extractor.h
+++ b/quic/core/tls_chlo_extractor.h
@@ -28,9 +28,9 @@
public:
TlsChloExtractor();
TlsChloExtractor(const TlsChloExtractor&) = delete;
- TlsChloExtractor(TlsChloExtractor&&) = default;
+ TlsChloExtractor(TlsChloExtractor&&);
TlsChloExtractor& operator=(const TlsChloExtractor&) = delete;
- TlsChloExtractor& operator=(TlsChloExtractor&&) = default;
+ TlsChloExtractor& operator=(TlsChloExtractor&&);
enum class State : uint8_t {
kInitial = 0,
@@ -229,6 +229,10 @@
std::string server_name_;
};
+// Convenience method to facilitate logging TlsChloExtractor::State.
+QUIC_NO_EXPORT std::ostream& operator<<(std::ostream& os,
+ const TlsChloExtractor::State& state);
+
} // namespace quic
#endif // QUICHE_QUIC_CORE_TLS_CHLO_EXTRACTOR_H_
diff --git a/quic/core/tls_chlo_extractor_test.cc b/quic/core/tls_chlo_extractor_test.cc
index ab99a72..ba57ad0 100644
--- a/quic/core/tls_chlo_extractor_test.cc
+++ b/quic/core/tls_chlo_extractor_test.cc
@@ -104,6 +104,54 @@
TlsChloExtractor::State::kParsedFullMultiPacketChlo);
}
+TEST_P(TlsChloExtractorTest, MoveAssignment) {
+ Initialize();
+ EXPECT_EQ(packets_.size(), 1u);
+ TlsChloExtractor other_extractor;
+ tls_chlo_extractor_ = std::move(other_extractor);
+ IngestPackets();
+ ValidateChloDetails();
+ EXPECT_EQ(tls_chlo_extractor_.state(),
+ TlsChloExtractor::State::kParsedFullSinglePacketChlo);
+}
+
+TEST_P(TlsChloExtractorTest, MoveAssignmentBetweenPackets) {
+ IncreaseSizeOfChlo();
+ Initialize();
+ ASSERT_EQ(packets_.size(), 2u);
+ TlsChloExtractor other_extractor;
+
+ // Have |other_extractor| parse the first packet.
+ ReceivedPacketInfo packet_info(
+ QuicSocketAddress(TestPeerIPAddress(), kTestPort),
+ QuicSocketAddress(TestPeerIPAddress(), kTestPort), *packets_[0]);
+ std::string detailed_error;
+ bool retry_token_present;
+ quiche::QuicheStringPiece retry_token;
+ const QuicErrorCode error = QuicFramer::ParsePublicHeaderDispatcher(
+ *packets_[0], /*expected_destination_connection_id_length=*/0,
+ &packet_info.form, &packet_info.long_packet_type,
+ &packet_info.version_flag, &packet_info.use_length_prefix,
+ &packet_info.version_label, &packet_info.version,
+ &packet_info.destination_connection_id, &packet_info.source_connection_id,
+ &retry_token_present, &retry_token, &detailed_error);
+ ASSERT_THAT(error, IsQuicNoError()) << detailed_error;
+ other_extractor.IngestPacket(packet_info.version, packet_info.packet);
+ // Remove the first packet from the list.
+ packets_.erase(packets_.begin());
+ EXPECT_EQ(packets_.size(), 1u);
+
+ // Move the extractor.
+ tls_chlo_extractor_ = std::move(other_extractor);
+
+ // Have |tls_chlo_extractor_| parse the second packet.
+ IngestPackets();
+
+ ValidateChloDetails();
+ EXPECT_EQ(tls_chlo_extractor_.state(),
+ TlsChloExtractor::State::kParsedFullMultiPacketChlo);
+}
+
} // namespace
} // namespace test
} // namespace quic
