diff --git a/quic/api/QuicTransportFunctions.cpp b/quic/api/QuicTransportFunctions.cpp index 32c8e2c1d..6e35a019b 100644 --- a/quic/api/QuicTransportFunctions.cpp +++ b/quic/api/QuicTransportFunctions.cpp @@ -1788,4 +1788,34 @@ bool hasInitialOrHandshakeCiphers(QuicConnectionStateBase& conn) { conn.readCodec->getHandshakeReadCipher(); } +bool setCustomTransportParameter( + std::unique_ptr customParam, + std::vector& customTransportParameters) { + // Check that the parameter id is in the "private parameter" range, as + // described by the spec. + if (static_cast(customParam->getParameterId()) < + kCustomTransportParameterThreshold) { + LOG(ERROR) << "invalid parameter id"; + return false; + } + + // check to see that we haven't already added in a parameter with the + // specified parameter id + auto it = std::find_if( + customTransportParameters.begin(), + customTransportParameters.end(), + [&customParam](const TransportParameter& param) { + return param.parameter == customParam->getParameterId(); + }); + + // if a match has been found, we return failure + if (it != customTransportParameters.end()) { + LOG(ERROR) << "transport parameter already present"; + return false; + } + + customTransportParameters.push_back(customParam->encode()); + return true; +} + } // namespace quic diff --git a/quic/api/QuicTransportFunctions.h b/quic/api/QuicTransportFunctions.h index 8cb322b87..6625c185d 100644 --- a/quic/api/QuicTransportFunctions.h +++ b/quic/api/QuicTransportFunctions.h @@ -14,6 +14,7 @@ #include #include #include +#include #include // Function to schedule writing data to socket. Return number of packets @@ -333,4 +334,14 @@ bool writeLoopTimeLimit( TimePoint loopBeginTime, const QuicConnectionStateBase& connection); +/** + * Sets private transport parameters that are not in the TransportParameterId + * enum. See kCustomTransportParameterThreshold in QuicConstants.h + * - checks that the custom param id complies to the spec + * - checks that the param is not in the provided vector already + */ +bool setCustomTransportParameter( + std::unique_ptr customParam, + std::vector& customTransportParameters); + } // namespace quic diff --git a/quic/api/test/QuicTransportFunctionsTest.cpp b/quic/api/test/QuicTransportFunctionsTest.cpp index d3617053e..399d8a560 100644 --- a/quic/api/test/QuicTransportFunctionsTest.cpp +++ b/quic/api/test/QuicTransportFunctionsTest.cpp @@ -4523,5 +4523,37 @@ TEST_F(QuicTransportFunctionsTest, MissingStreamFrameBytesSingleByteWrite) { } } +TEST_F(QuicTransportFunctionsTest, CustomTransportParamTest) { + std::vector customTransportParameters; + + // Add new param. + EXPECT_TRUE(setCustomTransportParameter( + std::make_unique( + kCustomTransportParameterThreshold, 0), + customTransportParameters)); + EXPECT_EQ(customTransportParameters.size(), 1); + + // Existing param not added. + EXPECT_FALSE(setCustomTransportParameter( + std::make_unique( + kCustomTransportParameterThreshold, 1), + customTransportParameters)); + EXPECT_EQ(customTransportParameters.size(), 1); + + // Bad param id is not added. + EXPECT_FALSE(setCustomTransportParameter( + std::make_unique( + kCustomTransportParameterThreshold - 1, 2), + customTransportParameters)); + EXPECT_EQ(customTransportParameters.size(), 1); + + // Another valid param added. + EXPECT_TRUE(setCustomTransportParameter( + std::make_unique( + kCustomTransportParameterThreshold + 1, 0), + customTransportParameters)); + EXPECT_EQ(customTransportParameters.size(), 2); +} + } // namespace test } // namespace quic diff --git a/quic/client/QuicClientTransport.cpp b/quic/client/QuicClientTransport.cpp index 787930675..f61aa9de4 100644 --- a/quic/client/QuicClientTransport.cpp +++ b/quic/client/QuicClientTransport.cpp @@ -1640,35 +1640,6 @@ void QuicClientTransport::setSelfOwning() { selfOwning_ = shared_from_this(); } -bool QuicClientTransport::setCustomTransportParameter( - std::unique_ptr customParam) { - // check that the parameter id is in the "private parameter" range, as - // described by the spec. - if (static_cast(customParam->getParameterId()) < - kCustomTransportParameterThreshold) { - LOG(ERROR) << "invalid parameter id"; - return false; - } - - // check to see that we haven't already added in a parameter with the - // specified parameter id - auto it = std::find_if( - customTransportParameters_.begin(), - customTransportParameters_.end(), - [&customParam](const TransportParameter& param) { - return param.parameter == customParam->getParameterId(); - }); - - // if a match has been found, we return failure - if (it != customTransportParameters_.end()) { - LOG(ERROR) << "transport parameter already present"; - return false; - } - - customTransportParameters_.push_back(customParam->encode()); - return true; -} - void QuicClientTransport::setD6DBasePMTUTransportParameter() { if (!conn_->transportSettings.d6dConfig.enabled) { return; @@ -1687,7 +1658,8 @@ void QuicClientTransport::setD6DBasePMTUTransportParameter() { auto basePMTUCustomParam = std::make_unique( kD6DBasePMTUParameterId, basePMTUSetting); - if (!setCustomTransportParameter(std::move(basePMTUCustomParam))) { + if (!setCustomTransportParameter( + std::move(basePMTUCustomParam), customTransportParameters_)) { LOG(ERROR) << "failed to set D6D base PMTU transport parameter"; } } @@ -1710,7 +1682,8 @@ void QuicClientTransport::setD6DRaiseTimeoutTransportParameter() { std::make_unique( kD6DRaiseTimeoutParameterId, raiseTimeoutSetting.count()); - if (!setCustomTransportParameter(std::move(raiseTimeoutCustomParam))) { + if (!setCustomTransportParameter( + std::move(raiseTimeoutCustomParam), customTransportParameters_)) { LOG(ERROR) << "failed to set D6D raise timeout transport parameter"; } } @@ -1733,7 +1706,8 @@ void QuicClientTransport::setD6DProbeTimeoutTransportParameter() { std::make_unique( kD6DProbeTimeoutParameterId, probeTimeoutSetting.count()); - if (!setCustomTransportParameter(std::move(probeTimeoutCustomParam))) { + if (!setCustomTransportParameter( + std::move(probeTimeoutCustomParam), customTransportParameters_)) { LOG(ERROR) << "failed to set D6D probe timeout transport parameter"; } } diff --git a/quic/client/QuicClientTransport.h b/quic/client/QuicClientTransport.h index fce6b3ff4..f1465747c 100644 --- a/quic/client/QuicClientTransport.h +++ b/quic/client/QuicClientTransport.h @@ -138,14 +138,6 @@ class QuicClientTransport */ void setSelfOwning(); - /** - * Used to set private transport parameters that are not in the - * TransportParameterId enum. - * See kCustomTransportParameterThreshold in QuicConstants.h - */ - bool setCustomTransportParameter( - std::unique_ptr customParam); - void onNetworkSwitch(std::unique_ptr newSock) override; /** diff --git a/quic/fizz/client/test/QuicClientTransportTest.cpp b/quic/fizz/client/test/QuicClientTransportTest.cpp index e11eae246..47c067134 100644 --- a/quic/fizz/client/test/QuicClientTransportTest.cpp +++ b/quic/fizz/client/test/QuicClientTransportTest.cpp @@ -1053,13 +1053,6 @@ TEST_F(QuicClientTransportTest, FirstPacketProcessedCallback) { client->closeNow(folly::none); } -TEST_F(QuicClientTransportTest, CustomTransportParam) { - EXPECT_TRUE(client->setCustomTransportParameter( - std::make_unique( - kCustomTransportParameterThreshold, 0))); - client->closeNow(folly::none); -} - TEST_F(QuicClientTransportTest, CloseSocketOnWriteError) { client->addNewPeerAddress(serverAddr); EXPECT_CALL(*sock, write(_, _)).WillOnce(SetErrnoAndReturn(EBADF, -1));