Enforce `QuicConnectionMigrationManager` and `QuicSpdyClientSessionWithMigration` to use QuicForceBlockablePacketWriter during construction.
And add a new subclass `QuicClientPathValidationContext` to enforce that client-side path validation contexts provide a `QuicForceBlockablePacketWriter`.
With both enforcement, remove unsafe static_cast from QuicConnection::writer() to `QuicForceBlockablePacketWriter` in `QuicConnectionMigrationManager` by caching the `QuicForceBlockablePacketWriter` object in QuicSpdyClientSessionWithMigration whenever there are path changes.
PiperOrigin-RevId: 804421523
diff --git a/quiche/quic/core/http/quic_connection_migration_manager.cc b/quiche/quic/core/http/quic_connection_migration_manager.cc
index 1988d84..a88dc5f 100644
--- a/quiche/quic/core/http/quic_connection_migration_manager.cc
+++ b/quiche/quic/core/http/quic_connection_migration_manager.cc
@@ -91,13 +91,13 @@
QuicConnectionMigrationManager* absl_nonnull migration_manager)
: migration_manager_(migration_manager) {}
void OnPathValidationSuccess(
- std::unique_ptr<quic::QuicPathValidationContext> context,
+ std::unique_ptr<QuicPathValidationContext> context,
quic::QuicTime start_time) override {
migration_manager_->OnConnectionMigrationProbeSucceeded(std::move(context),
start_time);
}
void OnPathValidationFailure(
- std::unique_ptr<quic::QuicPathValidationContext> context) override {
+ std::unique_ptr<QuicPathValidationContext> context) override {
migration_manager_->OnProbeFailed(std::move(context));
}
@@ -364,7 +364,7 @@
void QuicConnectionMigrationManager::
PathContextCreationResultDelegateForImmediateMigration::OnCreationSucceeded(
- std::unique_ptr<QuicPathValidationContext> context) {
+ std::unique_ptr<QuicClientPathValidationContext> context) {
migration_manager_->FinishMigrate(std::move(context), close_session_on_error_,
std::move(migration_callback_));
}
@@ -372,9 +372,7 @@
void QuicConnectionMigrationManager::
PathContextCreationResultDelegateForImmediateMigration::OnCreationFailed(
QuicNetworkHandle network, absl::string_view error) {
- static_cast<QuicForceBlockablePacketWriter*>(
- migration_manager_->connection_->writer())
- ->ForceWriteBlocked(false);
+ migration_manager_->session_->writer()->ForceWriteBlocked(false);
std::move(migration_callback_)(network, MigrationResult::FAILURE);
if (close_session_on_error_) {
migration_manager_->session_->OnConnectionToBeClosedDueToMigrationError(
@@ -398,7 +396,7 @@
void QuicConnectionMigrationManager::
PathContextCreationResultDelegateForProbing::OnCreationSucceeded(
- std::unique_ptr<QuicPathValidationContext> context) {
+ std::unique_ptr<QuicClientPathValidationContext> context) {
migration_manager_->FinishStartProbing(std::move(probing_callback_),
std::move(context));
}
@@ -447,8 +445,7 @@
"Connection is migrating with an invalid network handle.");
}
QUIC_DVLOG(1) << "Force blocking the current packet writer";
- static_cast<QuicForceBlockablePacketWriter*>(connection_->writer())
- ->ForceWriteBlocked(true);
+ session_->writer()->ForceWriteBlocked(true);
if (config_.disable_blackhole_detection_on_immediate_migrate) {
// Turn off the black hole detector since the writer is blocked.
// Blackhole will be re-enabled once a packet is sent again.
@@ -478,14 +475,13 @@
}
void QuicConnectionMigrationManager::FinishMigrate(
- std::unique_ptr<QuicPathValidationContext> path_context,
+ std::unique_ptr<QuicClientPathValidationContext> path_context,
bool close_session_on_error, MigrationCallback callback) {
// Migrate to the new socket.
MigrationCause current_migration_cause = current_migration_cause_;
QuicNetworkHandle network = path_context->network();
if (!session_->MigrateToNewPath(std::move(path_context))) {
- static_cast<QuicForceBlockablePacketWriter*>(connection_->writer())
- ->ForceWriteBlocked(false);
+ session_->writer()->ForceWriteBlocked(false);
std::move(callback)(network, MigrationResult::FAILURE);
if (close_session_on_error) {
session_->OnConnectionToBeClosedDueToMigrationError(
@@ -514,8 +510,7 @@
<< MigrationCauseToString(current_migration_cause_);
// Force blocking the packet writer to avoid any writes since there is no
// alternate network available.
- static_cast<QuicForceBlockablePacketWriter*>(connection_->writer())
- ->ForceWriteBlocked(true);
+ session_->writer()->ForceWriteBlocked(true);
if (config_.disable_blackhole_detection_on_immediate_migrate) {
// Turn off the black hole detector since the writer is blocked.
// Blackhole will be re-enabled once a packet is sent again.
@@ -768,7 +763,7 @@
void QuicConnectionMigrationManager::FinishStartProbing(
StartProbingCallback probing_callback,
- std::unique_ptr<QuicPathValidationContext> path_context) {
+ std::unique_ptr<QuicClientPathValidationContext> path_context) {
session_->PrepareForProbingOnPath(*path_context);
switch (current_migration_cause_) {
case MigrationCause::CHANGE_PORT_ON_PATH_DEGRADING:
@@ -836,7 +831,10 @@
return;
}
// Migrate to the probed socket immediately.
- if (!session_->MigrateToNewPath(std::move(path_context))) {
+ if (!session_->MigrateToNewPath(
+ std::unique_ptr<QuicClientPathValidationContext>(
+ static_cast<QuicClientPathValidationContext*>(
+ path_context.release())))) {
if (debug_visitor_) {
debug_visitor_->OnConnectionMigrationFailedAfterProbe();
}
@@ -881,7 +879,10 @@
return;
}
// Migrate to the probed socket immediately.
- if (!session_->MigrateToNewPath(std::move(path_context))) {
+ if (!session_->MigrateToNewPath(
+ std::unique_ptr<QuicClientPathValidationContext>(
+ static_cast<QuicClientPathValidationContext*>(
+ path_context.release())))) {
if (debug_visitor_) {
debug_visitor_->OnConnectionMigrationFailedAfterProbe();
}
diff --git a/quiche/quic/core/http/quic_connection_migration_manager.h b/quiche/quic/core/http/quic_connection_migration_manager.h
index 243d63d..25ebf12 100644
--- a/quiche/quic/core/http/quic_connection_migration_manager.h
+++ b/quiche/quic/core/http/quic_connection_migration_manager.h
@@ -239,7 +239,7 @@
bool close_session_on_error, MigrationCallback migration_callback);
void OnCreationSucceeded(
- std::unique_ptr<QuicPathValidationContext> context) override;
+ std::unique_ptr<QuicClientPathValidationContext> context) override;
void OnCreationFailed(QuicNetworkHandle network,
absl::string_view error) override;
@@ -261,7 +261,7 @@
StartProbingCallback probing_callback);
void OnCreationSucceeded(
- std::unique_ptr<QuicPathValidationContext> context) override;
+ std::unique_ptr<QuicClientPathValidationContext> context) override;
void OnCreationFailed(QuicNetworkHandle network,
absl::string_view error) override;
@@ -294,8 +294,9 @@
bool close_session_on_error,
MigrationCallback migration_callback);
// Helper to finish session migration once the |path_context| is provided.
- void FinishMigrate(std::unique_ptr<QuicPathValidationContext> path_context,
- bool close_session_on_error, MigrationCallback callback);
+ void FinishMigrate(
+ std::unique_ptr<QuicClientPathValidationContext> path_context,
+ bool close_session_on_error, MigrationCallback callback);
void StartMigrateBackToDefaultNetworkTimer(QuicTimeDelta delay);
void CancelMigrateBackToDefaultNetworkTimer();
@@ -312,7 +313,7 @@
const QuicSocketAddress& peer_address);
void FinishStartProbing(
StartProbingCallback probing_callback,
- std::unique_ptr<QuicPathValidationContext> path_context);
+ std::unique_ptr<QuicClientPathValidationContext> path_context);
bool MaybeCloseIdleSession(bool has_write_error,
ConnectionCloseBehavior close_behavior);
diff --git a/quiche/quic/core/http/quic_connection_migration_manager_test.cc b/quiche/quic/core/http/quic_connection_migration_manager_test.cc
index aff6e28..f933d5b 100644
--- a/quiche/quic/core/http/quic_connection_migration_manager_test.cc
+++ b/quiche/quic/core/http/quic_connection_migration_manager_test.cc
@@ -112,15 +112,18 @@
bool force_write_blocked_ = false;
};
-class TestQuicClientPathValidationContext : public QuicPathValidationContext {
+class TestQuicClientPathValidationContext
+ : public QuicClientPathValidationContext {
public:
TestQuicClientPathValidationContext(
const quic::QuicSocketAddress& self_address,
const quic::QuicSocketAddress& peer_address, QuicNetworkHandle network)
- : QuicPathValidationContext(self_address, peer_address, network),
+ : QuicClientPathValidationContext(self_address, peer_address, network),
writer_(std::make_unique<TestQuicForceBlockablePacketWriter>()) {}
- quic::QuicPacketWriter* WriterToUse() override { return writer_.get(); }
+ QuicForceBlockablePacketWriter* ForceBlockableWriterToUse() override {
+ return writer_.get();
+ }
bool ShouldConnectionOwnWriter() const override { return true; }
@@ -359,15 +362,16 @@
: public QuicSpdyClientSessionWithMigration {
public:
TestQuicSpdyClientSessionWithMigration(
- QuicConnection* connection, QuicSession::Visitor* visitor,
- const QuicConfig& config,
+ QuicConnection* connection, QuicForceBlockablePacketWriter* writer,
+ QuicSession::Visitor* visitor, const QuicConfig& config,
const ParsedQuicVersionVector& supported_versions,
QuicNetworkHandle default_network, QuicNetworkHandle current_network,
std::unique_ptr<QuicPathContextFactory> path_context_factory,
QuicConnectionMigrationConfig migration_config)
: QuicSpdyClientSessionWithMigration(
- connection, visitor, config, supported_versions, default_network,
- current_network, std::move(path_context_factory), migration_config),
+ connection, writer, visitor, config, supported_versions,
+ default_network, current_network, std::move(path_context_factory),
+ migration_config),
crypto_stream_(this) {
ON_CALL(*this, IsSessionProxied()).WillByDefault(Return(false));
ON_CALL(*this, OnMigrationToPathDone(_, _))
@@ -387,9 +391,10 @@
(QuicPathValidationContext & context), (override));
MOCK_METHOD(bool, IsSessionProxied, (), (const));
MOCK_METHOD(QuicTimeDelta, TimeSinceLastStreamClose, (), (override));
- MOCK_METHOD(bool, PrepareForMigrationToPath, (QuicPathValidationContext&));
+ MOCK_METHOD(bool, PrepareForMigrationToPath,
+ (QuicClientPathValidationContext&));
MOCK_METHOD(void, OnMigrationToPathDone,
- (std::unique_ptr<QuicPathValidationContext>, bool));
+ (std::unique_ptr<QuicClientPathValidationContext>, bool));
MOCK_METHOD(void, OnConnectionToBeClosedDueToMigrationError,
(MigrationCause migration_cause, QuicErrorCode quic_error),
(override));
@@ -497,8 +502,8 @@
migration_config_.migrate_idle_session = migrate_idle_session_;
session_ = std::make_unique<TestQuicSpdyClientSessionWithMigration>(
- connection_, &session_visitor_, config_, versions_, default_network_,
- initial_network_,
+ connection_, default_writer_, &session_visitor_, config_, versions_,
+ default_network_, initial_network_,
std::unique_ptr<QuicPathContextFactory>(path_context_factory_),
migration_config_);
session_->Initialize();
diff --git a/quiche/quic/core/http/quic_spdy_client_session_with_migration.cc b/quiche/quic/core/http/quic_spdy_client_session_with_migration.cc
index 7b55176..6e80690 100644
--- a/quiche/quic/core/http/quic_spdy_client_session_with_migration.cc
+++ b/quiche/quic/core/http/quic_spdy_client_session_with_migration.cc
@@ -4,11 +4,14 @@
#include "quiche/quic/core/http/quic_spdy_client_session_with_migration.h"
+#include "quiche/quic/core/quic_force_blockable_packet_writer.h"
+
namespace quic {
QuicSpdyClientSessionWithMigration::QuicSpdyClientSessionWithMigration(
- QuicConnection* connection, QuicSession::Visitor* visitor,
- const QuicConfig& config, const ParsedQuicVersionVector& supported_versions,
+ QuicConnection* connection, QuicForceBlockablePacketWriter* writer,
+ QuicSession::Visitor* visitor, const QuicConfig& config,
+ const ParsedQuicVersionVector& supported_versions,
QuicNetworkHandle default_network, QuicNetworkHandle current_network,
std::unique_ptr<QuicPathContextFactory> path_context_factory,
QuicConnectionMigrationConfig migration_config)
@@ -17,7 +20,11 @@
path_context_factory_(std::move(path_context_factory)),
migration_manager_(this, connection->clock(), default_network,
current_network, path_context_factory_.get(),
- migration_config) {}
+ migration_config),
+ writer_(writer) {
+ QUICHE_DCHECK_EQ(writer_, connection->writer())
+ << "Writer is not the connection writer";
+}
QuicSpdyClientSessionWithMigration::~QuicSpdyClientSessionWithMigration() =
default;
@@ -41,7 +48,7 @@
}
bool QuicSpdyClientSessionWithMigration::MigrateToNewPath(
- std::unique_ptr<QuicPathValidationContext> path_context) {
+ std::unique_ptr<QuicClientPathValidationContext> path_context) {
if (!PrepareForMigrationToPath(*path_context)) {
QUIC_CLIENT_HISTOGRAM_BOOL("QuicSession.PrepareForMigrationToPath", false,
"");
@@ -57,6 +64,8 @@
"No unused server connection ID");
QUIC_DVLOG(1) << "MigratePath fails as there is no CID available";
}
+ writer_ = path_context->ForceBlockableWriterToUse();
+ QUICHE_DCHECK_EQ(writer_, connection()->writer());
OnMigrationToPathDone(std::move(path_context), success);
return success;
}
diff --git a/quiche/quic/core/http/quic_spdy_client_session_with_migration.h b/quiche/quic/core/http/quic_spdy_client_session_with_migration.h
index f4aeab2..6d3346d 100644
--- a/quiche/quic/core/http/quic_spdy_client_session_with_migration.h
+++ b/quiche/quic/core/http/quic_spdy_client_session_with_migration.h
@@ -12,6 +12,7 @@
#include "quiche/quic/core/quic_config.h"
#include "quiche/quic/core/quic_connection.h"
#include "quiche/quic/core/quic_error_codes.h"
+#include "quiche/quic/core/quic_force_blockable_packet_writer.h"
#include "quiche/quic/core/quic_path_context_factory.h"
#include "quiche/quic/core/quic_session.h"
#include "quiche/quic/core/quic_time.h"
@@ -33,8 +34,8 @@
: public QuicSpdyClientSessionBase {
public:
QuicSpdyClientSessionWithMigration(
- QuicConnection* connection, QuicSession::Visitor* visitor,
- const QuicConfig& config,
+ QuicConnection* connection, QuicForceBlockablePacketWriter* writer,
+ QuicSession::Visitor* visitor, const QuicConfig& config,
const ParsedQuicVersionVector& supported_versions,
QuicNetworkHandle default_network, QuicNetworkHandle current_network,
std::unique_ptr<QuicPathContextFactory> path_context_factory,
@@ -78,7 +79,7 @@
// network.
// Returns true on successful migration.
bool MigrateToNewPath(
- std::unique_ptr<QuicPathValidationContext> path_context);
+ std::unique_ptr<QuicClientPathValidationContext> path_context);
void SetMigrationDebugVisitor(
quic::QuicConnectionMigrationDebugVisitor* visitor);
@@ -93,19 +94,23 @@
return migration_manager_;
}
+ QuicForceBlockablePacketWriter* writer() { return writer_; }
+
protected:
// Called in MigrateToNewPath() prior to calling MigratePath().
// Return false if MigratePath() should be skipped.
virtual bool PrepareForMigrationToPath(
- QuicPathValidationContext& context) = 0;
+ QuicClientPathValidationContext& context) = 0;
// Called in MigrateToNewPath() after MigratePath() for clean up.
virtual void OnMigrationToPathDone(
- std::unique_ptr<QuicPathValidationContext> context, bool success) = 0;
+ std::unique_ptr<QuicClientPathValidationContext> context,
+ bool success) = 0;
private:
std::unique_ptr<QuicPathContextFactory> path_context_factory_;
QuicConnectionMigrationManager migration_manager_;
+ QuicForceBlockablePacketWriter* absl_nonnull writer_;
};
} // namespace quic
diff --git a/quiche/quic/core/quic_path_context_factory.h b/quiche/quic/core/quic_path_context_factory.h
index 0134587..5fa5c2d 100644
--- a/quiche/quic/core/quic_path_context_factory.h
+++ b/quiche/quic/core/quic_path_context_factory.h
@@ -8,14 +8,40 @@
#include <memory>
#include "absl/strings/string_view.h"
+#include "quiche/quic/core/quic_force_blockable_packet_writer.h"
#include "quiche/quic/core/quic_path_validator.h"
#include "quiche/quic/platform/api/quic_socket_address.h"
#include "quiche/common/platform/api/quiche_export.h"
namespace quic {
-// An interface for creating QuicPathValidationContext objects used for probing
-// and migrating path.
+// A client side interface that enforces the writer to be force blockable.
+class QUICHE_EXPORT QuicClientPathValidationContext
+ : public QuicPathValidationContext {
+ public:
+ QuicClientPathValidationContext(const QuicSocketAddress& self_address,
+ const QuicSocketAddress& peer_address,
+ QuicNetworkHandle network)
+ : QuicClientPathValidationContext(self_address, peer_address,
+ peer_address, network) {}
+
+ QuicClientPathValidationContext(
+ const QuicSocketAddress& self_address,
+ const QuicSocketAddress& peer_address,
+ const QuicSocketAddress& effective_peer_address,
+ QuicNetworkHandle network)
+ : QuicPathValidationContext(self_address, peer_address,
+ effective_peer_address, network) {}
+
+ QuicPacketWriter* WriterToUse() override {
+ return ForceBlockableWriterToUse();
+ }
+
+ virtual QuicForceBlockablePacketWriter* ForceBlockableWriterToUse() = 0;
+};
+
+// An interface for creating QuicClientPathValidationContext objects used for
+// probing and migrating path.
class QUICHE_EXPORT QuicPathContextFactory {
public:
// An interface to handle creation success and failure given the creation
@@ -27,7 +53,7 @@
// Called when the manager successfully created a path context.
// |context| must not be nullptr.
virtual void OnCreationSucceeded(
- std::unique_ptr<QuicPathValidationContext> context) = 0;
+ std::unique_ptr<QuicClientPathValidationContext> context) = 0;
// Called when the manager fails to create a path context on |network|.
// |error| will be populated with details.
