facebook · deadalnix · Dec 7, 2019
diff --git a/quic/client/handshake/ClientHandshake.h b/quic/client/handshake/ClientHandshake.h
@@ -46,8 +46,7 @@ class ClientHandshake : public Handshake {
   virtual void connect(
       folly::Optional<std::string> hostname,
       folly::Optional<fizz::client::CachedPsk> cachedPsk,
-      const std::shared_ptr<ClientTransportParametersExtension>&
-          transportParams,
+      std::shared_ptr<ClientTransportParametersExtension> transportParams,
       HandshakeCallback* callback) = 0;
 
   /**

diff --git a/quic/client/handshake/ClientTransportParametersExtension.h b/quic/client/handshake/ClientTransportParametersExtension.h
@@ -8,13 +8,11 @@
 
 #pragma once
 
-#include <fizz/client/ClientExtensions.h>
 #include <quic/handshake/FizzTransportParameters.h>
 
 namespace quic {
 
-class ClientTransportParametersExtension : public fizz::ClientExtensions {
- public:
+struct ClientTransportParametersExtension {
   ClientTransportParametersExtension(
       folly::Optional<QuicVersion> initialVersion,
       uint64_t initialMaxData,
@@ -38,64 +36,10 @@ class ClientTransportParametersExtension : public fizz::ClientExtensions {
         activeConnectionLimit_(activeConnectionIdLimit),
         customTransportParameters_(customTransportParameters) {}
 
-  ~ClientTransportParametersExtension() override = default;
-
-  std::vector<fizz::Extension> getClientHelloExtensions() const override {
-    std::vector<fizz::Extension> exts;
-
-    ClientTransportParameters params;
-    params.initial_version = initialVersion_;
-    params.parameters.push_back(encodeIntegerParameter(
-        TransportParameterId::initial_max_stream_data_bidi_local,
-        initialMaxStreamDataBidiLocal_));
-    params.parameters.push_back(encodeIntegerParameter(
-        TransportParameterId::initial_max_stream_data_bidi_remote,
-        initialMaxStreamDataBidiRemote_));
-    params.parameters.push_back(encodeIntegerParameter(
-        TransportParameterId::initial_max_stream_data_uni,
-        initialMaxStreamDataUni_));
-    params.parameters.push_back(encodeIntegerParameter(
-        TransportParameterId::initial_max_data, initialMaxData_));
-    params.parameters.push_back(encodeIntegerParameter(
-        TransportParameterId::initial_max_streams_bidi,
-        std::numeric_limits<uint32_t>::max()));
-    params.parameters.push_back(encodeIntegerParameter(
-        TransportParameterId::initial_max_streams_uni,
-        std::numeric_limits<uint32_t>::max()));
-    params.parameters.push_back(encodeIntegerParameter(
-        TransportParameterId::idle_timeout, idleTimeout_.count()));
-    params.parameters.push_back(encodeIntegerParameter(
-        TransportParameterId::ack_delay_exponent, ackDelayExponent_));
-    params.parameters.push_back(encodeIntegerParameter(
-        TransportParameterId::max_packet_size, maxRecvPacketSize_));
-    params.parameters.push_back(encodeIntegerParameter(
-        TransportParameterId::active_connection_id_limit,
-        activeConnectionLimit_));
-
-    for (const auto& customParameter : customTransportParameters_) {
-      params.parameters.push_back(customParameter);
-    }
-
-    exts.push_back(encodeExtension(params));
-    return exts;
-  }
-
-  void onEncryptedExtensions(
-      const std::vector<fizz::Extension>& exts) override {
-    auto serverParams = fizz::getExtension<ServerTransportParameters>(exts);
-    if (!serverParams) {
-      throw fizz::FizzException(
-          "missing server quic transport parameters extension",
-          fizz::AlertDescription::missing_extension);
-    }
-    serverTransportParameters_ = std::move(serverParams);
-  }
-
   folly::Optional<ServerTransportParameters> getServerTransportParams() {
     return std::move(serverTransportParameters_);
   }
 
- private:
   folly::Optional<QuicVersion> initialVersion_;
   uint64_t initialMaxData_;
   uint64_t initialMaxStreamDataBidiLocal_;

diff --git a/quic/client/handshake/FizzClientExtensions.h b/quic/client/handshake/FizzClientExtensions.h
@@ -0,0 +1,84 @@
+/*
+ * 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 <fizz/client/ClientExtensions.h>
+#include <quic/client/handshake/ClientTransportParametersExtension.h>
+#include <quic/handshake/FizzTransportParameters.h>
+
+namespace quic {
+
+class FizzClientExtensions : public fizz::ClientExtensions {
+ public:
+  FizzClientExtensions(
+      std::shared_ptr<ClientTransportParametersExtension> clientParameters)
+      : clientParameters_(std::move(clientParameters)) {}
+
+  ~FizzClientExtensions() override = default;
+
+  std::vector<fizz::Extension> getClientHelloExtensions() const override {
+    std::vector<fizz::Extension> exts;
+
+    ClientTransportParameters params;
+    params.initial_version = clientParameters_->initialVersion_;
+    params.parameters.push_back(encodeIntegerParameter(
+        TransportParameterId::initial_max_stream_data_bidi_local,
+        clientParameters_->initialMaxStreamDataBidiLocal_));
+    params.parameters.push_back(encodeIntegerParameter(
+        TransportParameterId::initial_max_stream_data_bidi_remote,
+        clientParameters_->initialMaxStreamDataBidiRemote_));
+    params.parameters.push_back(encodeIntegerParameter(
+        TransportParameterId::initial_max_stream_data_uni,
+        clientParameters_->initialMaxStreamDataUni_));
+    params.parameters.push_back(encodeIntegerParameter(
+        TransportParameterId::initial_max_data,
+        clientParameters_->initialMaxData_));
+    params.parameters.push_back(encodeIntegerParameter(
+        TransportParameterId::initial_max_streams_bidi,
+        std::numeric_limits<uint32_t>::max()));
+    params.parameters.push_back(encodeIntegerParameter(
+        TransportParameterId::initial_max_streams_uni,
+        std::numeric_limits<uint32_t>::max()));
+    params.parameters.push_back(encodeIntegerParameter(
+        TransportParameterId::idle_timeout,
+        clientParameters_->idleTimeout_.count()));
+    params.parameters.push_back(encodeIntegerParameter(
+        TransportParameterId::ack_delay_exponent,
+        clientParameters_->ackDelayExponent_));
+    params.parameters.push_back(encodeIntegerParameter(
+        TransportParameterId::max_packet_size,
+        clientParameters_->maxRecvPacketSize_));
+    params.parameters.push_back(encodeIntegerParameter(
+        TransportParameterId::active_connection_id_limit,
+        clientParameters_->activeConnectionLimit_));
+
+    for (const auto& customParameter :
+         clientParameters_->customTransportParameters_) {
+      params.parameters.push_back(customParameter);
+    }
+
+    exts.push_back(encodeExtension(params));
+    return exts;
+  }
+
+  void onEncryptedExtensions(
+      const std::vector<fizz::Extension>& exts) override {
+    auto serverParams = fizz::getExtension<ServerTransportParameters>(exts);
+    if (!serverParams) {
+      throw fizz::FizzException(
+          "missing server quic transport parameters extension",
+          fizz::AlertDescription::missing_extension);
+    }
+    clientParameters_->serverTransportParameters_ = std::move(serverParams);
+  }
+
+ private:
+  std::shared_ptr<ClientTransportParametersExtension> clientParameters_;
+};
+} // namespace quic
diff --git a/quic/client/handshake/FizzClientHandshake.cpp b/quic/client/handshake/FizzClientHandshake.cpp
@@ -9,6 +9,7 @@
 #include <quic/client/handshake/FizzClientHandshake.h>
 
 #include <folly/Overload.h>
+#include <quic/client/handshake/FizzClientExtensions.h>
 #include <quic/client/handshake/FizzClientQuicHandshakeContext.h>
 #include <quic/handshake/FizzBridge.h>
 
@@ -24,7 +25,7 @@ FizzClientHandshake::FizzClientHandshake(
 void FizzClientHandshake::connect(
     folly::Optional<std::string> hostname,
     folly::Optional<fizz::client::CachedPsk> cachedPsk,
-    const std::shared_ptr<ClientTransportParametersExtension>& transportParams,
+    std::shared_ptr<ClientTransportParametersExtension> transportParams,
     HandshakeCallback* callback) {
   transportParams_ = transportParams;
   callback_ = callback;
@@ -43,7 +44,7 @@ void FizzClientHandshake::connect(
       fizzContext_->getCertificateVerifier(),
       std::move(hostname),
       std::move(cachedPsk),
-      transportParams));
+      std::make_shared<FizzClientExtensions>(std::move(transportParams))));
 }
 
 const CryptoFactory& FizzClientHandshake::getCryptoFactory() const {

diff --git a/quic/client/handshake/FizzClientHandshake.h b/quic/client/handshake/FizzClientHandshake.h
@@ -24,8 +24,7 @@ class FizzClientHandshake : public ClientHandshake {
   void connect(
       folly::Optional<std::string> hostname,
       folly::Optional<fizz::client::CachedPsk> cachedPsk,
-      const std::shared_ptr<ClientTransportParametersExtension>&
-          transportParams,
+      std::shared_ptr<ClientTransportParametersExtension> transportParams,
       HandshakeCallback* callback) override;
 
   const CryptoFactory& getCryptoFactory() const override;

diff --git a/quic/client/handshake/test/CMakeLists.txt b/quic/client/handshake/test/CMakeLists.txt
@@ -10,7 +10,7 @@ endif()
 quic_add_test(TARGET ClientHandshakeTest
   SOURCES
   ClientHandshakeTest.cpp
-  ClientTransportParametersTest.cpp
+  FizzClientExtensionsTest.cpp
   DEPENDS
   Folly::folly
   ${LIBFIZZ_LIBRARY}

diff --git a/...ke/test/ClientTransportParametersTest.cpp → ...ndshake/test/FizzClientExtensionsTest.cpp b/...ke/test/ClientTransportParametersTest.cpp → ...ndshake/test/FizzClientExtensionsTest.cpp
@@ -8,7 +8,7 @@
 
 #include <gtest/gtest.h>
 
-#include <quic/client/handshake/ClientTransportParametersExtension.h>
+#include <quic/client/handshake/FizzClientExtensions.h>
 #include <quic/common/test/TestUtils.h>
 
 #include <fizz/protocol/test/TestMessages.h>
@@ -30,8 +30,8 @@ static EncryptedExtensions getEncryptedExtensions() {
   return ee;
 }
 
-TEST(ClientTransportParametersTest, TestGetChloExtensions) {
-  ClientTransportParametersExtension ext(
+TEST(FizzClientHandshakeTest, TestGetChloExtensions) {
+  FizzClientExtensions ext(std::make_shared<ClientTransportParametersExtension>(
       folly::none,
       kDefaultConnectionWindowSize,
       kDefaultStreamWindowSize,
@@ -40,16 +40,16 @@ TEST(ClientTransportParametersTest, TestGetChloExtensions) {
       kDefaultIdleTimeout,
       kDefaultAckDelayExponent,
       kDefaultUDPSendPacketLen,
-      kDefaultActiveConnectionIdLimit);
+      kDefaultActiveConnectionIdLimit));
   auto extensions = ext.getClientHelloExtensions();
 
   EXPECT_EQ(extensions.size(), 1);
   auto serverParams = getExtension<ClientTransportParameters>(extensions);
   EXPECT_TRUE(serverParams.hasValue());
 }
 
-TEST(ClientTransportParametersTest, TestOnEE) {
-  ClientTransportParametersExtension ext(
+TEST(FizzClientHandshakeTest, TestOnEE) {
+  FizzClientExtensions ext(std::make_shared<ClientTransportParametersExtension>(
       MVFST1,
       kDefaultConnectionWindowSize,
       kDefaultStreamWindowSize,
@@ -58,13 +58,13 @@ TEST(ClientTransportParametersTest, TestOnEE) {
       kDefaultIdleTimeout,
       kDefaultAckDelayExponent,
       kDefaultUDPSendPacketLen,
-      kDefaultActiveConnectionIdLimit);
+      kDefaultActiveConnectionIdLimit));
   ext.getClientHelloExtensions();
   ext.onEncryptedExtensions(getEncryptedExtensions().extensions);
 }
 
-TEST(ClientTransportParametersTest, TestOnEEMissingServerParams) {
-  ClientTransportParametersExtension ext(
+TEST(FizzClientHandshakeTest, TestOnEEMissingServerParams) {
+  FizzClientExtensions ext(std::make_shared<ClientTransportParametersExtension>(
       MVFST1,
       kDefaultConnectionWindowSize,
       kDefaultStreamWindowSize,
@@ -73,14 +73,14 @@ TEST(ClientTransportParametersTest, TestOnEEMissingServerParams) {
       kDefaultIdleTimeout,
       kDefaultAckDelayExponent,
       kDefaultUDPSendPacketLen,
-      kDefaultActiveConnectionIdLimit);
+      kDefaultActiveConnectionIdLimit));
   ext.getClientHelloExtensions();
   EXPECT_THROW(
       ext.onEncryptedExtensions(TestMessages::encryptedExt().extensions),
       FizzException);
 }
 
-TEST(ClientTransportParametersTest, TestGetChloExtensionsCustomParams) {
+TEST(FizzClientHandshakeTest, TestGetChloExtensionsCustomParams) {
   std::vector<TransportParameter> customTransportParameters;
 
   std::string randomBytes = "\x01\x00\x55\x12\xff";
@@ -99,7 +99,7 @@ TEST(ClientTransportParametersTest, TestGetChloExtensionsCustomParams) {
   customTransportParameters.push_back(element2->encode());
   customTransportParameters.push_back(element3->encode());
 
-  ClientTransportParametersExtension ext(
+  FizzClientExtensions ext(std::make_shared<ClientTransportParametersExtension>(
       folly::none,
       kDefaultConnectionWindowSize,
       kDefaultStreamWindowSize,
@@ -109,7 +109,7 @@ TEST(ClientTransportParametersTest, TestGetChloExtensionsCustomParams) {
       kDefaultAckDelayExponent,
       kDefaultUDPSendPacketLen,
       kDefaultActiveConnectionIdLimit,
-      customTransportParameters);
+      customTransportParameters));
   auto extensions = ext.getClientHelloExtensions();
 
   EXPECT_EQ(extensions.size(), 1);
@@ -155,5 +155,5 @@ TEST(ClientTransportParametersTest, TestGetChloExtensionsCustomParams) {
 
   EXPECT_TRUE(eq(folly::IOBuf::copyBuffer(randomBytes), it3->value));
 }
-}
-}
+} // namespace test
+} // namespace quic
diff --git a/quic/client/test/QuicClientTransportTest.cpp b/quic/client/test/QuicClientTransportTest.cpp
@@ -1119,7 +1119,7 @@ class FakeOneRttHandshakeLayer : public ClientHandshake {
   void connect(
       folly::Optional<std::string>,
       folly::Optional<fizz::client::CachedPsk>,
-      const std::shared_ptr<ClientTransportParametersExtension>&,
+      std::shared_ptr<ClientTransportParametersExtension>,
       HandshakeCallback* callback) override {
     connected_ = true;
     writeDataToQuicStream(

diff --git a/quic/server/handshake/test/ServerHandshakeTest.cpp b/quic/server/handshake/test/ServerHandshakeTest.cpp
@@ -24,7 +24,7 @@
 #include <folly/ssl/Init.h>
 
 #include <quic/QuicConstants.h>
-#include <quic/client/handshake/ClientTransportParametersExtension.h>
+#include <quic/client/handshake/FizzClientExtensions.h>
 #include <quic/common/test/TestUtils.h>
 #include <quic/handshake/FizzBridge.h>
 #include <quic/handshake/HandshakeLayer.h>
@@ -151,7 +151,11 @@ class ServerHandshakeTest : public Test {
         }));
     auto cachedPsk = clientCtx->getPsk(hostname);
     fizzClient->connect(
-        clientCtx, verifier, hostname, cachedPsk, clientExtensions);
+        clientCtx,
+        verifier,
+        hostname,
+        cachedPsk,
+        std::make_shared<FizzClientExtensions>(clientExtensions));
   }
 
   void clientServerRound() {