gfe-relnote: Move QuicCryptoServerStream::Helper::GenerateConnectionIdForReject to QuicDispatcher::GenerateNewServerConnectionId. Refactor only.
This function used to be related to reject, but now it is only used to generate server connection ids when the incoming id length is unexpected by the dispatcher.
PiperOrigin-RevId: 263843315
Change-Id: I5405ad8e5712ced51c8090be6a99aff396f5ab32
diff --git a/quic/core/quic_crypto_server_stream.h b/quic/core/quic_crypto_server_stream.h
index c5e0abc..60a162c 100644
--- a/quic/core/quic_crypto_server_stream.h
+++ b/quic/core/quic_crypto_server_stream.h
@@ -125,12 +125,6 @@
public:
virtual ~Helper() {}
- // Given the current connection_id, generates a new ConnectionId to
- // be returned with a reject.
- virtual QuicConnectionId GenerateConnectionIdForReject(
- QuicTransportVersion version,
- QuicConnectionId connection_id) const = 0;
-
// Returns true if |message|, which was received on |self_address| is
// acceptable according to the visitor's policy. Otherwise, returns false
// and populates |error_details|.
diff --git a/quic/core/quic_crypto_server_stream_test.cc b/quic/core/quic_crypto_server_stream_test.cc
index 238162c..b58d987 100644
--- a/quic/core/quic_crypto_server_stream_test.cc
+++ b/quic/core/quic_crypto_server_stream_test.cc
@@ -90,8 +90,6 @@
server_session_.reset(server_session);
EXPECT_CALL(*server_session_->helper(), CanAcceptClientHello(_, _, _, _, _))
.Times(testing::AnyNumber());
- EXPECT_CALL(*server_session_->helper(), GenerateConnectionIdForReject(_, _))
- .Times(testing::AnyNumber());
crypto_test_utils::SetupCryptoServerConfigForTest(
server_connection_->clock(), server_connection_->random_generator(),
&server_crypto_config_);
diff --git a/quic/core/quic_dispatcher.cc b/quic/core/quic_dispatcher.cc
index 4c73dfe..b659bda 100644
--- a/quic/core/quic_dispatcher.cc
+++ b/quic/core/quic_dispatcher.cc
@@ -310,8 +310,7 @@
return it->second;
}
QuicConnectionId new_connection_id =
- session_helper_->GenerateConnectionIdForReject(version.transport_version,
- server_connection_id);
+ GenerateNewServerConnectionId(version, server_connection_id);
DCHECK_EQ(expected_server_connection_id_length_, new_connection_id.length());
// TODO(dschinazi) Prevent connection_id_map_ from growing indefinitely
// before we ship a version that supports variable length connection IDs
@@ -323,6 +322,12 @@
return new_connection_id;
}
+QuicConnectionId QuicDispatcher::GenerateNewServerConnectionId(
+ ParsedQuicVersion /*version*/,
+ QuicConnectionId /*connection_id*/) const {
+ return QuicUtils::CreateRandomConnectionId();
+}
+
bool QuicDispatcher::MaybeDispatchPacket(
const ReceivedPacketInfo& packet_info) {
// Port zero is only allowed for unidirectional UDP, so is disallowed by QUIC.
diff --git a/quic/core/quic_dispatcher.h b/quic/core/quic_dispatcher.h
index 091e727..f7b0450 100644
--- a/quic/core/quic_dispatcher.h
+++ b/quic/core/quic_dispatcher.h
@@ -152,6 +152,11 @@
// otherwise, returns false and the packet needs further processing.
virtual bool MaybeDispatchPacket(const ReceivedPacketInfo& packet_info);
+ // Generate a connection ID with a length that is expected by the dispatcher.
+ virtual QuicConnectionId GenerateNewServerConnectionId(
+ ParsedQuicVersion version,
+ QuicConnectionId connection_id) const;
+
// Values to be returned by ValidityChecks() to indicate what should be done
// with a packet. Fates with greater values are considered to be higher
// priority. ValidityChecks should return fate based on the priority order
diff --git a/quic/core/quic_dispatcher_test.cc b/quic/core/quic_dispatcher_test.cc
index bd30333..b41c57f 100644
--- a/quic/core/quic_dispatcher_test.cc
+++ b/quic/core/quic_dispatcher_test.cc
@@ -118,9 +118,10 @@
version_manager,
QuicMakeUnique<MockQuicConnectionHelper>(),
std::unique_ptr<QuicCryptoServerStream::Helper>(
- new QuicSimpleCryptoServerStreamHelper(random)),
+ new QuicSimpleCryptoServerStreamHelper()),
QuicMakeUnique<MockAlarmFactory>(),
- kQuicDefaultConnectionIdLength) {}
+ kQuicDefaultConnectionIdLength),
+ random_(random) {}
MOCK_METHOD4(CreateQuicSession,
QuicServerSessionBase*(QuicConnectionId connection_id,
@@ -131,6 +132,12 @@
MOCK_METHOD1(ShouldCreateOrBufferPacketForConnection,
bool(const ReceivedPacketInfo& packet_info));
+ QuicConnectionId GenerateNewServerConnectionId(
+ ParsedQuicVersion /*version*/,
+ QuicConnectionId /*connection_id*/) const override {
+ return QuicUtils::CreateRandomConnectionId(random_);
+ }
+
struct TestQuicPerPacketContext : public QuicPerPacketContext {
std::string custom_packet_context;
};
@@ -152,6 +159,8 @@
using QuicDispatcher::SetAllowShortInitialServerConnectionIds;
using QuicDispatcher::writer;
+
+ QuicRandom* random_;
};
// A Connection class which unregisters the session from the dispatcher when
@@ -187,9 +196,7 @@
: QuicDispatcherTest(crypto_test_utils::ProofSourceForTesting()) {}
explicit QuicDispatcherTest(std::unique_ptr<ProofSource> proof_source)
- :
-
- version_manager_(AllSupportedVersions()),
+ : version_manager_(AllSupportedVersions()),
crypto_config_(QuicCryptoServerConfig::TESTING,
QuicRandom::GetInstance(),
std::move(proof_source),
diff --git a/quic/quartc/quartc_crypto_helpers.cc b/quic/quartc/quartc_crypto_helpers.cc
index 1446ab3..4f39271 100644
--- a/quic/quartc/quartc_crypto_helpers.cc
+++ b/quic/quartc/quartc_crypto_helpers.cc
@@ -72,14 +72,6 @@
return nullptr;
}
-QuicConnectionId QuartcCryptoServerStreamHelper::GenerateConnectionIdForReject(
- QuicTransportVersion /*version*/,
- QuicConnectionId /*connection_id*/) const {
- // TODO(b/124399417): Request a zero-length connection id here when the QUIC
- // server perspective supports it.
- return QuicUtils::CreateRandomConnectionId();
-}
-
bool QuartcCryptoServerStreamHelper::CanAcceptClientHello(
const CryptoHandshakeMessage& /*message*/,
const QuicSocketAddress& /*client_address*/,
diff --git a/quic/quartc/quartc_crypto_helpers.h b/quic/quartc/quartc_crypto_helpers.h
index 1436aeb..cdf14a0 100644
--- a/quic/quartc/quartc_crypto_helpers.h
+++ b/quic/quartc/quartc_crypto_helpers.h
@@ -101,10 +101,6 @@
// Implementation of the server-side crypto stream helper.
class QuartcCryptoServerStreamHelper : public QuicCryptoServerStream::Helper {
public:
- QuicConnectionId GenerateConnectionIdForReject(
- QuicTransportVersion version,
- QuicConnectionId connection_id) const override;
-
bool CanAcceptClientHello(const CryptoHandshakeMessage& message,
const QuicSocketAddress& client_address,
const QuicSocketAddress& peer_address,
diff --git a/quic/quartc/quartc_dispatcher.h b/quic/quartc/quartc_dispatcher.h
index d1128d6..06e799e 100644
--- a/quic/quartc/quartc_dispatcher.h
+++ b/quic/quartc/quartc_dispatcher.h
@@ -45,6 +45,9 @@
QuicStringPiece alpn,
const ParsedQuicVersion& version) override;
+ // TODO(b/124399417): Override GenerateNewServerConnectionId and request a
+ // zero-length connection id when the QUIC server perspective supports it.
+
// QuartcPacketTransport::Delegate overrides.
void OnTransportCanWrite() override;
void OnTransportReceived(const char* data, size_t data_len) override;
diff --git a/quic/test_tools/crypto_test_utils.cc b/quic/test_tools/crypto_test_utils.cc
index 3fb896a..bfbc3d7 100644
--- a/quic/test_tools/crypto_test_utils.cc
+++ b/quic/test_tools/crypto_test_utils.cc
@@ -232,9 +232,6 @@
CanAcceptClientHello(testing::_, testing::_, testing::_,
testing::_, testing::_))
.Times(testing::AnyNumber());
- EXPECT_CALL(*server_session.helper(),
- GenerateConnectionIdForReject(testing::_, testing::_))
- .Times(testing::AnyNumber());
EXPECT_CALL(*server_conn, OnCanWrite()).Times(testing::AnyNumber());
EXPECT_CALL(*client_conn, OnCanWrite()).Times(testing::AnyNumber());
diff --git a/quic/test_tools/mock_quic_session_visitor.h b/quic/test_tools/mock_quic_session_visitor.h
index 11dc24a..0954bb7 100644
--- a/quic/test_tools/mock_quic_session_visitor.h
+++ b/quic/test_tools/mock_quic_session_visitor.h
@@ -39,9 +39,6 @@
MockQuicCryptoServerStreamHelper& operator=(
const MockQuicCryptoServerStreamHelper&) = delete;
~MockQuicCryptoServerStreamHelper() override;
- MOCK_CONST_METHOD2(GenerateConnectionIdForReject,
- QuicConnectionId(QuicTransportVersion version,
- QuicConnectionId connection_id));
MOCK_CONST_METHOD5(CanAcceptClientHello,
bool(const CryptoHandshakeMessage& message,
const QuicSocketAddress& client_address,
diff --git a/quic/test_tools/quic_test_server.cc b/quic/test_tools/quic_test_server.cc
index 68c500e..19b986e 100644
--- a/quic/test_tools/quic_test_server.cc
+++ b/quic/test_tools/quic_test_server.cc
@@ -184,7 +184,7 @@
QuicMakeUnique<QuicEpollConnectionHelper>(epoll_server(),
QuicAllocator::BUFFER_POOL),
std::unique_ptr<QuicCryptoServerStream::Helper>(
- new QuicSimpleCryptoServerStreamHelper(QuicRandom::GetInstance())),
+ new QuicSimpleCryptoServerStreamHelper()),
QuicMakeUnique<QuicEpollAlarmFactory>(epoll_server()), server_backend(),
expected_server_connection_id_length());
}
diff --git a/quic/test_tools/quic_test_utils.cc b/quic/test_tools/quic_test_utils.cc
index ed8c551..9274567 100644
--- a/quic/test_tools/quic_test_utils.cc
+++ b/quic/test_tools/quic_test_utils.cc
@@ -638,9 +638,6 @@
crypto_config,
compressed_certs_cache) {
Initialize();
- ON_CALL(helper_, GenerateConnectionIdForReject(_, _))
- .WillByDefault(testing::Return(
- QuicUtils::CreateRandomConnectionId(connection->random_generator())));
ON_CALL(helper_, CanAcceptClientHello(_, _, _, _, _))
.WillByDefault(testing::Return(true));
}
diff --git a/quic/tools/quic_server.cc b/quic/tools/quic_server.cc
index 8820040..46b04a5 100644
--- a/quic/tools/quic_server.cc
+++ b/quic/tools/quic_server.cc
@@ -155,7 +155,7 @@
std::unique_ptr<QuicEpollConnectionHelper>(new QuicEpollConnectionHelper(
&epoll_server_, QuicAllocator::BUFFER_POOL)),
std::unique_ptr<QuicCryptoServerStream::Helper>(
- new QuicSimpleCryptoServerStreamHelper(QuicRandom::GetInstance())),
+ new QuicSimpleCryptoServerStreamHelper()),
std::unique_ptr<QuicEpollAlarmFactory>(
new QuicEpollAlarmFactory(&epoll_server_)),
quic_simple_server_backend_, expected_server_connection_id_length_);
diff --git a/quic/tools/quic_server_test.cc b/quic/tools/quic_server_test.cc
index b1a88a6..3a2d520 100644
--- a/quic/tools/quic_server_test.cc
+++ b/quic/tools/quic_server_test.cc
@@ -72,7 +72,7 @@
new QuicEpollConnectionHelper(epoll_server(),
QuicAllocator::BUFFER_POOL)),
std::unique_ptr<QuicCryptoServerStream::Helper>(
- new QuicSimpleCryptoServerStreamHelper(QuicRandom::GetInstance())),
+ new QuicSimpleCryptoServerStreamHelper()),
std::unique_ptr<QuicEpollAlarmFactory>(
new QuicEpollAlarmFactory(epoll_server())),
&quic_simple_server_backend_);
@@ -163,8 +163,7 @@
new QuicEpollConnectionHelper(&eps_,
QuicAllocator::BUFFER_POOL)),
std::unique_ptr<QuicCryptoServerStream::Helper>(
- new QuicSimpleCryptoServerStreamHelper(
- QuicRandom::GetInstance())),
+ new QuicSimpleCryptoServerStreamHelper()),
std::unique_ptr<QuicEpollAlarmFactory>(
new QuicEpollAlarmFactory(&eps_)),
&quic_simple_server_backend_) {
diff --git a/quic/tools/quic_simple_crypto_server_stream_helper.cc b/quic/tools/quic_simple_crypto_server_stream_helper.cc
index 9d55600..d27506e 100644
--- a/quic/tools/quic_simple_crypto_server_stream_helper.cc
+++ b/quic/tools/quic_simple_crypto_server_stream_helper.cc
@@ -8,20 +8,12 @@
namespace quic {
-QuicSimpleCryptoServerStreamHelper::QuicSimpleCryptoServerStreamHelper(
- QuicRandom* random)
- : random_(random) {}
+QuicSimpleCryptoServerStreamHelper::QuicSimpleCryptoServerStreamHelper() =
+ default;
QuicSimpleCryptoServerStreamHelper::~QuicSimpleCryptoServerStreamHelper() =
default;
-QuicConnectionId
-QuicSimpleCryptoServerStreamHelper::GenerateConnectionIdForReject(
- QuicTransportVersion /*version*/,
- QuicConnectionId /*connection_id*/) const {
- return QuicUtils::CreateRandomConnectionId(random_);
-}
-
bool QuicSimpleCryptoServerStreamHelper::CanAcceptClientHello(
const CryptoHandshakeMessage& /*message*/,
const QuicSocketAddress& /*client_address*/,
diff --git a/quic/tools/quic_simple_crypto_server_stream_helper.h b/quic/tools/quic_simple_crypto_server_stream_helper.h
index 3480776..c4dcb08 100644
--- a/quic/tools/quic_simple_crypto_server_stream_helper.h
+++ b/quic/tools/quic_simple_crypto_server_stream_helper.h
@@ -15,22 +15,15 @@
class QuicSimpleCryptoServerStreamHelper
: public QuicCryptoServerStream::Helper {
public:
- explicit QuicSimpleCryptoServerStreamHelper(QuicRandom* random);
+ QuicSimpleCryptoServerStreamHelper();
~QuicSimpleCryptoServerStreamHelper() override;
- QuicConnectionId GenerateConnectionIdForReject(
- QuicTransportVersion /*version*/,
- QuicConnectionId /*connection_id*/) const override;
-
bool CanAcceptClientHello(const CryptoHandshakeMessage& message,
const QuicSocketAddress& client_address,
const QuicSocketAddress& peer_address,
const QuicSocketAddress& self_address,
std::string* error_details) const override;
-
- private:
- QuicRandom* random_; // Unowned.
};
} // namespace quic
diff --git a/quic/tools/quic_simple_crypto_server_stream_helper_test.cc b/quic/tools/quic_simple_crypto_server_stream_helper_test.cc
deleted file mode 100644
index 4ede588..0000000
--- a/quic/tools/quic_simple_crypto_server_stream_helper_test.cc
+++ /dev/null
@@ -1,25 +0,0 @@
-// Copyright (c) 2019 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "net/third_party/quiche/src/quic/tools/quic_simple_crypto_server_stream_helper.h"
-
-#include "net/third_party/quiche/src/quic/core/quic_utils.h"
-#include "net/third_party/quiche/src/quic/platform/api/quic_test.h"
-#include "net/third_party/quiche/src/quic/test_tools/mock_random.h"
-#include "net/third_party/quiche/src/quic/test_tools/quic_test_utils.h"
-
-namespace quic {
-
-class QuicSimpleCryptoServerStreamHelperTest : public QuicTest {};
-
-TEST_F(QuicSimpleCryptoServerStreamHelperTest, GenerateConnectionIdForReject) {
- test::MockRandom random;
- QuicSimpleCryptoServerStreamHelper helper(&random);
-
- EXPECT_EQ(QuicUtils::CreateRandomConnectionId(&random),
- helper.GenerateConnectionIdForReject(QUIC_VERSION_99,
- test::TestConnectionId()));
-}
-
-} // namespace quic
