Add an `OnConfigNegotiated()` method to `QuicSession::Visitor()`
and use it in various tests to avoid using the session's config
after the handshake is complete.
PiperOrigin-RevId: 834861444
diff --git a/quiche/quic/core/http/end_to_end_test.cc b/quiche/quic/core/http/end_to_end_test.cc
index be911c0..e305629 100644
--- a/quiche/quic/core/http/end_to_end_test.cc
+++ b/quiche/quic/core/http/end_to_end_test.cc
@@ -111,6 +111,7 @@
#include "quiche/quic/tools/quic_backend_response.h"
#include "quiche/quic/tools/quic_default_client.h"
#include "quiche/quic/tools/quic_server.h"
+#include "quiche/quic/tools/quic_simple_dispatcher.h"
#include "quiche/quic/tools/quic_simple_server_backend.h"
#include "quiche/quic/tools/quic_simple_server_stream.h"
#include "quiche/quic/tools/quic_spdy_client_base.h"
@@ -1050,6 +1051,18 @@
client_writer_ = nullptr;
}
+ QuicConfig PauseServerAndGetLastNegotiatedConfigFromDispatcher() {
+ server_thread_->Pause();
+ QuicSimpleDispatcher* dispatcher = static_cast<QuicSimpleDispatcher*>(
+ QuicServerPeer::GetDispatcher(server_thread_->server()));
+ std::optional<QuicConfig> config = dispatcher->last_negotiated_config();
+ if (!config.has_value()) {
+ ADD_FAILURE() << "Missing negotiated config ";
+ return {};
+ }
+ return *dispatcher->last_negotiated_config();
+ }
+
void TestMultiPacketChaosProtection(int num_packets, bool drop_first_packet,
bool kyber = false);
@@ -4062,22 +4075,18 @@
// IFWA only exists with QUIC_CRYPTO.
// Client should have the right values for server's receive window.
ASSERT_TRUE(client_->client()
- ->client_session()
- ->config()
+ ->negotiated_config()
->HasReceivedInitialStreamFlowControlWindowBytes());
EXPECT_EQ(kServerStreamIFCW,
client_->client()
- ->client_session()
- ->config()
+ ->negotiated_config()
->ReceivedInitialStreamFlowControlWindowBytes());
ASSERT_TRUE(client_->client()
- ->client_session()
- ->config()
+ ->negotiated_config()
->HasReceivedInitialSessionFlowControlWindowBytes());
EXPECT_EQ(kServerSessionIFCW,
client_->client()
- ->client_session()
- ->config()
+ ->negotiated_config()
->ReceivedInitialSessionFlowControlWindowBytes());
}
EXPECT_EQ(kServerStreamIFCW, QuicStreamPeer::SendWindowOffset(stream));
@@ -4087,14 +4096,15 @@
client_session->flow_controller()));
// Server should have the right values for client's receive window.
- server_thread_->Pause();
+ QuicConfig server_config =
+ PauseServerAndGetLastNegotiatedConfigFromDispatcher();
+
QuicSpdySession* server_session = GetServerSession();
if (server_session == nullptr) {
ADD_FAILURE() << "Missing server session";
server_thread_->Resume();
return;
}
- QuicConfig server_config = *server_session->config();
EXPECT_EQ(kClientSessionIFCW, QuicFlowControllerPeer::SendWindowOffset(
server_session->flow_controller()));
server_thread_->Resume();
@@ -4142,18 +4152,17 @@
ASSERT_TRUE(client_session);
if (!version_.IsIetfQuic()) {
+ std::optional<QuicConfig> config = client_->client()->negotiated_config();
+ ASSERT_TRUE(config.has_value());
+
// IFWA only exists with QUIC_CRYPTO.
// Client should have the right values for server's receive window.
- ASSERT_TRUE(client_session->config()
- ->HasReceivedInitialStreamFlowControlWindowBytes());
+ ASSERT_TRUE(config->HasReceivedInitialStreamFlowControlWindowBytes());
EXPECT_EQ(kExpectedStreamIFCW,
- client_session->config()
- ->ReceivedInitialStreamFlowControlWindowBytes());
- ASSERT_TRUE(client_session->config()
- ->HasReceivedInitialSessionFlowControlWindowBytes());
+ config->ReceivedInitialStreamFlowControlWindowBytes());
+ ASSERT_TRUE(config->HasReceivedInitialSessionFlowControlWindowBytes());
EXPECT_EQ(kExpectedSessionIFCW,
- client_session->config()
- ->ReceivedInitialSessionFlowControlWindowBytes());
+ config->ReceivedInitialSessionFlowControlWindowBytes());
}
EXPECT_EQ(kExpectedStreamIFCW, QuicStreamPeer::SendWindowOffset(stream));
EXPECT_EQ(kExpectedSessionIFCW, QuicFlowControllerPeer::SendWindowOffset(
@@ -4503,8 +4512,8 @@
EXPECT_TRUE(client_->client()->WaitForOneRttKeysAvailable());
QuicSpdySession* client_session = GetClientSession();
ASSERT_TRUE(client_session);
- QuicConfig* config = client_session->config();
- ASSERT_TRUE(config);
+ std::optional<QuicConfig> config = client_->client()->negotiated_config();
+ ASSERT_TRUE(config.has_value());
EXPECT_TRUE(config->HasReceivedStatelessResetToken());
StatelessResetToken stateless_reset_token =
config->ReceivedStatelessResetToken();
@@ -4541,8 +4550,8 @@
EXPECT_TRUE(client_->client()->WaitForOneRttKeysAvailable());
QuicSpdySession* client_session = GetClientSession();
ASSERT_TRUE(client_session);
- QuicConfig* config = client_session->config();
- ASSERT_TRUE(config);
+ std::optional<QuicConfig> config = client_->client()->negotiated_config();
+ ASSERT_TRUE(config.has_value());
EXPECT_TRUE(config->HasReceivedStatelessResetToken());
StatelessResetToken stateless_reset_token =
config->ReceivedStatelessResetToken();
@@ -5390,9 +5399,8 @@
ASSERT_TRUE(Initialize());
EXPECT_TRUE(client_->client()->WaitForOneRttKeysAvailable());
QuicSpdyClientSession* client_session = GetClientSession();
- ASSERT_TRUE(client_session);
- QuicConfig* config = client_session->config();
- ASSERT_TRUE(config);
+ std::optional<QuicConfig> config = client_->client()->negotiated_config();
+ ASSERT_TRUE(config.has_value());
EXPECT_TRUE(config->HasReceivedStatelessResetToken());
QuicConnection* client_connection = client_session->connection();
ASSERT_TRUE(client_connection);
@@ -6887,24 +6895,15 @@
EXPECT_TRUE(client_->client()->WaitForOneRttKeysAvailable());
- server_thread_->Pause();
- QuicSpdySession* server_session = GetServerSession();
- QuicConfig* server_config = nullptr;
- if (server_session != nullptr) {
- server_config = server_session->config();
+ QuicConfig server_config =
+ PauseServerAndGetLastNegotiatedConfigFromDispatcher();
+
+ if (auto it = server_config.received_custom_transport_parameters().find(
+ kCustomParameter);
+ it != server_config.received_custom_transport_parameters().end()) {
+ EXPECT_EQ(it->second, "test");
} else {
- ADD_FAILURE() << "Missing server session";
- }
- if (server_config != nullptr) {
- if (auto it = server_config->received_custom_transport_parameters().find(
- kCustomParameter);
- it != server_config->received_custom_transport_parameters().end()) {
- EXPECT_EQ(it->second, "test");
- } else {
- ADD_FAILURE() << "Did not find custom parameter";
- }
- } else {
- ADD_FAILURE() << "Missing server config";
+ ADD_FAILURE() << "Did not find custom parameter";
}
server_thread_->Resume();
}
@@ -6920,14 +6919,10 @@
EXPECT_TRUE(client_->client()->WaitForOneRttKeysAvailable());
- server_thread_->Pause();
- QuicSpdySession* server_session = GetServerSession();
- QuicConfig* server_config = nullptr;
- ASSERT_TRUE(server_session);
- server_config = server_session->config();
- ASSERT_TRUE(server_config);
+ QuicConfig server_config =
+ PauseServerAndGetLastNegotiatedConfigFromDispatcher();
const std::optional<std::string>& received_sni =
- server_config->GetReceivedDebuggingSni();
+ server_config.GetReceivedDebuggingSni();
EXPECT_THAT(received_sni, testing::Optional(debugging_sni));
server_thread_->Resume();
}
@@ -6942,14 +6937,10 @@
EXPECT_TRUE(client_->client()->WaitForOneRttKeysAvailable());
- server_thread_->Pause();
- QuicSpdySession* server_session = GetServerSession();
- QuicConfig* server_config = nullptr;
- ASSERT_TRUE(server_session);
- server_config = server_session->config();
- ASSERT_TRUE(server_config);
+ QuicConfig server_config =
+ PauseServerAndGetLastNegotiatedConfigFromDispatcher();
const std::optional<std::string>& received_sni =
- server_config->GetReceivedDebuggingSni();
+ server_config.GetReceivedDebuggingSni();
ASSERT_FALSE(received_sni.has_value());
server_thread_->Resume();
}
@@ -8749,12 +8740,12 @@
client_extra_copts_.push_back(kPRGC);
ASSERT_TRUE(Initialize());
EXPECT_TRUE(client_->client()->WaitForHandshakeConfirmed());
- server_thread_->Pause();
- QuicSession* session = GetServerSession();
+ QuicConfig server_config =
+ PauseServerAndGetLastNegotiatedConfigFromDispatcher();
// Check the server received the copt.
- ASSERT_TRUE(session->config()->HasReceivedConnectionOptions());
+ ASSERT_TRUE(server_config.HasReceivedConnectionOptions());
bool found_prgc = false;
- for (auto it : session->config()->ReceivedConnectionOptions()) {
+ for (auto it : server_config.ReceivedConnectionOptions()) {
if (it == kPRGC) {
found_prgc = true;
break;
@@ -8770,12 +8761,12 @@
client_extra_copts_.push_back(kCQBC);
ASSERT_TRUE(Initialize());
EXPECT_TRUE(client_->client()->WaitForHandshakeConfirmed());
- server_thread_->Pause();
- QuicSession* session = GetServerSession();
+ QuicConfig server_config =
+ PauseServerAndGetLastNegotiatedConfigFromDispatcher();
// Check the server received the copt.
- ASSERT_TRUE(session->config()->HasReceivedConnectionOptions());
+ ASSERT_TRUE(server_config.HasReceivedConnectionOptions());
bool found_cqbc = false;
- for (auto it : session->config()->ReceivedConnectionOptions()) {
+ for (auto it : server_config.ReceivedConnectionOptions()) {
if (it == kCQBC) {
found_cqbc = true;
break;
diff --git a/quiche/quic/core/http/quic_server_session_base_test.cc b/quiche/quic/core/http/quic_server_session_base_test.cc
index cd24599..c6bbd81 100644
--- a/quiche/quic/core/http/quic_server_session_base_test.cc
+++ b/quiche/quic/core/http/quic_server_session_base_test.cc
@@ -160,6 +160,7 @@
session_->Initialize();
QuicConfigPeer::SetReceivedInitialSessionFlowControlWindow(
session_->config(), kMinimumFlowControlSendWindow);
+ EXPECT_CALL(owner_, OnConfigNegotiated(_));
session_->OnConfigNegotiated();
if (version().IsIetfQuic()) {
QuicConnectionPeer::SetAddressValidated(connection_);
@@ -214,6 +215,7 @@
QuicMemoryCacheBackend memory_cache_backend_;
std::unique_ptr<TestServerSession> session_;
std::unique_ptr<CryptoHandshakeMessage> handshake_message_;
+ std::optional<QuicConfig> negotiated_config_;
};
// Compares CachedNetworkParameters.
@@ -355,7 +357,6 @@
// more than the negotiated stream limit to deal with rare cases where a
// client FIN/RST is lost.
connection_->SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE);
- session_->OnConfigNegotiated();
if (!VersionIsIetfQuic(transport_version())) {
// The slightly increased stream limit is set during config negotiation. It
// is either an increase of 10 over negotiated limit, or a fixed percentage
@@ -413,6 +414,7 @@
// streams available. The server accepts slightly more than the negotiated
// stream limit to deal with rare cases where a client FIN/RST is lost.
connection_->SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE);
+ EXPECT_CALL(owner_, OnConfigNegotiated(_));
session_->OnConfigNegotiated();
const size_t kAvailableStreamLimit =
session_->MaxAvailableBidirectionalStreams();
@@ -517,6 +519,7 @@
copt.push_back(kBWID);
QuicConfigPeer::SetReceivedConnectionOptions(session_->config(), copt);
connection_->SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE);
+ EXPECT_CALL(owner_, OnConfigNegotiated(_));
session_->OnConfigNegotiated();
EXPECT_TRUE(
QuicServerSessionBasePeer::IsBandwidthResumptionEnabled(session_.get()));
@@ -670,6 +673,7 @@
// No effect if no CachedNetworkParameters provided.
EXPECT_CALL(*connection_, ResumeConnectionState(_, _)).Times(0);
+ EXPECT_CALL(owner_, OnConfigNegotiated(_));
session_->OnConfigNegotiated();
// No effect if CachedNetworkParameters provided, but different serving
@@ -679,6 +683,7 @@
cached_network_params.set_serving_region("different serving region");
crypto_stream->SetPreviousCachedNetworkParams(cached_network_params);
EXPECT_CALL(*connection_, ResumeConnectionState(_, _)).Times(0);
+ EXPECT_CALL(owner_, OnConfigNegotiated(_));
session_->OnConfigNegotiated();
// Same serving region, but timestamp is too old, should have no effect.
@@ -686,6 +691,7 @@
cached_network_params.set_timestamp(0);
crypto_stream->SetPreviousCachedNetworkParams(cached_network_params);
EXPECT_CALL(*connection_, ResumeConnectionState(_, _)).Times(0);
+ EXPECT_CALL(owner_, OnConfigNegotiated(_));
session_->OnConfigNegotiated();
// Same serving region, and timestamp is recent: estimate is stored.
@@ -693,6 +699,7 @@
connection_->clock()->WallNow().ToUNIXSeconds());
crypto_stream->SetPreviousCachedNetworkParams(cached_network_params);
EXPECT_CALL(*connection_, ResumeConnectionState(_, _)).Times(1);
+ EXPECT_CALL(owner_, OnConfigNegotiated(_));
session_->OnConfigNegotiated();
}
@@ -705,6 +712,7 @@
copt.push_back(kBWMX);
QuicConfigPeer::SetReceivedConnectionOptions(session_->config(), copt);
connection_->SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE);
+ EXPECT_CALL(owner_, OnConfigNegotiated(_));
session_->OnConfigNegotiated();
EXPECT_TRUE(
QuicServerSessionBasePeer::IsBandwidthResumptionEnabled(session_.get()));
@@ -714,6 +722,7 @@
EXPECT_FALSE(
QuicServerSessionBasePeer::IsBandwidthResumptionEnabled(session_.get()));
connection_->SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE);
+ EXPECT_CALL(owner_, OnConfigNegotiated(_));
session_->OnConfigNegotiated();
EXPECT_FALSE(
QuicServerSessionBasePeer::IsBandwidthResumptionEnabled(session_.get()));
@@ -730,6 +739,7 @@
EXPECT_CALL(*crypto_stream, encryption_established())
.WillRepeatedly(testing::Return(true));
connection_->SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE);
+ EXPECT_CALL(owner_, OnConfigNegotiated(_));
session_->OnConfigNegotiated();
size_t i = 0u;
diff --git a/quiche/quic/core/quic_crypto_client_stream_test.cc b/quiche/quic/core/quic_crypto_client_stream_test.cc
index 9810bb7..cf2c259 100644
--- a/quiche/quic/core/quic_crypto_client_stream_test.cc
+++ b/quiche/quic/core/quic_crypto_client_stream_test.cc
@@ -135,9 +135,13 @@
}
TEST_F(QuicCryptoClientStreamTest, NegotiatedParameters) {
+ std::optional<QuicConfig> config;
+ EXPECT_CALL(*session_, OnConfigNegotiated()).WillOnce([&] {
+ config.emplace(*session_->config());
+ });
CompleteCryptoHandshake();
- const QuicConfig* config = session_->config();
+ ASSERT_TRUE(config.has_value());
EXPECT_EQ(kMaximumIdleTimeoutSecs, config->IdleNetworkTimeout().ToSeconds());
const QuicCryptoNegotiatedParameters& crypto_params(
diff --git a/quiche/quic/core/quic_dispatcher.h b/quiche/quic/core/quic_dispatcher.h
index b4ce3d6..960300c 100644
--- a/quiche/quic/core/quic_dispatcher.h
+++ b/quiche/quic/core/quic_dispatcher.h
@@ -162,6 +162,7 @@
const QuicSocketAddress& peer_address, ParsedQuicVersion version,
const ParsedClientHello* parsed_chlo) override;
void OnPathDegrading() override {}
+ void OnConfigNegotiated(const QuicConfig&) override {}
// Create connections for previously buffered CHLOs as many as allowed.
virtual void ProcessBufferedChlos(size_t max_connections_to_create);
diff --git a/quiche/quic/core/quic_session.cc b/quiche/quic/core/quic_session.cc
index 156f600..72f1c9a 100644
--- a/quiche/quic/core/quic_session.cc
+++ b/quiche/quic/core/quic_session.cc
@@ -1312,6 +1312,9 @@
}
void QuicSession::OnConfigNegotiated() {
+ if (visitor_) {
+ visitor_->OnConfigNegotiated(*config());
+ }
// In versions with TLS, the configs will be set twice if 0-RTT is available.
// In the second config setting, 1-RTT keys are guaranteed to be available.
if (version().IsIetfQuic() && is_configured_ &&
diff --git a/quiche/quic/core/quic_session.h b/quiche/quic/core/quic_session.h
index 3dc9068..9faeb23 100644
--- a/quiche/quic/core/quic_session.h
+++ b/quiche/quic/core/quic_session.h
@@ -110,6 +110,9 @@
// Called when connection detected path degrading.
virtual void OnPathDegrading() = 0;
+
+ // Called when the config has been negotiated.
+ virtual void OnConfigNegotiated(const QuicConfig& config) = 0;
};
// Does not take ownership of |connection| or |visitor|.
diff --git a/quiche/quic/core/quic_time_wait_list_manager.h b/quiche/quic/core/quic_time_wait_list_manager.h
index 16cadd9..57a5e7c 100644
--- a/quiche/quic/core/quic_time_wait_list_manager.h
+++ b/quiche/quic/core/quic_time_wait_list_manager.h
@@ -95,6 +95,7 @@
class QUICHE_EXPORT Visitor : public QuicSession::Visitor {
public:
void OnPathDegrading() override {}
+ void OnConfigNegotiated(const quic::QuicConfig& confir) override {}
};
// writer - the entity that writes to the socket. (Owned by the caller)
diff --git a/quiche/quic/test_tools/mock_quic_session_visitor.h b/quiche/quic/test_tools/mock_quic_session_visitor.h
index 3ef812d..9e6b14a 100644
--- a/quiche/quic/test_tools/mock_quic_session_visitor.h
+++ b/quiche/quic/test_tools/mock_quic_session_visitor.h
@@ -35,6 +35,7 @@
(const quic::QuicConnectionId& server_connection_id), (override));
MOCK_METHOD(void, OnServerPreferredAddressAvailable,
(const QuicSocketAddress& server_preferred_address), (override));
+ MOCK_METHOD(void, OnConfigNegotiated, (const QuicConfig& config), (override));
};
class MockQuicCryptoServerStreamHelper
diff --git a/quiche/quic/test_tools/quic_test_client.h b/quiche/quic/test_tools/quic_test_client.h
index c7c1b7d..34fc991 100644
--- a/quiche/quic/test_tools/quic_test_client.h
+++ b/quiche/quic/test_tools/quic_test_client.h
@@ -136,6 +136,13 @@
// Casts the network helper to a MockableQuicClientDefaultNetworkHelper.
MockableQuicClientDefaultNetworkHelper* mockable_network_helper();
const MockableQuicClientDefaultNetworkHelper* mockable_network_helper() const;
+ void OnConfigNegotiated(const quic::QuicConfig& config) override {
+ negotiated_config_.emplace(config);
+ }
+
+ const std::optional<QuicConfig>& negotiated_config() {
+ return negotiated_config_;
+ }
private:
// Client connection ID to use, if client_connection_id_overridden_.
@@ -146,6 +153,7 @@
CachedNetworkParameters cached_network_paramaters_;
// Owned by the base class.
QuicTestMigrationHelper* migration_helper_ = nullptr;
+ std::optional<QuicConfig> negotiated_config_;
};
// A toy QUIC client used for testing.
diff --git a/quiche/quic/tools/quic_client_base.h b/quiche/quic/tools/quic_client_base.h
index 9f59c96..a38d251 100644
--- a/quiche/quic/tools/quic_client_base.h
+++ b/quiche/quic/tools/quic_client_base.h
@@ -114,6 +114,7 @@
void OnServerPreferredAddressAvailable(
const QuicSocketAddress& server_preferred_address) override;
void OnPathDegrading() override;
+ void OnConfigNegotiated(const QuicConfig&) override {}
// Initializes the client to create a connection. Should be called exactly
// once before calling StartConnect or Connect. Returns true if the
diff --git a/quiche/quic/tools/quic_simple_dispatcher.h b/quiche/quic/tools/quic_simple_dispatcher.h
index 884fdf5..1a6e58b 100644
--- a/quiche/quic/tools/quic_simple_dispatcher.h
+++ b/quiche/quic/tools/quic_simple_dispatcher.h
@@ -33,6 +33,14 @@
void OnRstStreamReceived(const QuicRstStreamFrame& frame) override;
+ void OnConfigNegotiated(const QuicConfig& config) override {
+ last_negotiated_config_.emplace(config);
+ }
+
+ const std::optional<QuicConfig>& last_negotiated_config() {
+ return last_negotiated_config_;
+ }
+
protected:
std::unique_ptr<QuicSession> CreateQuicSession(
QuicConnectionId connection_id, const QuicSocketAddress& self_address,
@@ -49,6 +57,8 @@
// The map of the reset error code with its counter.
std::map<QuicRstStreamErrorCode, int> rst_error_map_;
+
+ std::optional<QuicConfig> last_negotiated_config_;
};
} // namespace quic
diff --git a/quiche/quic/tools/quic_simple_server_session_test.cc b/quiche/quic/tools/quic_simple_server_session_test.cc
index 6ab4938..351af82 100644
--- a/quiche/quic/tools/quic_simple_server_session_test.cc
+++ b/quiche/quic/tools/quic_simple_server_session_test.cc
@@ -209,6 +209,7 @@
EXPECT_CALL(*session_, WriteControlFrame(_, _))
.WillRepeatedly(&ClearControlFrameWithTransmissionType);
}
+ EXPECT_CALL(owner_, OnConfigNegotiated(_));
session_->OnConfigNegotiated();
}
diff --git a/quiche/quic/tools/quic_simple_server_stream_test.cc b/quiche/quic/tools/quic_simple_server_stream_test.cc
index 44b8e3d..e895565 100644
--- a/quiche/quic/tools/quic_simple_server_stream_test.cc
+++ b/quiche/quic/tools/quic_simple_server_stream_test.cc
@@ -262,6 +262,7 @@
QuicConfigPeer::SetReceivedInitialMaxStreamDataBytesOutgoingBidirectional(
session_.config(), kMinimumFlowControlSendWindow);
QuicConfigPeer::SetReceivedMaxUnidirectionalStreams(session_.config(), 10);
+ EXPECT_CALL(session_owner_, OnConfigNegotiated(_));
session_.OnConfigNegotiated();
simulator_.RunFor(QuicTime::Delta::FromSeconds(1));
}
