Skip to content

Commit

Permalink
remove QuicVersion::QUIC_DRAFT
Browse files Browse the repository at this point in the history
Summary: - as title, deprecate QuicVersion::QUIC_DRAFT

Reviewed By: jbeshay

Differential Revision: D50466992

fbshipit-source-id: 2b2776d142142bc07fd27053a68dd635959a7f31
  • Loading branch information
hanidamlaj authored and facebook-github-bot committed Nov 10, 2023
1 parent dd7db53 commit 4eefa3b
Show file tree
Hide file tree
Showing 24 changed files with 50 additions and 122 deletions.
1 change: 0 additions & 1 deletion quic/QuicConstants.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@ std::vector<QuicVersion> filterSupportedVersions(
return version == QuicVersion::MVFST ||
version == QuicVersion::QUIC_V1 ||
version == QuicVersion::QUIC_V1_ALIAS ||
version == QuicVersion::QUIC_DRAFT ||
version == QuicVersion::MVFST_INVALID ||
version == QuicVersion::MVFST_EXPERIMENTAL ||
version == QuicVersion::MVFST_ALIAS;
Expand Down
1 change: 0 additions & 1 deletion quic/QuicConstants.h
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,6 @@ enum class QuicVersion : uint32_t {
// QuicTransportBase::isKnobSupported() and make sure that knob support is not
// broken.
MVFST = 0xfaceb002,
QUIC_DRAFT = 0xff00001d, // Draft-29
QUIC_V1 = 0x00000001,
QUIC_V1_ALIAS = 0xfaceb003,
// MVFST_EXPERIMENTAL used to set initial congestion window
Expand Down
6 changes: 3 additions & 3 deletions quic/api/test/QuicPacketSchedulerTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ PacketNum addInitialOutstandingPacket(QuicConnectionStateBase& conn) {
srcConnId,
conn.clientConnectionId.value_or(quic::test::getTestConnectionId()),
nextPacketNum,
QuicVersion::QUIC_DRAFT);
QuicVersion::MVFST);
RegularQuicWritePacket packet(std::move(header));
conn.outstandings.packets.emplace_back(
packet,
Expand Down Expand Up @@ -68,7 +68,7 @@ PacketNum addHandshakeOutstandingPacket(QuicConnectionStateBase& conn) {
srcConnId,
conn.clientConnectionId.value_or(quic::test::getTestConnectionId()),
nextPacketNum,
QuicVersion::QUIC_DRAFT);
QuicVersion::MVFST);
RegularQuicWritePacket packet(std::move(header));
conn.outstandings.packets.emplace_back(
packet,
Expand Down Expand Up @@ -697,7 +697,7 @@ TEST_F(QuicPacketSchedulerTest, CloneSchedulerHasHandshakeDataAndAcks) {
srcConnId,
conn.clientConnectionId.value_or(quic::test::getTestConnectionId()),
nextPacketNum,
QuicVersion::QUIC_DRAFT);
QuicVersion::MVFST);
RegularQuicPacketBuilder builder(
conn.udpSendPacketLen,
std::move(header),
Expand Down
3 changes: 1 addition & 2 deletions quic/client/state/ClientStateMachine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -151,8 +151,7 @@ void processServerInitialParams(
static_cast<TransportParameterId>(
TransportParameterId::knob_frames_supported),
serverParams.parameters);
if (conn.version == QuicVersion::QUIC_DRAFT ||
conn.version == QuicVersion::QUIC_V1 ||
if (conn.version == QuicVersion::QUIC_V1 ||
conn.version == QuicVersion::QUIC_V1_ALIAS) {
auto initialSourceConnId = getConnIdParameter(
TransportParameterId::initial_source_connection_id,
Expand Down
2 changes: 0 additions & 2 deletions quic/codec/Types.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -463,8 +463,6 @@ std::string toString(QuicVersion version) {
return "QUIC_V1";
case QuicVersion::QUIC_V1_ALIAS:
return "QUIC_V1_ALIAS";
case QuicVersion::QUIC_DRAFT:
return "QUIC_DRAFT";
case QuicVersion::MVFST_EXPERIMENTAL:
return "MVFST_EXPERIMENTAL";
case QuicVersion::MVFST_ALIAS:
Expand Down
14 changes: 7 additions & 7 deletions quic/codec/test/TypesTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,14 +49,14 @@ folly::Expected<ParsedLongHeaderResult, TransportErrorCode> makeLongHeader(
getTestConnectionId(),
getTestConnectionId(),
321,
QuicVersion::QUIC_DRAFT);
QuicVersion::MVFST);

LongHeader headerRetry(
packetType,
getTestConnectionId(),
getTestConnectionId(),
321,
QuicVersion::QUIC_DRAFT,
QuicVersion::MVFST,
std::string("this is a retry token :)"));

RegularQuicPacketBuilder builder(
Expand Down Expand Up @@ -213,7 +213,7 @@ TEST_F(TypesTest, LongHeaderPacketNumberSpace) {
getTestConnectionId(0),
getTestConnectionId(1),
200,
QuicVersion::QUIC_DRAFT);
QuicVersion::MVFST);
EXPECT_EQ(
PacketNumberSpace::Initial, initialLongHeader.getPacketNumberSpace());
EXPECT_EQ(
Expand All @@ -225,7 +225,7 @@ TEST_F(TypesTest, LongHeaderPacketNumberSpace) {
getTestConnectionId(2),
getTestConnectionId(3),
201,
QuicVersion::QUIC_DRAFT);
QuicVersion::MVFST);
EXPECT_EQ(PacketNumberSpace::Initial, retryLongHeader.getPacketNumberSpace());
EXPECT_EQ(
PacketNumberSpace::Initial,
Expand All @@ -236,7 +236,7 @@ TEST_F(TypesTest, LongHeaderPacketNumberSpace) {
getTestConnectionId(4),
getTestConnectionId(5),
202,
QuicVersion::QUIC_DRAFT);
QuicVersion::MVFST);
EXPECT_EQ(
PacketNumberSpace::Handshake, handshakeLongHeader.getPacketNumberSpace());
EXPECT_EQ(
Expand All @@ -249,7 +249,7 @@ TEST_F(TypesTest, LongHeaderPacketNumberSpace) {
getTestConnectionId(6),
getTestConnectionId(7),
203,
QuicVersion::QUIC_DRAFT);
QuicVersion::MVFST);
EXPECT_EQ(
PacketNumberSpace::AppData, zeroRttLongHeader.getPacketNumberSpace());
EXPECT_EQ(
Expand All @@ -266,7 +266,7 @@ TEST_F(PacketHeaderTest, LongHeader) {
getTestConnectionId(4),
getTestConnectionId(5),
packetNumber,
QuicVersion::QUIC_DRAFT);
QuicVersion::MVFST);
PacketHeader readHeader(std::move(handshakeLongHeader));
EXPECT_NE(readHeader.asLong(), nullptr);
EXPECT_EQ(readHeader.asShort(), nullptr);
Expand Down
4 changes: 2 additions & 2 deletions quic/common/test/TestUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -605,14 +605,14 @@ RegularQuicWritePacket createNewPacket(
getTestConnectionId(1),
getTestConnectionId(2),
packetNum,
QuicVersion::QUIC_DRAFT));
QuicVersion::MVFST));
case PacketNumberSpace::Handshake:
return RegularQuicWritePacket(LongHeader(
LongHeader::Types::Handshake,
getTestConnectionId(0),
getTestConnectionId(4),
packetNum,
QuicVersion::QUIC_DRAFT));
QuicVersion::MVFST));
case PacketNumberSpace::AppData:
return RegularQuicWritePacket(ShortHeader(
ProtectionType::KeyPhaseOne, getTestConnectionId(), packetNum));
Expand Down
3 changes: 1 addition & 2 deletions quic/fizz/client/handshake/FizzClientExtensions.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,7 @@ class FizzClientExtensions : public fizz::ClientExtensions {
params.parameters.push_back(encodeIntegerParameter(
TransportParameterId::active_connection_id_limit,
clientParameters_->activeConnectionLimit_));
if (clientParameters_->encodingVersion_ == QuicVersion::QUIC_DRAFT ||
clientParameters_->encodingVersion_ == QuicVersion::QUIC_V1 ||
if (clientParameters_->encodingVersion_ == QuicVersion::QUIC_V1 ||
clientParameters_->encodingVersion_ == QuicVersion::QUIC_V1_ALIAS) {
params.parameters.push_back(encodeConnIdParameter(
TransportParameterId::initial_source_connection_id,
Expand Down
25 changes: 1 addition & 24 deletions quic/fizz/client/handshake/test/FizzClientExtensionsTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,28 +48,6 @@ TEST(FizzClientHandshakeTest, TestGetChloExtensionsMvfst) {
EXPECT_EQ(clientParams->parameters.size(), 10);
}

TEST(FizzClientHandshakeTest, TestGetChloExtensions) {
FizzClientExtensions ext(std::make_shared<ClientTransportParametersExtension>(
QuicVersion::QUIC_DRAFT,
kDefaultConnectionFlowControlWindow,
kDefaultStreamFlowControlWindow,
kDefaultStreamFlowControlWindow,
kDefaultStreamFlowControlWindow,
kDefaultMaxStreamsBidirectional,
kDefaultMaxStreamsUnidirectional,
kDefaultIdleTimeout,
kDefaultAckDelayExponent,
kDefaultUDPSendPacketLen,
kDefaultActiveConnectionIdLimit,
ConnectionId(std::vector<uint8_t>())));
auto extensions = ext.getClientHelloExtensions();

EXPECT_EQ(extensions.size(), 1);
auto clientParams = getClientExtension(extensions, QuicVersion::QUIC_DRAFT);
ASSERT_TRUE(clientParams.has_value());
EXPECT_EQ(clientParams->parameters.size(), 11);
}

TEST(FizzClientHandshakeTest, TestGetChloExtensionsV1) {
FizzClientExtensions ext(std::make_shared<ClientTransportParametersExtension>(
QuicVersion::QUIC_V1,
Expand Down Expand Up @@ -151,8 +129,7 @@ TEST(FizzClientHandshakeTest, TestV1RejectExtensionNumberMismatch) {

auto ee = TestMessages::encryptedExt();
ServerTransportParameters serverParams;
ee.extensions.push_back(
encodeExtension(serverParams, QuicVersion::QUIC_DRAFT));
ee.extensions.push_back(encodeExtension(serverParams, QuicVersion::MVFST));

EXPECT_THROW(ext.onEncryptedExtensions(ee.extensions), FizzException);

Expand Down
7 changes: 2 additions & 5 deletions quic/fizz/client/test/QuicClientTransportTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -979,9 +979,7 @@ INSTANTIATE_TEST_SUITE_P(
TestingParams(QuicVersion::QUIC_V1),
TestingParams(QuicVersion::QUIC_V1, 0),
TestingParams(QuicVersion::QUIC_V1_ALIAS),
TestingParams(QuicVersion::QUIC_V1_ALIAS, 0),
TestingParams(QuicVersion::QUIC_DRAFT),
TestingParams(QuicVersion::QUIC_DRAFT, 0)));
TestingParams(QuicVersion::QUIC_V1_ALIAS, 0)));

class QuicClientTransportTest : public QuicClientTransportTestBase {
public:
Expand Down Expand Up @@ -3274,8 +3272,7 @@ INSTANTIATE_TEST_SUITE_P(
Values(
QuicVersion::MVFST,
QuicVersion::QUIC_V1,
QuicVersion::QUIC_V1_ALIAS,
QuicVersion::QUIC_DRAFT));
QuicVersion::QUIC_V1_ALIAS));

TEST_P(
QuicClientTransportAfterStartTestTimeout,
Expand Down
3 changes: 1 addition & 2 deletions quic/fizz/client/test/QuicClientTransportTestUtil.h
Original file line number Diff line number Diff line change
Expand Up @@ -411,8 +411,7 @@ class QuicClientTransportTestBase : public virtual testing::Test {
{QuicVersion::MVFST,
MVFST1,
QuicVersion::QUIC_V1,
QuicVersion::QUIC_V1_ALIAS,
QuicVersion::QUIC_DRAFT});
QuicVersion::QUIC_V1_ALIAS});
connIdAlgo_ = std::make_unique<DefaultConnectionIdAlgo>();
ON_CALL(*sock, resumeRead(testing::_))
.WillByDefault(testing::SaveArg<0>(&networkReadCallback));
Expand Down
11 changes: 0 additions & 11 deletions quic/fizz/handshake/FizzTransportParameters.h
Original file line number Diff line number Diff line change
Expand Up @@ -192,17 +192,6 @@ inline void validateTransportExtensions(
"unexpected extension type ({:#x}) for quic v1",
extension.extension_type),
fizz::AlertDescription::illegal_parameter);
} else if (
encodingVersion == quic::QuicVersion::QUIC_DRAFT &&
extension.extension_type !=
fizz::ExtensionType::quic_transport_parameters_draft) {
// This is a QUIC draft version using an incorrect transport parameters
// extension type
throw fizz::FizzException(
fmt::format(
"unexpected extension type ({:#x}) for quic draft version",
extension.extension_type),
fizz::AlertDescription::illegal_parameter);
}
found = true;
}
Expand Down
23 changes: 0 additions & 23 deletions quic/fizz/handshake/test/FizzCryptoFactoryTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -62,29 +62,6 @@ class FizzCryptoFactoryTest : public Test {
folly::Optional<std::unique_ptr<folly::IOBuf>> packetCipherKey_;
};

TEST_F(FizzCryptoFactoryTest, TestDraft29ClearTextCipher) {
// test vector taken from
// https://tools.ietf.org/html/draft-ietf-quic-tls-29#appendix-A
auto connid = folly::unhexlify("8394c8f03e515708");
std::vector<uint8_t> destinationConnidVector;
for (size_t i = 0; i < connid.size(); ++i) {
destinationConnidVector.push_back(connid.data()[i]);
}
ConnectionId destinationConnid(destinationConnidVector);
auto fizzFactory = std::make_shared<QuicFizzTestFactory>();
fizzFactory->setMockAead(createMockAead());
auto aead =
FizzCryptoTestFactory(fizzFactory)
.getClientInitialCipher(destinationConnid, QuicVersion::QUIC_DRAFT);

std::string expectedKey = "175257a31eb09dea9366d8bb79ad80ba";
std::string expectedIv = "6b26114b9cba2b63a9e8dd4f";
auto trafficKeyHex = folly::hexlify(trafficKey_->key->coalesce());
auto trafficIvHex = folly::hexlify(trafficKey_->iv->coalesce());
EXPECT_EQ(trafficKeyHex, expectedKey);
EXPECT_EQ(trafficIvHex, expectedIv);
}

TEST_F(FizzCryptoFactoryTest, TestV1ClearTextCipher) {
// test vector taken from
// https://datatracker.ietf.org/doc/html/rfc9001#appendix-A
Expand Down
20 changes: 11 additions & 9 deletions quic/fizz/handshake/test/FizzTransportParametersTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ StringPiece serverParamsD29{"ffa500100008121254761256146904049d7f3e7d"};
StringPiece ticketParamsV1{"0039000604049d7f3e7d"};
StringPiece ticketParamsD29{"ffa5000604049d7f3e7d"};

constexpr QuicVersion kQuicDraft = static_cast<QuicVersion>(0xff00001d);

TEST_F(QuicExtensionsTest, TestClientParamsV1) {
auto exts = getExtensions(clientParamsV1);
auto ext = getClientExtension(exts, QuicVersion::QUIC_V1);
Expand All @@ -70,21 +72,21 @@ TEST_F(QuicExtensionsTest, TestClientParamsV1) {

// Confirm D29 parameters are not parsed for V1.
auto d29Exts = getExtensions(clientParamsD29);
auto d2Ext = getClientExtension(exts, QuicVersion::QUIC_DRAFT);
auto d2Ext = getClientExtension(exts, kQuicDraft);
EXPECT_EQ(d2Ext, folly::none);
}

TEST_F(QuicExtensionsTest, TestClientParamsD29) {
auto exts = getExtensions(clientParamsD29);
auto ext = getClientExtension(exts, QuicVersion::QUIC_DRAFT);
auto ext = getClientExtension(exts, kQuicDraft);
EXPECT_EQ(ext->parameters.size(), 1);
EXPECT_EQ(
ext->parameters[0].parameter, TransportParameterId::initial_max_data);
EXPECT_EQ(
*getIntegerParameter(
TransportParameterId::initial_max_data, ext->parameters),
494878333ULL);
checkEncode(std::move(*ext), clientParamsD29, QuicVersion::QUIC_DRAFT);
checkEncode(std::move(*ext), clientParamsD29, kQuicDraft);
}

TEST_F(QuicExtensionsTest, TestServerParamsV1) {
Expand All @@ -111,13 +113,13 @@ TEST_F(QuicExtensionsTest, TestServerParamsV1) {

// Confirm D29 parameters are not parsed for V1.
auto d29Exts = getExtensions(serverParamsD29);
auto d2Ext = getServerExtension(exts, QuicVersion::QUIC_DRAFT);
auto d2Ext = getServerExtension(exts, kQuicDraft);
EXPECT_EQ(d2Ext, folly::none);
}

TEST_F(QuicExtensionsTest, TestServerParamsD29) {
auto exts = getExtensions(serverParamsD29);
auto ext = getServerExtension(exts, QuicVersion::QUIC_DRAFT);
auto ext = getServerExtension(exts, kQuicDraft);

EXPECT_EQ(
ext->parameters[0].parameter,
Expand All @@ -135,7 +137,7 @@ TEST_F(QuicExtensionsTest, TestServerParamsD29) {
TransportParameterId::original_destination_connection_id,
ext->parameters),
connId);
checkEncode(std::move(*ext), serverParamsD29, QuicVersion::QUIC_DRAFT);
checkEncode(std::move(*ext), serverParamsD29, kQuicDraft);
}

TEST_F(QuicExtensionsTest, TestTicketParamsV1) {
Expand All @@ -153,13 +155,13 @@ TEST_F(QuicExtensionsTest, TestTicketParamsV1) {

// Confirm D29 parameters are not parsed for V1.
auto d29Exts = getExtensions(ticketParamsD29);
auto d2Ext = getTicketExtension(exts, QuicVersion::QUIC_DRAFT);
auto d2Ext = getTicketExtension(exts, kQuicDraft);
EXPECT_EQ(d2Ext, folly::none);
}

TEST_F(QuicExtensionsTest, TestTicketParamsD29) {
auto exts = getExtensions(ticketParamsD29);
auto ext = getTicketExtension(exts, QuicVersion::QUIC_DRAFT);
auto ext = getTicketExtension(exts, kQuicDraft);

EXPECT_EQ(ext->parameters.size(), 1);
EXPECT_EQ(
Expand All @@ -168,7 +170,7 @@ TEST_F(QuicExtensionsTest, TestTicketParamsD29) {
*getIntegerParameter(
TransportParameterId::initial_max_data, ext->parameters),
494878333ULL);
checkEncode(std::move(*ext), ticketParamsD29, QuicVersion::QUIC_DRAFT);
checkEncode(std::move(*ext), ticketParamsD29, kQuicDraft);
}

} // namespace test
Expand Down
2 changes: 0 additions & 2 deletions quic/handshake/HandshakeLayer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,6 @@ folly::StringPiece getQuicVersionSalt(QuicVersion version) {
FOLLY_FALLTHROUGH;
case QuicVersion::QUIC_V1_ALIAS:
return kQuicV1Salt;
case QuicVersion::QUIC_DRAFT:
return kQuicDraft29Salt;
case QuicVersion::MVFST:
FOLLY_FALLTHROUGH;
default:
Expand Down
18 changes: 9 additions & 9 deletions quic/server/QuicServer.h
Original file line number Diff line number Diff line change
Expand Up @@ -393,15 +393,15 @@ class QuicServer : public QuicServerWorker::WorkerCallback,
const folly::SocketAddress& address,
const std::vector<folly::EventBase*>& evbs);

std::vector<QuicVersion> supportedVersions_{
{QuicVersion::MVFST,
QuicVersion::MVFST_EXPERIMENTAL,
QuicVersion::MVFST_EXPERIMENTAL2,
QuicVersion::MVFST_EXPERIMENTAL3,
QuicVersion::MVFST_ALIAS,
QuicVersion::QUIC_V1,
QuicVersion::QUIC_V1_ALIAS,
QuicVersion::QUIC_DRAFT}};
std::vector<QuicVersion> supportedVersions_{{
QuicVersion::MVFST,
QuicVersion::MVFST_EXPERIMENTAL,
QuicVersion::MVFST_EXPERIMENTAL2,
QuicVersion::MVFST_EXPERIMENTAL3,
QuicVersion::MVFST_ALIAS,
QuicVersion::QUIC_V1,
QuicVersion::QUIC_V1_ALIAS,
}};

std::atomic<bool> shutdown_{true};
std::shared_ptr<const fizz::server::FizzServerContext> ctx_;
Expand Down
Loading

0 comments on commit 4eefa3b

Please sign in to comment.