Do not issue a new connection ID if it has been taken by another connection in the session map.
Protected by FLAGS_quic_reloadable_flag_quic_check_cid_collision_when_issue_new_cid.
PiperOrigin-RevId: 456851394
diff --git a/quiche/quic/core/quic_connection.cc b/quiche/quic/core/quic_connection.cc
index ad7a103..f5bf7be 100644
--- a/quiche/quic/core/quic_connection.cc
+++ b/quiche/quic/core/quic_connection.cc
@@ -6398,11 +6398,12 @@
return connected_;
}
-void QuicConnection::OnNewConnectionIdIssued(
+bool QuicConnection::MaybeReserveConnectionId(
const QuicConnectionId& connection_id) {
if (perspective_ == Perspective::IS_SERVER) {
- visitor_->OnServerConnectionIdIssued(connection_id);
+ return visitor_->MaybeReserveConnectionId(connection_id);
}
+ return true;
}
void QuicConnection::OnSelfIssuedConnectionIdRetired(
diff --git a/quiche/quic/core/quic_connection.h b/quiche/quic/core/quic_connection.h
index 87f9c14..db2d320 100644
--- a/quiche/quic/core/quic_connection.h
+++ b/quiche/quic/core/quic_connection.h
@@ -164,8 +164,9 @@
// Called to send a RETIRE_CONNECTION_ID frame.
virtual void SendRetireConnectionId(uint64_t sequence_number) = 0;
- // Called when server starts to use a server issued connection ID.
- virtual void OnServerConnectionIdIssued(
+ // Called when server starts to use a server issued connection ID. Returns
+ // true if this connection ID hasn't been used by another connection.
+ virtual bool MaybeReserveConnectionId(
const QuicConnectionId& server_connection_id) = 0;
// Called when server stops to use a server issued connection ID.
@@ -718,7 +719,7 @@
// QuicConnectionIdManagerVisitorInterface
void OnPeerIssuedConnectionIdRetired() override;
bool SendNewConnectionId(const QuicNewConnectionIdFrame& frame) override;
- void OnNewConnectionIdIssued(const QuicConnectionId& connection_id) override;
+ bool MaybeReserveConnectionId(const QuicConnectionId& connection_id) override;
void OnSelfIssuedConnectionIdRetired(
const QuicConnectionId& connection_id) override;
diff --git a/quiche/quic/core/quic_connection_id_manager.cc b/quiche/quic/core/quic_connection_id_manager.cc
index 46ff85f..17f7a72 100644
--- a/quiche/quic/core/quic_connection_id_manager.cc
+++ b/quiche/quic/core/quic_connection_id_manager.cc
@@ -10,6 +10,7 @@
#include "quiche/quic/core/quic_connection_id.h"
#include "quiche/quic/core/quic_error_codes.h"
#include "quiche/quic/core/quic_utils.h"
+#include "quiche/quic/platform/api/quic_flag_utils.h"
#include "quiche/common/platform/api/quiche_logging.h"
namespace quic {
@@ -298,14 +299,28 @@
return QuicUtils::CreateReplacementConnectionId(old_connection_id);
}
-QuicNewConnectionIdFrame
-QuicSelfIssuedConnectionIdManager::IssueNewConnectionId() {
+absl::optional<QuicNewConnectionIdFrame>
+QuicSelfIssuedConnectionIdManager::MaybeIssueNewConnectionId() {
+ const bool check_cid_collision_when_issue_new_cid =
+ GetQuicReloadableFlag(quic_check_cid_collision_when_issue_new_cid);
+ QuicConnectionId new_cid = GenerateNewConnectionId(last_connection_id_);
+ if (check_cid_collision_when_issue_new_cid) {
+ QUIC_RELOADABLE_FLAG_COUNT_N(quic_check_cid_collision_when_issue_new_cid, 1,
+ 2);
+ if (!visitor_->MaybeReserveConnectionId(new_cid)) {
+ QUIC_RELOADABLE_FLAG_COUNT_N(quic_check_cid_collision_when_issue_new_cid,
+ 2, 2);
+ return {};
+ }
+ }
QuicNewConnectionIdFrame frame;
- frame.connection_id = GenerateNewConnectionId(last_connection_id_);
+ frame.connection_id = new_cid;
frame.sequence_number = next_connection_id_sequence_number_++;
frame.stateless_reset_token =
QuicUtils::GenerateStatelessResetToken(frame.connection_id);
- visitor_->OnNewConnectionIdIssued(frame.connection_id);
+ if (!check_cid_collision_when_issue_new_cid) {
+ visitor_->MaybeReserveConnectionId(frame.connection_id);
+ }
active_connection_ids_.emplace_back(frame.connection_id,
frame.sequence_number);
frame.retire_prior_to = active_connection_ids_.front().second;
@@ -313,10 +328,10 @@
return frame;
}
-QuicNewConnectionIdFrame
-QuicSelfIssuedConnectionIdManager::IssueNewConnectionIdForPreferredAddress() {
- QuicNewConnectionIdFrame frame = IssueNewConnectionId();
- QUICHE_DCHECK_EQ(frame.sequence_number, 1u);
+absl::optional<QuicNewConnectionIdFrame> QuicSelfIssuedConnectionIdManager::
+ MaybeIssueNewConnectionIdForPreferredAddress() {
+ absl::optional<QuicNewConnectionIdFrame> frame = MaybeIssueNewConnectionId();
+ QUICHE_DCHECK(!frame.has_value() || (frame->sequence_number == 1u));
return frame;
}
@@ -406,8 +421,12 @@
void QuicSelfIssuedConnectionIdManager::MaybeSendNewConnectionIds() {
while (active_connection_ids_.size() < active_connection_id_limit_) {
- QuicNewConnectionIdFrame frame = IssueNewConnectionId();
- if (!visitor_->SendNewConnectionId(frame)) {
+ absl::optional<QuicNewConnectionIdFrame> frame =
+ MaybeIssueNewConnectionId();
+ if (!frame.has_value()) {
+ break;
+ }
+ if (!visitor_->SendNewConnectionId(*frame)) {
break;
}
}
diff --git a/quiche/quic/core/quic_connection_id_manager.h b/quiche/quic/core/quic_connection_id_manager.h
index 501310b..7f271e2 100644
--- a/quiche/quic/core/quic_connection_id_manager.h
+++ b/quiche/quic/core/quic_connection_id_manager.h
@@ -47,7 +47,7 @@
virtual ~QuicConnectionIdManagerVisitorInterface() = default;
virtual void OnPeerIssuedConnectionIdRetired() = 0;
virtual bool SendNewConnectionId(const QuicNewConnectionIdFrame& frame) = 0;
- virtual void OnNewConnectionIdIssued(
+ virtual bool MaybeReserveConnectionId(
const QuicConnectionId& connection_id) = 0;
virtual void OnSelfIssuedConnectionIdRetired(
const QuicConnectionId& connection_id) = 0;
@@ -130,7 +130,8 @@
virtual ~QuicSelfIssuedConnectionIdManager();
- QuicNewConnectionIdFrame IssueNewConnectionIdForPreferredAddress();
+ absl::optional<QuicNewConnectionIdFrame>
+ MaybeIssueNewConnectionIdForPreferredAddress();
QuicErrorCode OnRetireConnectionIdFrame(
const QuicRetireConnectionIdFrame& frame, QuicTime::Delta pto_delay,
@@ -164,7 +165,8 @@
private:
friend class test::QuicConnectionIdManagerPeer;
- QuicNewConnectionIdFrame IssueNewConnectionId();
+ // Issue a new connection ID. Can return nullopt.
+ absl::optional<QuicNewConnectionIdFrame> MaybeIssueNewConnectionId();
// This should be set to the min of:
// (1) # of active connection IDs that peer can maintain.
diff --git a/quiche/quic/core/quic_connection_id_manager_test.cc b/quiche/quic/core/quic_connection_id_manager_test.cc
index 1c2657d..dbd239f 100644
--- a/quiche/quic/core/quic_connection_id_manager_test.cc
+++ b/quiche/quic/core/quic_connection_id_manager_test.cc
@@ -64,8 +64,10 @@
bool SendNewConnectionId(const QuicNewConnectionIdFrame& /*frame*/) override {
return false;
}
- void OnNewConnectionIdIssued(
- const QuicConnectionId& /*connection_id*/) override {}
+ bool MaybeReserveConnectionId(const QuicConnectionId&) override {
+ return false;
+ }
+
void OnSelfIssuedConnectionIdRetired(
const QuicConnectionId& /*connection_id*/) override {}
@@ -522,7 +524,7 @@
MOCK_METHOD(bool, SendNewConnectionId,
(const QuicNewConnectionIdFrame& frame), (override));
- MOCK_METHOD(void, OnNewConnectionIdIssued,
+ MOCK_METHOD(bool, MaybeReserveConnectionId,
(const QuicConnectionId& connection_id), (override));
MOCK_METHOD(void, OnSelfIssuedConnectionIdRetired,
(const QuicConnectionId& connection_id), (override));
@@ -568,7 +570,8 @@
QuicConnectionId cid5 = cid_manager_.GenerateNewConnectionId(cid4);
// Sends CID #1 to peer.
- EXPECT_CALL(cid_manager_visitor_, OnNewConnectionIdIssued(cid1));
+ EXPECT_CALL(cid_manager_visitor_, MaybeReserveConnectionId(cid1))
+ .WillOnce(Return(true));
EXPECT_CALL(cid_manager_visitor_,
SendNewConnectionId(ExpectedNewConnectionIdFrame(cid1, 1u, 0u)))
.WillOnce(Return(true));
@@ -578,7 +581,8 @@
// Peer retires CID #0;
// Sends CID #2 and asks peer to retire CIDs prior to #1.
// Outcome: (#1, #2) are active.
- EXPECT_CALL(cid_manager_visitor_, OnNewConnectionIdIssued(cid2));
+ EXPECT_CALL(cid_manager_visitor_, MaybeReserveConnectionId(cid2))
+ .WillOnce(Return(true));
EXPECT_CALL(cid_manager_visitor_,
SendNewConnectionId(ExpectedNewConnectionIdFrame(cid2, 2u, 1u)))
.WillOnce(Return(true));
@@ -593,7 +597,8 @@
// Peer retires CID #1;
// Sends CID #3 and asks peer to retire CIDs prior to #2.
// Outcome: (#2, #3) are active.
- EXPECT_CALL(cid_manager_visitor_, OnNewConnectionIdIssued(cid3));
+ EXPECT_CALL(cid_manager_visitor_, MaybeReserveConnectionId(cid3))
+ .WillOnce(Return(true));
EXPECT_CALL(cid_manager_visitor_,
SendNewConnectionId(ExpectedNewConnectionIdFrame(cid3, 3u, 2u)))
.WillOnce(Return(true));
@@ -608,7 +613,8 @@
// Peer retires CID #2;
// Sends CID #4 and asks peer to retire CIDs prior to #3.
// Outcome: (#3, #4) are active.
- EXPECT_CALL(cid_manager_visitor_, OnNewConnectionIdIssued(cid4));
+ EXPECT_CALL(cid_manager_visitor_, MaybeReserveConnectionId(cid4))
+ .WillOnce(Return(true));
EXPECT_CALL(cid_manager_visitor_,
SendNewConnectionId(ExpectedNewConnectionIdFrame(cid4, 4u, 3u)))
.WillOnce(Return(true));
@@ -623,7 +629,8 @@
// Peer retires CID #3;
// Sends CID #5 and asks peer to retire CIDs prior to #4.
// Outcome: (#4, #5) are active.
- EXPECT_CALL(cid_manager_visitor_, OnNewConnectionIdIssued(cid5));
+ EXPECT_CALL(cid_manager_visitor_, MaybeReserveConnectionId(cid5))
+ .WillOnce(Return(true));
EXPECT_CALL(cid_manager_visitor_,
SendNewConnectionId(ExpectedNewConnectionIdFrame(cid5, 5u, 4u)))
.WillOnce(Return(true));
@@ -644,7 +651,8 @@
QuicConnectionId cid4 = cid_manager_.GenerateNewConnectionId(cid3);
// Sends CID #1 to peer.
- EXPECT_CALL(cid_manager_visitor_, OnNewConnectionIdIssued(cid1));
+ EXPECT_CALL(cid_manager_visitor_, MaybeReserveConnectionId(cid1))
+ .WillOnce(Return(true));
EXPECT_CALL(cid_manager_visitor_,
SendNewConnectionId(ExpectedNewConnectionIdFrame(cid1, 1u, 0u)))
.WillOnce(Return(true));
@@ -654,7 +662,8 @@
// Peer retires CID #1;
// Sends CID #2 and asks peer to retire CIDs prior to #0.
// Outcome: (#0, #2) are active.
- EXPECT_CALL(cid_manager_visitor_, OnNewConnectionIdIssued(cid2));
+ EXPECT_CALL(cid_manager_visitor_, MaybeReserveConnectionId(cid2))
+ .WillOnce(Return(true));
EXPECT_CALL(cid_manager_visitor_,
SendNewConnectionId(ExpectedNewConnectionIdFrame(cid2, 2u, 0u)))
.WillOnce(Return(true));
@@ -678,7 +687,8 @@
// Peer retires CID #0;
// Sends CID #3 and asks peer to retire CIDs prior to #2.
// Outcome: (#2, #3) are active.
- EXPECT_CALL(cid_manager_visitor_, OnNewConnectionIdIssued(cid3));
+ EXPECT_CALL(cid_manager_visitor_, MaybeReserveConnectionId(cid3))
+ .WillOnce(Return(true));
EXPECT_CALL(cid_manager_visitor_,
SendNewConnectionId(ExpectedNewConnectionIdFrame(cid3, 3u, 2u)))
.WillOnce(Return(true));
@@ -693,7 +703,8 @@
// Peer retires CID #3;
// Sends CID #4 and asks peer to retire CIDs prior to #2.
// Outcome: (#2, #4) are active.
- EXPECT_CALL(cid_manager_visitor_, OnNewConnectionIdIssued(cid4));
+ EXPECT_CALL(cid_manager_visitor_, MaybeReserveConnectionId(cid4))
+ .WillOnce(Return(true));
EXPECT_CALL(cid_manager_visitor_,
SendNewConnectionId(ExpectedNewConnectionIdFrame(cid4, 4u, 2u)))
.WillOnce(Return(true));
@@ -720,7 +731,9 @@
QuicConnectionId cid1 = cid_manager_.GenerateNewConnectionId(cid0);
QuicConnectionId cid2 = cid_manager_.GenerateNewConnectionId(cid1);
QuicConnectionId cid3 = cid_manager_.GenerateNewConnectionId(cid2);
- EXPECT_CALL(cid_manager_visitor_, OnNewConnectionIdIssued(_)).Times(3);
+ EXPECT_CALL(cid_manager_visitor_, MaybeReserveConnectionId(_))
+ .Times(3)
+ .WillRepeatedly(Return(true));
EXPECT_CALL(cid_manager_visitor_, SendNewConnectionId(_))
.Times(3)
.WillRepeatedly(Return(true));
@@ -777,7 +790,9 @@
QuicConnectionId cid1 = cid_manager_.GenerateNewConnectionId(cid0);
QuicConnectionId cid2 = cid_manager_.GenerateNewConnectionId(cid1);
QuicConnectionId cid3 = cid_manager_.GenerateNewConnectionId(cid2);
- EXPECT_CALL(cid_manager_visitor_, OnNewConnectionIdIssued(_)).Times(3);
+ EXPECT_CALL(cid_manager_visitor_, MaybeReserveConnectionId(_))
+ .Times(3)
+ .WillRepeatedly(Return(true));
EXPECT_CALL(cid_manager_visitor_, SendNewConnectionId(_))
.Times(3)
.WillRepeatedly(Return(true));
@@ -834,7 +849,9 @@
QuicConnectionId cid2 = cid_manager_.GenerateNewConnectionId(cid1);
QuicConnectionId cid3 = cid_manager_.GenerateNewConnectionId(cid2);
QuicConnectionId cid;
- EXPECT_CALL(cid_manager_visitor_, OnNewConnectionIdIssued(_)).Times(3);
+ EXPECT_CALL(cid_manager_visitor_, MaybeReserveConnectionId(_))
+ .Times(3)
+ .WillRepeatedly(Return(true));
EXPECT_CALL(cid_manager_visitor_, SendNewConnectionId(_))
.Times(3)
.WillRepeatedly(Return(true));
@@ -893,7 +910,8 @@
QuicConnectionId cid1 = cid_manager_.GenerateNewConnectionId(cid0);
// CID #1 is sent to peer.
- EXPECT_CALL(cid_manager_visitor_, OnNewConnectionIdIssued(_)).Times(1);
+ EXPECT_CALL(cid_manager_visitor_, MaybeReserveConnectionId(_))
+ .WillOnce(Return(true));
EXPECT_CALL(cid_manager_visitor_, SendNewConnectionId(_))
.WillOnce(Return(true));
cid_manager_.MaybeSendNewConnectionIds();
@@ -909,7 +927,8 @@
TEST_F(QuicSelfIssuedConnectionIdManagerTest,
ErrorWhenTooManyConnectionIdWaitingToBeRetired) {
// CID #0 & #1 are issued.
- EXPECT_CALL(cid_manager_visitor_, OnNewConnectionIdIssued(_));
+ EXPECT_CALL(cid_manager_visitor_, MaybeReserveConnectionId(_))
+ .WillOnce(Return(true));
EXPECT_CALL(cid_manager_visitor_, SendNewConnectionId(_))
.WillOnce(Return(true));
cid_manager_.MaybeSendNewConnectionIds();
@@ -918,7 +937,8 @@
QuicConnectionId last_connection_id =
cid_manager_.GenerateNewConnectionId(initial_connection_id_);
for (int i = 0; i < 8; ++i) {
- EXPECT_CALL(cid_manager_visitor_, OnNewConnectionIdIssued(_));
+ EXPECT_CALL(cid_manager_visitor_, MaybeReserveConnectionId(_))
+ .WillOnce(Return(true));
EXPECT_CALL(cid_manager_visitor_, SendNewConnectionId(_));
QuicRetireConnectionIdFrame retire_cid_frame;
retire_cid_frame.sequence_number = i;
@@ -937,17 +957,60 @@
IsError(QUIC_TOO_MANY_CONNECTION_ID_WAITING_TO_RETIRE));
}
+TEST_F(QuicSelfIssuedConnectionIdManagerTest, CannotIssueNewCidDueToVisitor) {
+ QuicConnectionId cid0 = initial_connection_id_;
+ QuicConnectionId cid1 = cid_manager_.GenerateNewConnectionId(cid0);
+ EXPECT_CALL(cid_manager_visitor_, MaybeReserveConnectionId(cid1))
+ .WillOnce(Return(false));
+ if (GetQuicReloadableFlag(quic_check_cid_collision_when_issue_new_cid)) {
+ EXPECT_CALL(cid_manager_visitor_, SendNewConnectionId(_)).Times(0);
+ } else {
+ EXPECT_CALL(cid_manager_visitor_, SendNewConnectionId(_)).Times(1);
+ }
+ cid_manager_.MaybeSendNewConnectionIds();
+}
+
+TEST_F(QuicSelfIssuedConnectionIdManagerTest,
+ CannotIssueNewCidUponRetireConnectionIdDueToVisitor) {
+ QuicConnectionId cid0 = initial_connection_id_;
+ QuicConnectionId cid1 = cid_manager_.GenerateNewConnectionId(cid0);
+ QuicConnectionId cid2 = cid_manager_.GenerateNewConnectionId(cid1);
+ // CID #0 & #1 are issued.
+ EXPECT_CALL(cid_manager_visitor_, MaybeReserveConnectionId(cid1))
+ .WillOnce(Return(true));
+ EXPECT_CALL(cid_manager_visitor_, SendNewConnectionId(_))
+ .WillOnce(Return(true));
+ cid_manager_.MaybeSendNewConnectionIds();
+
+ // CID #2 is not issued.
+ EXPECT_CALL(cid_manager_visitor_, MaybeReserveConnectionId(cid2))
+ .WillOnce(Return(false));
+ if (GetQuicReloadableFlag(quic_check_cid_collision_when_issue_new_cid)) {
+ EXPECT_CALL(cid_manager_visitor_, SendNewConnectionId(_)).Times(0);
+ } else {
+ EXPECT_CALL(cid_manager_visitor_, SendNewConnectionId(_)).Times(1);
+ }
+ QuicRetireConnectionIdFrame retire_cid_frame;
+ retire_cid_frame.sequence_number = 1;
+ ASSERT_THAT(cid_manager_.OnRetireConnectionIdFrame(
+ retire_cid_frame, pto_delay_, &error_details_),
+ IsQuicNoError());
+}
+
TEST_F(QuicSelfIssuedConnectionIdManagerTest,
DoNotIssueConnectionIdVoluntarilyIfOneHasIssuedForPerferredAddress) {
QuicConnectionId cid0 = initial_connection_id_;
QuicConnectionId cid1 = cid_manager_.GenerateNewConnectionId(cid0);
- EXPECT_CALL(cid_manager_visitor_, OnNewConnectionIdIssued(cid1));
- ASSERT_THAT(cid_manager_.IssueNewConnectionIdForPreferredAddress(),
- ExpectedNewConnectionIdFrame(cid1, 1u, 0u));
+ EXPECT_CALL(cid_manager_visitor_, MaybeReserveConnectionId(cid1))
+ .WillOnce(Return(true));
+ absl::optional<QuicNewConnectionIdFrame> new_cid_frame =
+ cid_manager_.MaybeIssueNewConnectionIdForPreferredAddress();
+ ASSERT_TRUE(new_cid_frame.has_value());
+ ASSERT_THAT(*new_cid_frame, ExpectedNewConnectionIdFrame(cid1, 1u, 0u));
EXPECT_THAT(cid_manager_.GetUnretiredConnectionIds(),
ElementsAre(cid0, cid1));
- EXPECT_CALL(cid_manager_visitor_, OnNewConnectionIdIssued(_)).Times(0);
+ EXPECT_CALL(cid_manager_visitor_, MaybeReserveConnectionId(_)).Times(0);
EXPECT_CALL(cid_manager_visitor_, SendNewConnectionId(_)).Times(0);
cid_manager_.MaybeSendNewConnectionIds();
}
diff --git a/quiche/quic/core/quic_connection_test.cc b/quiche/quic/core/quic_connection_test.cc
index 35c30f4..d33a090 100644
--- a/quiche/quic/core/quic_connection_test.cc
+++ b/quiche/quic/core/quic_connection_test.cc
@@ -1866,9 +1866,11 @@
QuicConnectionPeer::SetAddressValidated(&connection_);
// Sends new server CID to client.
- EXPECT_CALL(visitor_, OnServerConnectionIdIssued(_))
- .WillOnce(
- Invoke([&](const QuicConnectionId& cid) { server_cid1 = cid; }));
+ EXPECT_CALL(visitor_, MaybeReserveConnectionId(_))
+ .WillOnce(Invoke([&](const QuicConnectionId& cid) {
+ server_cid1 = cid;
+ return true;
+ }));
EXPECT_CALL(visitor_, SendNewConnectionId(_));
connection_.OnHandshakeComplete();
@@ -2057,8 +2059,11 @@
// Sends new server CID to client.
QuicConnectionId new_cid;
- EXPECT_CALL(visitor_, OnServerConnectionIdIssued(_))
- .WillOnce(Invoke([&](const QuicConnectionId& cid) { new_cid = cid; }));
+ EXPECT_CALL(visitor_, MaybeReserveConnectionId(_))
+ .WillOnce(Invoke([&](const QuicConnectionId& cid) {
+ new_cid = cid;
+ return true;
+ }));
EXPECT_CALL(visitor_, SendNewConnectionId(_));
// Discard INITIAL key.
connection_.RemoveEncrypter(ENCRYPTION_INITIAL);
@@ -2111,8 +2116,11 @@
// Sends new server CID to client.
QuicConnectionId new_cid;
- EXPECT_CALL(visitor_, OnServerConnectionIdIssued(_))
- .WillOnce(Invoke([&](const QuicConnectionId& cid) { new_cid = cid; }));
+ EXPECT_CALL(visitor_, MaybeReserveConnectionId(_))
+ .WillOnce(Invoke([&](const QuicConnectionId& cid) {
+ new_cid = cid;
+ return true;
+ }));
EXPECT_CALL(visitor_, SendNewConnectionId(_));
// Discard INITIAL key.
connection_.RemoveEncrypter(ENCRYPTION_INITIAL);
@@ -2176,9 +2184,11 @@
QuicConnectionId server_cid0 = connection_.connection_id();
QuicConnectionId server_cid1;
// Sends new server CID to client.
- EXPECT_CALL(visitor_, OnServerConnectionIdIssued(_))
- .WillOnce(
- Invoke([&](const QuicConnectionId& cid) { server_cid1 = cid; }));
+ EXPECT_CALL(visitor_, MaybeReserveConnectionId(_))
+ .WillOnce(Invoke([&](const QuicConnectionId& cid) {
+ server_cid1 = cid;
+ return true;
+ }));
EXPECT_CALL(visitor_, SendNewConnectionId(_));
connection_.OnHandshakeComplete();
// Receives new client CID from client.
@@ -13315,9 +13325,11 @@
QuicConnectionId client_cid1 = TestConnectionId(2);
QuicConnectionId server_cid1;
// Sends new server CID to client.
- EXPECT_CALL(visitor_, OnServerConnectionIdIssued(_))
- .WillOnce(
- Invoke([&](const QuicConnectionId& cid) { server_cid1 = cid; }));
+ EXPECT_CALL(visitor_, MaybeReserveConnectionId(_))
+ .WillOnce(Invoke([&](const QuicConnectionId& cid) {
+ server_cid1 = cid;
+ return true;
+ }));
EXPECT_CALL(visitor_, SendNewConnectionId(_));
connection_.MaybeSendConnectionIdToClient();
// Receives new client CID from client.
@@ -13461,9 +13473,11 @@
QuicConnectionId server_cid0 = connection_.connection_id();
QuicConnectionId server_cid1;
// Sends new server CID to client.
- EXPECT_CALL(visitor_, OnServerConnectionIdIssued(_))
- .WillOnce(
- Invoke([&](const QuicConnectionId& cid) { server_cid1 = cid; }));
+ EXPECT_CALL(visitor_, MaybeReserveConnectionId(_))
+ .WillOnce(Invoke([&](const QuicConnectionId& cid) {
+ server_cid1 = cid;
+ return true;
+ }));
EXPECT_CALL(visitor_, SendNewConnectionId(_));
connection_.MaybeSendConnectionIdToClient();
auto* packet_creator = QuicConnectionPeer::GetPacketCreator(&connection_);
@@ -13580,9 +13594,11 @@
QuicConnectionId client_cid1 = TestConnectionId(2);
QuicConnectionId server_cid1;
// Sends new server CID to client.
- EXPECT_CALL(visitor_, OnServerConnectionIdIssued(_))
- .WillOnce(
- Invoke([&](const QuicConnectionId& cid) { server_cid1 = cid; }));
+ EXPECT_CALL(visitor_, MaybeReserveConnectionId(_))
+ .WillOnce(Invoke([&](const QuicConnectionId& cid) {
+ server_cid1 = cid;
+ return true;
+ }));
EXPECT_CALL(visitor_, SendNewConnectionId(_));
connection_.MaybeSendConnectionIdToClient();
// Receives new client CID from client.
@@ -13682,6 +13698,27 @@
}
}
+TEST_P(QuicConnectionTest, DoNotIssueNewCidIfVisitorSaysNo) {
+ set_perspective(Perspective::IS_SERVER);
+ if (!connection_.connection_migration_use_new_cid()) {
+ return;
+ }
+
+ connection_.CreateConnectionIdManager();
+
+ QuicConnectionId server_cid0 = connection_.connection_id();
+ QuicConnectionId client_cid1 = TestConnectionId(2);
+ QuicConnectionId server_cid1;
+ // Sends new server CID to client.
+ EXPECT_CALL(visitor_, MaybeReserveConnectionId(_)).WillOnce(Return(false));
+ if (GetQuicReloadableFlag(quic_check_cid_collision_when_issue_new_cid)) {
+ EXPECT_CALL(visitor_, SendNewConnectionId(_)).Times(0);
+ } else {
+ EXPECT_CALL(visitor_, SendNewConnectionId(_)).Times(1);
+ }
+ connection_.MaybeSendConnectionIdToClient();
+}
+
TEST_P(QuicConnectionTest,
ProbedOnAnotherPathAfterPeerIpAddressChangeAtServer) {
PathProbeTestInit(Perspective::IS_SERVER);
@@ -14217,7 +14254,7 @@
set_perspective(Perspective::IS_SERVER);
connection_.CreateConnectionIdManager();
- EXPECT_CALL(visitor_, OnServerConnectionIdIssued(_));
+ EXPECT_CALL(visitor_, MaybeReserveConnectionId(_));
EXPECT_CALL(visitor_, SendNewConnectionId(_));
connection_.MaybeSendConnectionIdToClient();
@@ -14250,7 +14287,9 @@
QuicRetireConnectionIdFrame frame;
frame.sequence_number = 0u;
if (connection_.connection_migration_use_new_cid()) {
- EXPECT_CALL(visitor_, OnServerConnectionIdIssued(_)).Times(2);
+ EXPECT_CALL(visitor_, MaybeReserveConnectionId(_))
+ .Times(2)
+ .WillRepeatedly(Return(true));
EXPECT_CALL(visitor_, SendNewConnectionId(_)).Times(2);
}
EXPECT_TRUE(connection_.OnRetireConnectionIdFrame(frame));
@@ -14270,8 +14309,9 @@
set_perspective(Perspective::IS_SERVER);
connection_.CreateConnectionIdManager();
QuicConnectionId recorded_cid;
- auto cid_recorder = [&recorded_cid](const QuicConnectionId& cid) {
+ auto cid_recorder = [&recorded_cid](const QuicConnectionId& cid) -> bool {
recorded_cid = cid;
+ return true;
};
QuicConnectionId cid0 = connection_id_;
QuicConnectionId cid1;
@@ -14280,7 +14320,7 @@
EXPECT_EQ(connection_.GetOneActiveServerConnectionId(), cid0);
connection_.SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE);
- EXPECT_CALL(visitor_, OnServerConnectionIdIssued(_))
+ EXPECT_CALL(visitor_, MaybeReserveConnectionId(_))
.WillOnce(Invoke(cid_recorder));
EXPECT_CALL(visitor_, SendNewConnectionId(_));
connection_.MaybeSendConnectionIdToClient();
@@ -14311,7 +14351,7 @@
// Packet2 with RetireConnectionId frame trigers sending NewConnectionId
// immediately.
- EXPECT_CALL(visitor_, OnServerConnectionIdIssued(_))
+ EXPECT_CALL(visitor_, MaybeReserveConnectionId(_))
.WillOnce(Invoke(cid_recorder));
EXPECT_CALL(visitor_, SendNewConnectionId(_));
peer_creator_.SetServerConnectionId(cid1);
diff --git a/quiche/quic/core/quic_dispatcher.cc b/quiche/quic/core/quic_dispatcher.cc
index 30a93df..883a59d 100644
--- a/quiche/quic/core/quic_dispatcher.cc
+++ b/quiche/quic/core/quic_dispatcher.cc
@@ -1099,7 +1099,7 @@
void QuicDispatcher::OnStopSendingReceived(
const QuicStopSendingFrame& /*frame*/) {}
-void QuicDispatcher::OnNewConnectionIdSent(
+bool QuicDispatcher::TryAddNewConnectionId(
const QuicConnectionId& server_connection_id,
const QuicConnectionId& new_connection_id) {
auto it = reference_counted_session_map_.find(server_connection_id);
@@ -1108,13 +1108,16 @@
<< "Couldn't locate the session that issues the connection ID in "
"reference_counted_session_map_. server_connection_id:"
<< server_connection_id << " new_connection_id: " << new_connection_id;
- return;
+ return false;
}
// Count new connection ID added to the dispatcher map.
QUIC_RELOADABLE_FLAG_COUNT_N(quic_connection_migration_use_new_cid_v2, 6, 6);
auto insertion_result = reference_counted_session_map_.insert(
std::make_pair(new_connection_id, it->second));
- QUICHE_DCHECK(insertion_result.second);
+ if (!insertion_result.second) {
+ QUIC_CODE_COUNT(quic_cid_already_in_session_map);
+ }
+ return insertion_result.second;
}
void QuicDispatcher::OnConnectionIdRetired(
diff --git a/quiche/quic/core/quic_dispatcher.h b/quiche/quic/core/quic_dispatcher.h
index 0a103cd..878f6ac 100644
--- a/quiche/quic/core/quic_dispatcher.h
+++ b/quiche/quic/core/quic_dispatcher.h
@@ -102,8 +102,9 @@
// QuicSession::Visitor interface implementation (via inheritance of
// QuicTimeWaitListManager::Visitor):
- // Add the newly issued connection ID to the session map.
- void OnNewConnectionIdSent(
+ // Try to add the new connection ID to the session map. Returns true on
+ // success.
+ bool TryAddNewConnectionId(
const QuicConnectionId& server_connection_id,
const QuicConnectionId& new_connection_id) override;
diff --git a/quiche/quic/core/quic_dispatcher_test.cc b/quiche/quic/core/quic_dispatcher_test.cc
index f8c169c..b8b8b44 100644
--- a/quiche/quic/core/quic_dispatcher_test.cc
+++ b/quiche/quic/core/quic_dispatcher_test.cc
@@ -170,7 +170,10 @@
active_connection_ids_({connection_id}) {}
void AddNewConnectionId(QuicConnectionId id) {
- dispatcher_->OnNewConnectionIdSent(active_connection_ids_.back(), id);
+ if (!dispatcher_->TryAddNewConnectionId(active_connection_ids_.back(),
+ id)) {
+ return;
+ }
QuicConnectionPeer::SetServerConnectionId(this, id);
active_connection_ids_.push_back(id);
}
@@ -2243,7 +2246,14 @@
::testing::PrintToStringParamName());
TEST_P(QuicDispatcherSupportMultipleConnectionIdPerConnectionTest,
- OnNewConnectionIdSent) {
+ FailToAddExistingConnectionId) {
+ AddConnection1();
+ EXPECT_FALSE(dispatcher_->TryAddNewConnectionId(TestConnectionId(1),
+ TestConnectionId(1)));
+}
+
+TEST_P(QuicDispatcherSupportMultipleConnectionIdPerConnectionTest,
+ TryAddNewConnectionId) {
AddConnection1();
ASSERT_EQ(dispatcher_->NumSessions(), 1u);
ASSERT_THAT(session1_, testing::NotNull());
diff --git a/quiche/quic/core/quic_session.cc b/quiche/quic/core/quic_session.cc
index d297600..22b05ef 100644
--- a/quiche/quic/core/quic_session.cc
+++ b/quiche/quic/core/quic_session.cc
@@ -2098,12 +2098,13 @@
control_frame_manager_.WriteOrBufferRetireConnectionId(sequence_number);
}
-void QuicSession::OnServerConnectionIdIssued(
+bool QuicSession::MaybeReserveConnectionId(
const QuicConnectionId& server_connection_id) {
if (visitor_) {
- visitor_->OnNewConnectionIdSent(
+ return visitor_->TryAddNewConnectionId(
connection_->GetOneActiveServerConnectionId(), server_connection_id);
}
+ return true;
}
void QuicSession::OnServerConnectionIdRetired(
diff --git a/quiche/quic/core/quic_session.h b/quiche/quic/core/quic_session.h
index 7bdcaa9..0da3b6c 100644
--- a/quiche/quic/core/quic_session.h
+++ b/quiche/quic/core/quic_session.h
@@ -89,8 +89,8 @@
// peer.
virtual void OnStopSendingReceived(const QuicStopSendingFrame& frame) = 0;
- // Called when a NewConnectionId frame has been sent.
- virtual void OnNewConnectionIdSent(
+ // Called when on whether a NewConnectionId frame can been sent.
+ virtual bool TryAddNewConnectionId(
const QuicConnectionId& server_connection_id,
const QuicConnectionId& new_connection_id) = 0;
@@ -145,7 +145,12 @@
void SendAckFrequency(const QuicAckFrequencyFrame& frame) override;
void SendNewConnectionId(const QuicNewConnectionIdFrame& frame) override;
void SendRetireConnectionId(uint64_t sequence_number) override;
- void OnServerConnectionIdIssued(
+ // Returns true if server_connection_id can be issued. If returns true,
+ // |visitor_| may establish a mapping from |server_connection_id| to this
+ // session, if that's not desired,
+ // OnServerConnectionIdRetired(server_connection_id) can be used to remove the
+ // mapping.
+ bool MaybeReserveConnectionId(
const QuicConnectionId& server_connection_id) override;
void OnServerConnectionIdRetired(
const QuicConnectionId& server_connection_id) override;
diff --git a/quiche/quic/test_tools/mock_quic_session_visitor.h b/quiche/quic/test_tools/mock_quic_session_visitor.h
index 53c09d8..61dc5d5 100644
--- a/quiche/quic/test_tools/mock_quic_session_visitor.h
+++ b/quiche/quic/test_tools/mock_quic_session_visitor.h
@@ -27,7 +27,7 @@
(override));
MOCK_METHOD(void, OnStopSendingReceived, (const QuicStopSendingFrame& frame),
(override));
- MOCK_METHOD(void, OnNewConnectionIdSent,
+ MOCK_METHOD(bool, TryAddNewConnectionId,
(const QuicConnectionId& server_connection_id,
const QuicConnectionId& new_connection_id),
(override));
diff --git a/quiche/quic/test_tools/quic_test_utils.h b/quiche/quic/test_tools/quic_test_utils.h
index 0e2c20c..5e4b52e 100644
--- a/quiche/quic/test_tools/quic_test_utils.h
+++ b/quiche/quic/test_tools/quic_test_utils.h
@@ -475,7 +475,7 @@
(const QuicNewConnectionIdFrame& frame), (override));
MOCK_METHOD(void, SendRetireConnectionId, (uint64_t sequence_number),
(override));
- MOCK_METHOD(void, OnServerConnectionIdIssued,
+ MOCK_METHOD(bool, MaybeReserveConnectionId,
(const QuicConnectionId& server_connection_id), (override));
MOCK_METHOD(void, OnServerConnectionIdRetired,
(const QuicConnectionId& server_connection_id), (override));
diff --git a/quiche/quic/test_tools/simulator/quic_endpoint.h b/quiche/quic/test_tools/simulator/quic_endpoint.h
index d3741c5..7654d89 100644
--- a/quiche/quic/test_tools/simulator/quic_endpoint.h
+++ b/quiche/quic/test_tools/simulator/quic_endpoint.h
@@ -74,8 +74,10 @@
void SendNewConnectionId(const QuicNewConnectionIdFrame& /*frame*/) override {
}
void SendRetireConnectionId(uint64_t /*sequence_number*/) override {}
- void OnServerConnectionIdIssued(
- const QuicConnectionId& /*server_connection_id*/) override {}
+ bool MaybeReserveConnectionId(
+ const QuicConnectionId& /*server_connection_id*/) override {
+ return true;
+ }
void OnServerConnectionIdRetired(
const QuicConnectionId& /*server_connection_id*/) override {}
bool AllowSelfAddressChange() const override;
