Use a factory to create ClientHandshake (#59)

Summary:
This will allow to be able to create different kind of handshake going forward.
Pull Request resolved: https://github.com/facebookincubator/mvfst/pull/59

Reviewed By: siyengar

Differential Revision: D18088574

Pulled By: mjoras

fbshipit-source-id: 0732bb63a9e243fef77cdaf4f76e711fcb09ecdc

quic/api/test/IoBufQuicBatchTest.cpp

@@ -7,7 +7,9 @@
  */
 
 #include <quic/api/IoBufQuicBatch.h>
+
 #include <gtest/gtest.h>
+#include <quic/client/handshake/FizzClientQuicHandshakeContext.h>
 #include <quic/client/state/ClientStateMachine.h>
 #include <quic/state/StateData.h>
 
@@ -53,7 +55,8 @@ void RunTest(int numBatch) {
 
   auto batchWriter = std::make_unique<TestPacketBatchWriter>(numBatch);
   folly::SocketAddress peerAddress{"127.0.0.1", 1234};
-  QuicClientConnectionState conn;
+  QuicClientConnectionState conn(
+      std::make_shared<FizzClientQuicHandshakeContext>());
   QuicConnectionStateBase::HappyEyeballsState happyEyeballsState;
 
   IOBufQuicBatch ioBufBatch(

quic/api/test/QuicPacketSchedulerTest.cpp

@@ -11,6 +11,7 @@
 #include <folly/portability/GTest.h>
 
 #include <quic/api/test/Mocks.h>
+#include <quic/client/handshake/FizzClientQuicHandshakeContext.h>
 #include <quic/client/state/ClientStateMachine.h>
 #include <quic/codec/QuicPacketBuilder.h>
 #include <quic/codec/test/Mocks.h>
@@ -115,7 +116,8 @@ TEST_F(QuicPacketSchedulerTest, NoopScheduler) {
 }
 
 TEST_F(QuicPacketSchedulerTest, CryptoPaddingInitialPacket) {
-  QuicClientConnectionState conn;
+  QuicClientConnectionState conn(
+      std::make_shared<FizzClientQuicHandshakeContext>());
   auto connId = getTestConnectionId();
   LongHeader longHeader1(
       LongHeader::Types::Initial,
@@ -174,7 +176,8 @@ TEST_F(QuicPacketSchedulerTest, CryptoServerInitialNotPadded) {
 }
 
 TEST_F(QuicPacketSchedulerTest, CryptoPaddingRetransmissionClientInitial) {
-  QuicClientConnectionState conn;
+  QuicClientConnectionState conn(
+      std::make_shared<FizzClientQuicHandshakeContext>());
   auto connId = getTestConnectionId();
   LongHeader longHeader(
       LongHeader::Types::Initial,
@@ -221,7 +224,8 @@ TEST_F(QuicPacketSchedulerTest, CryptoSchedulerOnlySingleLossFits) {
 }
 
 TEST_F(QuicPacketSchedulerTest, CryptoWritePartialLossBuffer) {
-  QuicClientConnectionState conn;
+  QuicClientConnectionState conn(
+      std::make_shared<FizzClientQuicHandshakeContext>());
   auto connId = getTestConnectionId();
   LongHeader longHeader(
       LongHeader::Types::Initial,
@@ -308,7 +312,8 @@ TEST_F(QuicPacketSchedulerTest, StreamFrameSchedulerStreamNotExists) {
 }
 
 TEST_F(QuicPacketSchedulerTest, CloningSchedulerTest) {
-  QuicClientConnectionState conn;
+  QuicClientConnectionState conn(
+      std::make_shared<FizzClientQuicHandshakeContext>());
   FrameScheduler noopScheduler("frame");
   ASSERT_FALSE(noopScheduler.hasData());
   CloningScheduler cloningScheduler(noopScheduler, conn, "CopyCat", 0);
@@ -335,7 +340,8 @@ TEST_F(QuicPacketSchedulerTest, CloningSchedulerTest) {
 }
 
 TEST_F(QuicPacketSchedulerTest, WriteOnlyOutstandingPacketsTest) {
-  QuicClientConnectionState conn;
+  QuicClientConnectionState conn(
+      std::make_shared<FizzClientQuicHandshakeContext>());
   FrameScheduler noopScheduler("frame");
   ASSERT_FALSE(noopScheduler.hasData());
   CloningScheduler cloningScheduler(noopScheduler, conn, "CopyCat", 0);
@@ -402,7 +408,8 @@ TEST_F(QuicPacketSchedulerTest, WriteOnlyOutstandingPacketsTest) {
 }
 
 TEST_F(QuicPacketSchedulerTest, DoNotCloneProcessedClonedPacket) {
-  QuicClientConnectionState conn;
+  QuicClientConnectionState conn(
+      std::make_shared<FizzClientQuicHandshakeContext>());
   FrameScheduler noopScheduler("frame");
   CloningScheduler cloningScheduler(noopScheduler, conn, "CopyCat", 0);
   // Add two outstanding packets, but then mark the second one processed by
@@ -432,7 +439,8 @@ TEST_F(QuicPacketSchedulerTest, DoNotCloneProcessedClonedPacket) {
 }
 
 TEST_F(QuicPacketSchedulerTest, DoNotClonePureAck) {
-  QuicClientConnectionState conn;
+  QuicClientConnectionState conn(
+      std::make_shared<FizzClientQuicHandshakeContext>());
   FrameScheduler noopScheduler("frame");
   CloningScheduler cloningScheduler(noopScheduler, conn, "CopyCat", 0);
   // Add two outstanding packets, with second one being pureAck
@@ -458,7 +466,8 @@ TEST_F(QuicPacketSchedulerTest, DoNotClonePureAck) {
 }
 
 TEST_F(QuicPacketSchedulerTest, CloneSchedulerHasDataIgnoresNonAppData) {
-  QuicClientConnectionState conn;
+  QuicClientConnectionState conn(
+      std::make_shared<FizzClientQuicHandshakeContext>());
   FrameScheduler noopScheduler("frame");
   CloningScheduler cloningScheduler(noopScheduler, conn, "CopyCat", 0);
   EXPECT_FALSE(cloningScheduler.hasData());
@@ -474,7 +483,8 @@ TEST_F(QuicPacketSchedulerTest, CloneSchedulerHasDataIgnoresNonAppData) {
 }
 
 TEST_F(QuicPacketSchedulerTest, DoNotCloneHandshake) {
-  QuicClientConnectionState conn;
+  QuicClientConnectionState conn(
+      std::make_shared<FizzClientQuicHandshakeContext>());
   FrameScheduler noopScheduler("frame");
   CloningScheduler cloningScheduler(noopScheduler, conn, "CopyCat", 0);
   // Add two outstanding packets, with second one being handshake
@@ -501,7 +511,8 @@ TEST_F(QuicPacketSchedulerTest, DoNotCloneHandshake) {
 }
 
 TEST_F(QuicPacketSchedulerTest, CloneSchedulerUseNormalSchedulerFirst) {
-  QuicClientConnectionState conn;
+  QuicClientConnectionState conn(
+      std::make_shared<FizzClientQuicHandshakeContext>());
   MockFrameScheduler mockScheduler;
   CloningScheduler cloningScheduler(mockScheduler, conn, "Mocker", 0);
   ShortHeader header(
@@ -549,7 +560,8 @@ TEST_F(QuicPacketSchedulerTest, CloneSchedulerUseNormalSchedulerFirst) {
 }
 
 TEST_F(QuicPacketSchedulerTest, CloneWillGenerateNewWindowUpdate) {
-  QuicClientConnectionState conn;
+  QuicClientConnectionState conn(
+      std::make_shared<FizzClientQuicHandshakeContext>());
   conn.streamManager->setMaxLocalBidirectionalStreams(10);
   auto stream = conn.streamManager->createNextBidirectionalStream().value();
   FrameScheduler noopScheduler("frame");
@@ -630,7 +642,8 @@ TEST_F(QuicPacketSchedulerTest, CloneWillGenerateNewWindowUpdate) {
 class AckSchedulingTest : public TestWithParam<PacketNumberSpace> {};
 
 TEST_F(QuicPacketSchedulerTest, AckStateHasAcksToSchedule) {
-  QuicClientConnectionState conn;
+  QuicClientConnectionState conn(
+      std::make_shared<FizzClientQuicHandshakeContext>());
   EXPECT_FALSE(hasAcksToSchedule(conn.ackStates.initialAckState));
   EXPECT_FALSE(hasAcksToSchedule(conn.ackStates.handshakeAckState));
   EXPECT_FALSE(hasAcksToSchedule(conn.ackStates.appDataAckState));
@@ -647,7 +660,8 @@ TEST_F(QuicPacketSchedulerTest, AckStateHasAcksToSchedule) {
 }
 
 TEST_F(QuicPacketSchedulerTest, AckSchedulerHasAcksToSchedule) {
-  QuicClientConnectionState conn;
+  QuicClientConnectionState conn(
+      std::make_shared<FizzClientQuicHandshakeContext>());
   AckScheduler initialAckScheduler(
       conn, getAckState(conn, PacketNumberSpace::Initial));
   AckScheduler handshakeAckScheduler(
@@ -670,7 +684,8 @@ TEST_F(QuicPacketSchedulerTest, AckSchedulerHasAcksToSchedule) {
 }
 
 TEST_F(QuicPacketSchedulerTest, LargestAckToSend) {
-  QuicClientConnectionState conn;
+  QuicClientConnectionState conn(
+      std::make_shared<FizzClientQuicHandshakeContext>());
   EXPECT_EQ(folly::none, largestAckToSend(conn.ackStates.initialAckState));
   EXPECT_EQ(folly::none, largestAckToSend(conn.ackStates.handshakeAckState));
   EXPECT_EQ(folly::none, largestAckToSend(conn.ackStates.appDataAckState));
@@ -685,7 +700,8 @@ TEST_F(QuicPacketSchedulerTest, LargestAckToSend) {
 }
 
 TEST_F(QuicPacketSchedulerTest, StreamFrameSchedulerAllFit) {
-  QuicClientConnectionState conn;
+  QuicClientConnectionState conn(
+      std::make_shared<FizzClientQuicHandshakeContext>());
   conn.streamManager->setMaxLocalBidirectionalStreams(10);
   conn.flowControlState.peerAdvertisedMaxOffset = 100000;
   conn.flowControlState.peerAdvertisedInitialMaxStreamOffsetBidiRemote = 100000;
@@ -710,7 +726,8 @@ TEST_F(QuicPacketSchedulerTest, StreamFrameSchedulerAllFit) {
 }
 
 TEST_F(QuicPacketSchedulerTest, StreamFrameSchedulerRoundRobin) {
-  QuicClientConnectionState conn;
+  QuicClientConnectionState conn(
+      std::make_shared<FizzClientQuicHandshakeContext>());
   conn.streamManager->setMaxLocalBidirectionalStreams(10);
   conn.flowControlState.peerAdvertisedMaxOffset = 100000;
   conn.flowControlState.peerAdvertisedInitialMaxStreamOffsetBidiRemote = 100000;
@@ -827,7 +844,8 @@ TEST_F(QuicPacketSchedulerTest, StreamFrameSchedulerRoundRobinControl) {
 }
 
 TEST_F(QuicPacketSchedulerTest, StreamFrameSchedulerOneStream) {
-  QuicClientConnectionState conn;
+  QuicClientConnectionState conn(
+      std::make_shared<FizzClientQuicHandshakeContext>());
   conn.streamManager->setMaxLocalBidirectionalStreams(10);
   conn.flowControlState.peerAdvertisedMaxOffset = 100000;
   conn.flowControlState.peerAdvertisedInitialMaxStreamOffsetBidiRemote = 100000;
@@ -848,7 +866,8 @@ TEST_F(QuicPacketSchedulerTest, StreamFrameSchedulerOneStream) {
 }
 
 TEST_F(QuicPacketSchedulerTest, StreamFrameSchedulerRemoveOne) {
-  QuicClientConnectionState conn;
+  QuicClientConnectionState conn(
+      std::make_shared<FizzClientQuicHandshakeContext>());
   conn.streamManager->setMaxLocalBidirectionalStreams(10);
   conn.flowControlState.peerAdvertisedMaxOffset = 100000;
   conn.flowControlState.peerAdvertisedInitialMaxStreamOffsetBidiRemote = 100000;

quic/client/CMakeLists.txt

@@ -7,6 +7,8 @@ add_library(
   mvfst_client STATIC
   QuicClientTransport.cpp
   handshake/ClientHandshake.cpp
+  handshake/FizzClientQuicHandshakeContext.cpp
+  handshake/FizzClientHandshake.cpp
   state/ClientStateMachine.cpp
 )
 

quic/client/QuicClientTransport.cpp

@@ -12,6 +12,7 @@
 
 #include <quic/api/QuicTransportFunctions.h>
 #include <quic/client/handshake/ClientTransportParametersExtension.h>
+#include <quic/client/handshake/FizzClientQuicHandshakeContext.h>
 #include <quic/client/state/ClientStateMachine.h>
 #include <quic/flowcontrol/QuicFlowController.h>
 #include <quic/handshake/FizzCryptoFactory.h>
@@ -40,7 +41,8 @@ QuicClientTransport::QuicClientTransport(
       connectionIdSize == 0 ||
       (connectionIdSize >= kMinInitialDestinationConnIdLength &&
        connectionIdSize <= kMaxConnectionIdSize));
-  auto tempConn = std::make_unique<QuicClientConnectionState>();
+  auto tempConn = std::make_unique<QuicClientConnectionState>(
+      std::make_shared<FizzClientQuicHandshakeContext>());
   clientConn_ = tempConn.get();
   conn_ = std::move(tempConn);
   std::vector<uint8_t> connIdData(
@@ -1168,7 +1170,7 @@ void QuicClientTransport::setHostname(const std::string& hostname) {
   hostname_ = hostname;
 }
 
-void QuicClientTransport::setFizzClientContext(
+void QuicClientTransport::setFizzClientQuicHandshakeContext(
     std::shared_ptr<const fizz::client::FizzClientContext> ctx) {
   ctx_ = std::move(ctx);
 }

quic/client/QuicClientTransport.h

@@ -62,7 +62,7 @@ class QuicClientTransport
   /**
    * Set the client context for fizz. Must be set before start()
    */
-  void setFizzClientContext(
+  void setFizzClientQuicHandshakeContext(
       std::shared_ptr<const fizz::client::FizzClientContext> ctx);
 
   /**

quic/client/handshake/ClientHandshakeFactory.h

@@ -0,0 +1,32 @@
+/*
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ */
+
+#pragma once
+
+#include <memory>
+
+namespace quic {
+
+class ClientHandshake;
+struct QuicCryptoState;
+
+class ClientHandshakeFactory {
+ public:
+  virtual ~ClientHandshakeFactory() = default;
+
+  /**
+   * Construct a new client handshake.
+   * /!\ The ClientHandshake constructed might keep a reference to cryptoState.
+   * It is up to the caller to ensure the lifetime of cryptoState exceed the one
+   * of the ClientHandshake.
+   */
+  virtual std::unique_ptr<ClientHandshake> makeClientHandshake(
+      QuicCryptoState& cryptoState) = 0;
+};
+
+} // namespace quic

quic/client/handshake/FizzClientHandshake.cpp

@@ -0,0 +1,18 @@
+/*
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ */
+
+#include <quic/client/handshake/FizzClientHandshake.h>
+
+namespace quic {
+
+FizzClientHandshake::FizzClientHandshake(
+    QuicCryptoState& cryptoState,
+    std::shared_ptr<FizzClientQuicHandshakeContext> fizzContext)
+    : ClientHandshake(cryptoState), fizzContext_(std::move(fizzContext)) {}
+
+} // namespace quic

quic/client/handshake/FizzClientHandshake.h

@@ -0,0 +1,27 @@
+/*
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ */
+
+#pragma once
+
+#include <quic/client/handshake/ClientHandshake.h>
+
+namespace quic {
+
+class FizzClientQuicHandshakeContext;
+
+class FizzClientHandshake : public ClientHandshake {
+ public:
+  FizzClientHandshake(
+      QuicCryptoState& cryptoState,
+      std::shared_ptr<FizzClientQuicHandshakeContext> fizzContext);
+
+ private:
+  std::shared_ptr<FizzClientQuicHandshakeContext> fizzContext_;
+};
+
+} // namespace quic

quic/client/handshake/FizzClientQuicHandshakeContext.cpp

@@ -0,0 +1,21 @@
+/*
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ */
+
+#include <quic/client/handshake/FizzClientQuicHandshakeContext.h>
+
+#include <quic/client/handshake/FizzClientHandshake.h>
+
+namespace quic {
+
+std::unique_ptr<ClientHandshake>
+FizzClientQuicHandshakeContext::makeClientHandshake(
+    QuicCryptoState& cryptoState) {
+  return std::make_unique<FizzClientHandshake>(cryptoState, shared_from_this());
+}
+
+} // namespace quic

quic/client/handshake/FizzClientQuicHandshakeContext.h

@@ -0,0 +1,25 @@
+/*
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ */
+
+#pragma once
+
+#include <quic/client/handshake/ClientHandshakeFactory.h>
+
+namespace quic {
+
+class FizzClientHandshake;
+
+class FizzClientQuicHandshakeContext
+    : public ClientHandshakeFactory,
+      public std::enable_shared_from_this<FizzClientQuicHandshakeContext> {
+ public:
+  std::unique_ptr<ClientHandshake> makeClientHandshake(
+      QuicCryptoState& cryptoState) override;
+};
+
+} // namespace quic

quic/client/state/ClientStateMachine.cpp

@@ -22,7 +22,8 @@ std::unique_ptr<QuicClientConnectionState> undoAllClientStateCommon(
     std::unique_ptr<QuicClientConnectionState> conn) {
   // Create a new connection state and copy over properties that don't change
   // across stateless retry.
-  auto newConn = std::make_unique<QuicClientConnectionState>();
+  auto newConn = std::make_unique<QuicClientConnectionState>(
+      std::move(conn->handshakeFactory));
   newConn->qLogger = conn->qLogger;
   newConn->clientConnectionId = conn->clientConnectionId;
   newConn->initialDestinationConnectionId =

quic/client/state/ClientStateMachine.h

@@ -10,6 +10,7 @@
 
 #include <folly/io/async/AsyncSocketException.h>
 #include <quic/client/handshake/ClientHandshake.h>
+#include <quic/client/handshake/ClientHandshakeFactory.h>
 #include <quic/congestion_control/QuicCubic.h>
 #include <quic/flowcontrol/QuicFlowController.h>
 #include <quic/handshake/TransportParameters.h>
@@ -30,6 +31,7 @@ struct QuicClientConnectionState : public QuicConnectionStateBase {
   // Initial destination connection id.
   folly::Optional<ConnectionId> initialDestinationConnectionId;
 
+  std::shared_ptr<ClientHandshakeFactory> handshakeFactory;
   ClientHandshake* clientHandshakeLayer;
 
   // Save the server transport params here so that client can access the value
@@ -43,15 +45,21 @@ struct QuicClientConnectionState : public QuicConnectionStateBase {
   // TODO: use this to get rid of the data in the crypto stream.
   // folly::Optional<PacketNum> clientInitialPacketNum;
 
-  QuicClientConnectionState() : QuicConnectionStateBase(QuicNodeType::Client) {
+  explicit QuicClientConnectionState(
+      std::shared_ptr<ClientHandshakeFactory> handshakeFactoryIn)
+      : QuicConnectionStateBase(QuicNodeType::Client),
+        handshakeFactory(std::move(handshakeFactoryIn)) {
     cryptoState = std::make_unique<QuicCryptoState>();
     congestionController = std::make_unique<Cubic>(*this);
     // TODO: this is wrong, it should be the handshake finish time. But i need
     // a relatively sane time now to make the timestamps all sane.
     connectionTime = Clock::now();
     originalVersion = QuicVersion::MVFST;
-    clientHandshakeLayer = new ClientHandshake(*cryptoState);
-    handshakeLayer.reset(clientHandshakeLayer);
+    DCHECK(handshakeFactory);
+    auto tmpClientHandshake =
+        handshakeFactory->makeClientHandshake(*cryptoState);
+    clientHandshakeLayer = tmpClientHandshake.get();
+    handshakeLayer.reset(tmpClientHandshake.release());
     // We shouldn't normally need to set this until we're starting the
     // transport, however writing unit tests is much easier if we set this here.
     updateFlowControlStateWithSettings(flowControlState, transportSettings);

quic/client/test/QuicClientTransportTest.cpp

@@ -203,7 +203,7 @@ class QuicClientTransportIntegrationTest : public TestWithParam<TestingParams> {
     client->setCongestionControllerFactory(
         std::make_shared<DefaultCongestionControllerFactory>());
     client->setHostname(hostname);
-    client->setFizzClientContext(clientCtx);
+    client->setFizzClientQuicHandshakeContext(clientCtx);
     client->setCertificateVerifier(createTestCertificateVerifier());
     client->addNewPeerAddress(serverAddr);
     client->setPskCache(pskCache_);

quic/loss/test/QuicLossFunctionsTest.cpp

@@ -13,6 +13,7 @@
 #include <folly/io/async/test/MockTimeoutManager.h>
 #include <quic/api/QuicTransportFunctions.h>
 #include <quic/api/test/MockQuicStats.h>
+#include <quic/client/handshake/FizzClientQuicHandshakeContext.h>
 #include <quic/client/state/ClientStateMachine.h>
 #include <quic/codec/DefaultConnectionIdAlgo.h>
 #include <quic/common/test/TestUtils.h>
@@ -85,7 +86,8 @@ class QuicLossFunctionsTest : public TestWithParam<PacketNumberSpace> {
   }
 
   std::unique_ptr<QuicClientConnectionState> createClientConn() {
-    auto conn = std::make_unique<QuicClientConnectionState>();
+    auto conn = std::make_unique<QuicClientConnectionState>(
+        std::make_shared<FizzClientQuicHandshakeContext>());
     conn->clientConnectionId = getTestConnectionId();
     conn->version = QuicVersion::MVFST;
     conn->ackStates.initialAckState.nextPacketNum = 1;

quic/state/test/QuicStreamFunctionsTest.cpp

@@ -11,6 +11,7 @@
 
 #include <quic/state/QuicStreamFunctions.h>
 
+#include <quic/client/handshake/FizzClientQuicHandshakeContext.h>
 #include <quic/client/state/ClientStateMachine.h>
 #include <quic/common/test/TestUtils.h>
 #include <quic/server/state/ServerStateMachine.h>
@@ -27,6 +28,9 @@ using PeekIterator = std::deque<StreamBuffer>::const_iterator;
 
 class QuicStreamFunctionsTest : public Test {
  public:
+  QuicStreamFunctionsTest()
+      : conn(std::make_shared<FizzClientQuicHandshakeContext>()) {}
+
   void SetUp() override {
     conn.flowControlState.peerAdvertisedInitialMaxStreamOffsetBidiLocal =
         kDefaultStreamWindowSize;
@@ -1083,7 +1087,8 @@ TEST_F(QuicStreamFunctionsTest, IsBidirectionalStream) {
 }
 
 TEST_F(QuicStreamFunctionsTest, IsSendingStream) {
-  QuicClientConnectionState clientState;
+  QuicClientConnectionState clientState(
+      std::make_shared<FizzClientQuicHandshakeContext>());
   QuicServerConnectionState serverState;
   QuicNodeType nodeType;
   StreamId id;
@@ -1110,7 +1115,8 @@ TEST_F(QuicStreamFunctionsTest, IsSendingStream) {
 }
 
 TEST_F(QuicStreamFunctionsTest, IsReceivingStream) {
-  QuicClientConnectionState clientState;
+  QuicClientConnectionState clientState(
+      std::make_shared<FizzClientQuicHandshakeContext>());
   QuicServerConnectionState serverState;
   QuicNodeType nodeType;
   StreamId id;