QuicDispatcherTest uses MockConnectionIdGenerator instead of assuming connection IDs.
GfeQuicDispatcherTest to come
PiperOrigin-RevId: 474605289
diff --git a/quiche/quic/core/quic_dispatcher.cc b/quiche/quic/core/quic_dispatcher.cc
index 4cdb014..1ea2783 100644
--- a/quiche/quic/core/quic_dispatcher.cc
+++ b/quiche/quic/core/quic_dispatcher.cc
@@ -451,7 +451,11 @@
absl::optional<QuicConnectionId> QuicDispatcher::MaybeReplaceServerConnectionId(
const QuicConnectionId& server_connection_id,
const ParsedQuicVersion& version) {
- if (GetQuicRestartFlag(quic_abstract_connection_id_generator)) {
+ if (GetQuicRestartFlag(quic_abstract_connection_id_generator) &&
+ GetQuicRestartFlag(quic_map_original_connection_ids2)) {
+ // If the Dispatcher doesn't map the original connection ID, then using a
+ // connection ID generator that isn't deterministic may break the handshake
+ // and will certainly drop all 0-RTT packets.
QUIC_RESTART_FLAG_COUNT(quic_abstract_connection_id_generator);
return connection_id_generator_.MaybeReplaceConnectionId(
server_connection_id, version);
diff --git a/quiche/quic/core/quic_dispatcher_test.cc b/quiche/quic/core/quic_dispatcher_test.cc
index ef3bfbd..74eb760 100644
--- a/quiche/quic/core/quic_dispatcher_test.cc
+++ b/quiche/quic/core/quic_dispatcher_test.cc
@@ -16,7 +16,6 @@
#include "quiche/quic/core/crypto/crypto_protocol.h"
#include "quiche/quic/core/crypto/quic_crypto_server_config.h"
#include "quiche/quic/core/crypto/quic_random.h"
-#include "quiche/quic/core/deterministic_connection_id_generator.h"
#include "quiche/quic/core/frames/quic_new_connection_id_frame.h"
#include "quiche/quic/core/quic_config.h"
#include "quiche/quic/core/quic_connection.h"
@@ -226,7 +225,6 @@
QuicRandom::GetInstance(), std::move(proof_source),
KeyExchangeSource::Default()),
server_address_(QuicIpAddress::Any4(), 5),
- connection_id_generator_(kQuicDefaultConnectionIdLength),
dispatcher_(new NiceMock<TestDispatcher>(
&config_, &crypto_config_, &version_manager_,
mock_helper_.GetRandomGenerator(), connection_id_generator_)),
@@ -432,6 +430,16 @@
const QuicConnectionId& server_connection_id,
const QuicConnectionId& client_connection_id,
std::unique_ptr<QuicCryptoClientConfig> client_crypto_config) {
+ if (expect_generator_is_called_ &&
+ GetQuicRestartFlag(quic_map_original_connection_ids2) &&
+ GetQuicRestartFlag(quic_abstract_connection_id_generator)) {
+ // Can't replace the connection ID in early gQUIC versions!
+ ASSERT_TRUE(version.AllowsVariableLengthConnectionIds() ||
+ generated_connection_id_ == absl::nullopt);
+ EXPECT_CALL(connection_id_generator_,
+ MaybeReplaceConnectionId(server_connection_id, version))
+ .WillOnce(Return(generated_connection_id_));
+ }
std::vector<std::unique_ptr<QuicReceivedPacket>> packets =
GetFirstFlightOfPackets(version, DefaultQuicConfig(),
server_connection_id, client_connection_id,
@@ -498,6 +506,7 @@
EXPECT_CALL(*dispatcher_,
CreateQuicSession(connection_id, _, client_address, _, _, _))
.Times(0);
+ expect_generator_is_called_ = false;
ProcessFirstFlight(version, client_address, connection_id);
}
@@ -520,7 +529,12 @@
QuicVersionManager version_manager_;
QuicCryptoServerConfig crypto_config_;
QuicSocketAddress server_address_;
- DeterministicConnectionIdGenerator connection_id_generator_;
+ bool expect_generator_is_called_ = true;
+ absl::optional<QuicConnectionId> generated_connection_id_;
+ // Constant to set generated_connection_id to when needed.
+ QuicConnectionId return_connection_id_{
+ {0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07}};
+ MockConnectionIdGenerator connection_id_generator_;
std::unique_ptr<NiceMock<TestDispatcher>> dispatcher_;
MockTimeWaitListManager* time_wait_list_manager_;
TestQuicSpdyServerSession* session1_;
@@ -579,13 +593,28 @@
QuicSocketAddress client_address(QuicIpAddress::Loopback4(), 1);
QuicConnectionId original_connection_id, new_connection_id;
if (long_connection_id) {
- original_connection_id = QuicConnectionId(
- {0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09});
- new_connection_id =
- QuicConnectionId({0x6c, 0x6b, 0x4b, 0xad, 0x8d, 0x00, 0x24, 0xd8});
+ if (GetQuicRestartFlag(quic_map_original_connection_ids2) &&
+ GetQuicRestartFlag(quic_abstract_connection_id_generator)) {
+ original_connection_id = TestConnectionIdNineBytesLong(1);
+ new_connection_id = return_connection_id_;
+ EXPECT_CALL(connection_id_generator_,
+ MaybeReplaceConnectionId(original_connection_id, version_))
+ .WillOnce(Return(new_connection_id));
+ } else {
+ original_connection_id = QuicConnectionId(
+ {0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09});
+ new_connection_id =
+ QuicConnectionId({0x6c, 0x6b, 0x4b, 0xad, 0x8d, 0x00, 0x24, 0xd8});
+ }
} else {
original_connection_id = TestConnectionId();
new_connection_id = original_connection_id;
+ if (GetQuicRestartFlag(quic_map_original_connection_ids2) &&
+ GetQuicRestartFlag(quic_abstract_connection_id_generator)) {
+ EXPECT_CALL(connection_id_generator_,
+ MaybeReplaceConnectionId(original_connection_id, version_))
+ .WillOnce(Return(absl::nullopt));
+ }
}
QuicConfig client_config = DefaultQuicConfig();
// Add a 2000-byte custom parameter to increase the length of the CHLO.
@@ -793,6 +822,7 @@
*time_wait_list_manager_,
SendVersionNegotiationPacket(TestConnectionId(1), _, _, _, _, _, _, _))
.Times(1);
+ expect_generator_is_called_ = false;
ProcessFirstFlight(QuicVersionReservedForNegotiation(), client_address,
TestConnectionId(1));
}
@@ -807,6 +837,7 @@
EXPECT_CALL(*time_wait_list_manager_,
SendVersionNegotiationPacket(connection_id, _, _, _, _, _, _, _))
.Times(1);
+ expect_generator_is_called_ = false;
ProcessFirstFlight(QuicVersionReservedForNegotiation(), client_address,
connection_id);
}
@@ -821,6 +852,7 @@
SendVersionNegotiationPacket(
TestConnectionId(1), TestConnectionId(2), _, _, _, _, _, _))
.Times(1);
+ expect_generator_is_called_ = false;
ProcessFirstFlight(QuicVersionReservedForNegotiation(), client_address,
TestConnectionId(1), TestConnectionId(2));
}
@@ -1063,79 +1095,6 @@
"data");
}
-// TODO(b/243181134): re-enable those tests once they compile in Chromium.
-TEST_P(QuicDispatcherTestAllVersions, UsesConnectionIdGenerator) {
- SetQuicRestartFlag(quic_abstract_connection_id_generator, true);
- // Avoid multiple calls to MaybeReplaceConnectionId()
- SetQuicRestartFlag(quic_map_original_connection_ids2, true);
- // Create a new dispatcher with the Mock generator.
- QuicSocketAddress client_address(QuicIpAddress::Loopback4(), 1);
- MockConnectionIdGenerator generator;
- dispatcher_.reset();
- dispatcher_ = std::make_unique<NiceMock<TestDispatcher>>(
- &config_, &crypto_config_, &version_manager_,
- mock_helper_.GetRandomGenerator(), generator);
- SetUp();
- // Generator does not change connection ID.
- EXPECT_CALL(generator, MaybeReplaceConnectionId(TestConnectionId(1), _))
- .WillOnce(Return(absl::optional<QuicConnectionId>()));
- EXPECT_CALL(*dispatcher_,
- CreateQuicSession(TestConnectionId(1), _, client_address,
- Eq(ExpectedAlpn()), _, _))
- .WillOnce(Return(ByMove(CreateSession(
- dispatcher_.get(), config_, TestConnectionId(1), client_address,
- &mock_helper_, &mock_alarm_factory_, &crypto_config_,
- QuicDispatcherPeer::GetCache(dispatcher_.get()), &session1_))));
- ProcessFirstFlight(client_address, TestConnectionId(1));
- // Generator changes connection ID.
- QuicConnectionId new_connection_id(
- {0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08});
- EXPECT_CALL(generator, MaybeReplaceConnectionId(TestConnectionId(2), _))
- .WillOnce(Return(new_connection_id));
- EXPECT_CALL(*dispatcher_,
- CreateQuicSession(new_connection_id, _, client_address,
- Eq(ExpectedAlpn()), _, _))
- .WillOnce(Return(ByMove(CreateSession(
- dispatcher_.get(), config_, new_connection_id, client_address,
- &mock_helper_, &mock_alarm_factory_, &crypto_config_,
- QuicDispatcherPeer::GetCache(dispatcher_.get()), &session1_))));
- ProcessFirstFlight(client_address, TestConnectionId(2));
-}
-
-TEST_P(QuicDispatcherTestAllVersions, DoesNotUseConnectionIdGenerator) {
- SetQuicRestartFlag(quic_abstract_connection_id_generator, false);
- // Avoid multiple calls to MaybeReplaceConnectionId()
- SetQuicRestartFlag(quic_map_original_connection_ids2, true);
- // Create a new dispatcher with the Mock generator.
- QuicSocketAddress client_address(QuicIpAddress::Loopback4(), 1);
- MockConnectionIdGenerator generator;
- dispatcher_.reset();
- dispatcher_ = std::make_unique<NiceMock<TestDispatcher>>(
- &config_, &crypto_config_, &version_manager_,
- mock_helper_.GetRandomGenerator(), generator);
- SetUp();
- EXPECT_CALL(generator, MaybeReplaceConnectionId(TestConnectionId(1), _))
- .Times(0);
- EXPECT_CALL(*dispatcher_,
- CreateQuicSession(TestConnectionId(1), _, client_address,
- Eq(ExpectedAlpn()), _, _))
- .WillOnce(Return(ByMove(CreateSession(
- dispatcher_.get(), config_, TestConnectionId(1), client_address,
- &mock_helper_, &mock_alarm_factory_, &crypto_config_,
- QuicDispatcherPeer::GetCache(dispatcher_.get()), &session1_))));
- ProcessFirstFlight(client_address, TestConnectionId(1));
- EXPECT_CALL(generator, MaybeReplaceConnectionId(TestConnectionId(2), _))
- .Times(0);
- EXPECT_CALL(*dispatcher_,
- CreateQuicSession(TestConnectionId(2), _, client_address,
- Eq(ExpectedAlpn()), _, _))
- .WillOnce(Return(ByMove(CreateSession(
- dispatcher_.get(), config_, TestConnectionId(2), client_address,
- &mock_helper_, &mock_alarm_factory_, &crypto_config_,
- QuicDispatcherPeer::GetCache(dispatcher_.get()), &session1_))));
- ProcessFirstFlight(client_address, TestConnectionId(2));
-}
-
// Makes sure nine-byte connection IDs are replaced by 8-byte ones.
TEST_P(QuicDispatcherTestAllVersions, LongConnectionIdLengthReplaced) {
if (!version_.AllowsVariableLengthConnectionIds()) {
@@ -1146,14 +1105,17 @@
QuicSocketAddress client_address(QuicIpAddress::Loopback4(), 1);
QuicConnectionId bad_connection_id = TestConnectionIdNineBytesLong(2);
- QuicConnectionId fixed_connection_id =
- QuicUtils::CreateReplacementConnectionId(bad_connection_id);
+ generated_connection_id_ =
+ (GetQuicRestartFlag(quic_abstract_connection_id_generator) &&
+ GetQuicRestartFlag(quic_map_original_connection_ids2))
+ ? return_connection_id_
+ : QuicUtils::CreateReplacementConnectionId(bad_connection_id);
EXPECT_CALL(*dispatcher_,
- CreateQuicSession(fixed_connection_id, _, client_address,
+ CreateQuicSession(*generated_connection_id_, _, client_address,
Eq(ExpectedAlpn()), _, _))
.WillOnce(Return(ByMove(CreateSession(
- dispatcher_.get(), config_, fixed_connection_id, client_address,
+ dispatcher_.get(), config_, *generated_connection_id_, client_address,
&mock_helper_, &mock_alarm_factory_, &crypto_config_,
QuicDispatcherPeer::GetCache(dispatcher_.get()), &session1_))));
EXPECT_CALL(*reinterpret_cast<MockQuicConnection*>(session1_->connection()),
@@ -1175,8 +1137,11 @@
QuicSocketAddress client_address(QuicIpAddress::Loopback4(), 1);
QuicConnectionId bad_connection_id = EmptyQuicConnectionId();
- QuicConnectionId fixed_connection_id =
- QuicUtils::CreateReplacementConnectionId(bad_connection_id);
+ generated_connection_id_ =
+ (GetQuicRestartFlag(quic_abstract_connection_id_generator) &&
+ GetQuicRestartFlag(quic_map_original_connection_ids2))
+ ? return_connection_id_
+ : QuicUtils::CreateReplacementConnectionId(bad_connection_id);
// Disable validation of invalid short connection IDs.
dispatcher_->SetAllowShortInitialServerConnectionIds(true);
@@ -1184,10 +1149,10 @@
// validation is still enabled.
EXPECT_CALL(*dispatcher_,
- CreateQuicSession(fixed_connection_id, _, client_address,
+ CreateQuicSession(*generated_connection_id_, _, client_address,
Eq(ExpectedAlpn()), _, _))
.WillOnce(Return(ByMove(CreateSession(
- dispatcher_.get(), config_, fixed_connection_id, client_address,
+ dispatcher_.get(), config_, *generated_connection_id_, client_address,
&mock_helper_, &mock_alarm_factory_, &crypto_config_,
QuicDispatcherPeer::GetCache(dispatcher_.get()), &session1_))));
EXPECT_CALL(*reinterpret_cast<MockQuicConnection*>(session1_->connection()),
@@ -1208,8 +1173,6 @@
QuicSocketAddress client_address(QuicIpAddress::Loopback4(), 1);
QuicConnectionId bad_connection_id = TestConnectionIdNineBytesLong(2);
- QuicConnectionId fixed_connection_id =
- QuicUtils::CreateReplacementConnectionId(bad_connection_id);
EXPECT_CALL(*dispatcher_,
CreateQuicSession(TestConnectionId(1), _, client_address,
@@ -1225,11 +1188,16 @@
})));
ProcessFirstFlight(client_address, TestConnectionId(1));
+ generated_connection_id_ =
+ (GetQuicRestartFlag(quic_abstract_connection_id_generator) &&
+ GetQuicRestartFlag(quic_map_original_connection_ids2))
+ ? return_connection_id_
+ : QuicUtils::CreateReplacementConnectionId(bad_connection_id);
EXPECT_CALL(*dispatcher_,
- CreateQuicSession(fixed_connection_id, _, client_address,
+ CreateQuicSession(*generated_connection_id_, _, client_address,
Eq(ExpectedAlpn()), _, _))
.WillOnce(Return(ByMove(CreateSession(
- dispatcher_.get(), config_, fixed_connection_id, client_address,
+ dispatcher_.get(), config_, *generated_connection_id_, client_address,
&mock_helper_, &mock_alarm_factory_, &crypto_config_,
QuicDispatcherPeer::GetCache(dispatcher_.get()), &session2_))));
EXPECT_CALL(*reinterpret_cast<MockQuicConnection*>(session2_->connection()),
@@ -1317,6 +1285,7 @@
.Times(0);
EXPECT_CALL(*time_wait_list_manager_, AddConnectionIdToTimeWait(_, _))
.Times(0);
+ expect_generator_is_called_ = false;
ProcessFirstFlight(client_address, EmptyQuicConnectionId());
}
@@ -1359,6 +1328,7 @@
/*ietf_quic=*/true,
/*use_length_prefix=*/true, _, _, client_address, _))
.Times(1);
+ expect_generator_is_called_ = false;
ProcessFirstFlight(ParsedQuicVersion::ReservedForNegotiation(),
client_address, server_connection_id,
client_connection_id);
@@ -1851,6 +1821,7 @@
CreateQuicSession(TestConnectionId(2), _, client_address,
Eq(ExpectedAlpn()), _, _))
.Times(0u);
+ expect_generator_is_called_ = false;
ProcessFirstFlight(client_address, TestConnectionId(2));
// Existing connections should be able to continue.
@@ -1872,6 +1843,7 @@
CreateQuicSession(TestConnectionId(2), _, client_address,
Eq(ExpectedAlpn()), _, _))
.Times(0u);
+ expect_generator_is_called_ = false;
ProcessFirstFlight(client_address, TestConnectionId(2));
dispatcher_->StartAcceptingNewConnections();
@@ -2475,6 +2447,12 @@
// When CHLO arrives, a new session should be created, and all packets
// buffered should be delivered to the session.
+ if (GetQuicRestartFlag(quic_map_original_connection_ids2) &&
+ GetQuicRestartFlag(quic_abstract_connection_id_generator)) {
+ EXPECT_CALL(connection_id_generator_,
+ MaybeReplaceConnectionId(conn_id, version_))
+ .WillOnce(Return(absl::nullopt));
+ }
EXPECT_CALL(*dispatcher_,
CreateQuicSession(conn_id, _, client_addr_, Eq(ExpectedAlpn()), _,
Eq(ParsedClientHelloForTest())))
@@ -2491,6 +2469,7 @@
ValidatePacket(conn_id, packet);
}
})));
+ expect_generator_is_called_ = false;
ProcessFirstFlight(conn_id);
}
@@ -2507,6 +2486,12 @@
data_connection_map_[conn_id].pop_back();
// When CHLO arrives, a new session should be created, and all packets
// buffered should be delivered to the session.
+ if (GetQuicRestartFlag(quic_map_original_connection_ids2) &&
+ GetQuicRestartFlag(quic_abstract_connection_id_generator)) {
+ EXPECT_CALL(connection_id_generator_,
+ MaybeReplaceConnectionId(conn_id, version_))
+ .WillOnce(Return(absl::nullopt));
+ }
EXPECT_CALL(*dispatcher_, CreateQuicSession(conn_id, _, client_addr_,
Eq(ExpectedAlpn()), _, _))
.WillOnce(Return(ByMove(CreateSession(
@@ -2525,6 +2510,7 @@
ValidatePacket(conn_id, packet);
}
})));
+ expect_generator_is_called_ = false;
ProcessFirstFlight(conn_id);
}
@@ -2551,6 +2537,12 @@
for (size_t i = 1; i <= kNumConnections; ++i) {
QuicSocketAddress client_address(QuicIpAddress::Loopback4(), 20000 + i);
QuicConnectionId conn_id = TestConnectionId(i);
+ if (GetQuicRestartFlag(quic_map_original_connection_ids2) &&
+ GetQuicRestartFlag(quic_abstract_connection_id_generator)) {
+ EXPECT_CALL(connection_id_generator_,
+ MaybeReplaceConnectionId(conn_id, version_))
+ .WillOnce(Return(absl::nullopt));
+ }
EXPECT_CALL(*dispatcher_, CreateQuicSession(conn_id, _, client_address,
Eq(ExpectedAlpn()), _, _))
.WillOnce(Return(ByMove(CreateSession(
@@ -2569,7 +2561,7 @@
ValidatePacket(conn_id, packet);
}
})));
-
+ expect_generator_is_called_ = false;
ProcessFirstFlight(client_address, conn_id);
}
}
@@ -2597,6 +2589,12 @@
// When CHLO arrives, a new session should be created, and all packets
// buffered should be delivered to the session.
+ if (GetQuicRestartFlag(quic_map_original_connection_ids2) &&
+ GetQuicRestartFlag(quic_abstract_connection_id_generator)) {
+ EXPECT_CALL(connection_id_generator_,
+ MaybeReplaceConnectionId(conn_id, version_))
+ .WillOnce(Return(absl::nullopt));
+ }
EXPECT_CALL(*dispatcher_, CreateQuicSession(conn_id, _, client_addr_,
Eq(ExpectedAlpn()), _, _))
.Times(1) // Only triggered by 1st CHLO.
@@ -2646,6 +2644,7 @@
// list.
ASSERT_TRUE(time_wait_list_manager_->IsConnectionIdInTimeWait(conn_id));
EXPECT_CALL(*time_wait_list_manager_, ProcessPacket(_, _, conn_id, _, _, _));
+ expect_generator_is_called_ = false;
ProcessFirstFlight(conn_id);
}
@@ -2661,6 +2660,13 @@
kMaxNumSessionsToCreate + kDefaultMaxConnectionsInStore + 1;
for (uint64_t conn_id = 1; conn_id <= kNumCHLOs; ++conn_id) {
if (conn_id <= kMaxNumSessionsToCreate) {
+ if (GetQuicRestartFlag(quic_map_original_connection_ids2) &&
+ GetQuicRestartFlag(quic_abstract_connection_id_generator)) {
+ EXPECT_CALL(
+ connection_id_generator_,
+ MaybeReplaceConnectionId(TestConnectionId(conn_id), version_))
+ .WillOnce(Return(absl::nullopt));
+ }
EXPECT_CALL(*dispatcher_,
CreateQuicSession(TestConnectionId(conn_id), _, client_addr_,
Eq(ExpectedAlpn()), _,
@@ -2680,6 +2686,7 @@
}
})));
}
+ expect_generator_is_called_ = false;
ProcessFirstFlight(TestConnectionId(conn_id));
if (conn_id <= kMaxNumSessionsToCreate + kDefaultMaxConnectionsInStore &&
conn_id > kMaxNumSessionsToCreate) {
@@ -2697,6 +2704,12 @@
for (uint64_t conn_id = kMaxNumSessionsToCreate + 1;
conn_id <= kMaxNumSessionsToCreate + kDefaultMaxConnectionsInStore;
++conn_id) {
+ if (GetQuicRestartFlag(quic_map_original_connection_ids2) &&
+ GetQuicRestartFlag(quic_abstract_connection_id_generator)) {
+ EXPECT_CALL(connection_id_generator_,
+ MaybeReplaceConnectionId(TestConnectionId(conn_id), version_))
+ .WillOnce(Return(absl::nullopt));
+ }
EXPECT_CALL(*dispatcher_,
CreateQuicSession(TestConnectionId(conn_id), _, client_addr_,
Eq(ExpectedAlpn()), _,
@@ -2714,6 +2727,9 @@
}
})));
}
+ EXPECT_CALL(connection_id_generator_,
+ MaybeReplaceConnectionId(TestConnectionId(kNumCHLOs), version_))
+ .Times(0);
EXPECT_CALL(*dispatcher_,
CreateQuicSession(TestConnectionId(kNumCHLOs), _, client_addr_,
Eq(ExpectedAlpn()), _, _))
@@ -2757,6 +2773,7 @@
// Retransmit CHLO on last connection should be dropped.
QuicConnectionId last_connection =
TestConnectionId(kMaxNumSessionsToCreate + 1);
+ expect_generator_is_called_ = false;
ProcessFirstFlight(last_connection);
size_t packets_buffered = 2;
@@ -2869,6 +2886,8 @@
ValidatePacket(TestConnectionId(conn_id), packet);
}
})));
+ } else {
+ expect_generator_is_called_ = false;
}
ProcessFirstFlight(TestConnectionId(conn_id));
}
