diff --git a/quic/api/QuicPacketScheduler.cpp b/quic/api/QuicPacketScheduler.cpp index d83f5c0d6..986d27426 100644 --- a/quic/api/QuicPacketScheduler.cpp +++ b/quic/api/QuicPacketScheduler.cpp @@ -560,7 +560,8 @@ CloningScheduler::scheduleFramesForPacket( RegularQuicPacketBuilder regularBuilder( conn_.udpSendPacketLen, builder.getPacketHeader(), - getAckState(conn_, builderPnSpace).largestAckedByPeer); + getAckState(conn_, builderPnSpace).largestAckedByPeer, + conn_.version.value_or(*conn_.originalVersion)); PacketRebuilder rebuilder(regularBuilder, conn_); // We shouldn't clone Handshake packet. For PureAcks, cloning them bring // perf down as shown by load test. diff --git a/quic/api/QuicTransportFunctions.cpp b/quic/api/QuicTransportFunctions.cpp index f06deb08d..0d1ab7a30 100644 --- a/quic/api/QuicTransportFunctions.cpp +++ b/quic/api/QuicTransportFunctions.cpp @@ -725,7 +725,8 @@ void writeCloseCommon( RegularQuicPacketBuilder packetBuilder( connection.udpSendPacketLen, std::move(header), - getAckState(connection, pnSpace).largestAckedByPeer); + getAckState(connection, pnSpace).largestAckedByPeer, + connection.version.value_or(*connection.originalVersion)); packetBuilder.setCipherOverhead(aead.getCipherOverhead()); size_t written = 0; if (!closeDetails) { @@ -933,7 +934,8 @@ uint64_t writeConnectionDataToSocket( RegularQuicPacketBuilder pktBuilder( connection.udpSendPacketLen, std::move(header), - getAckState(connection, pnSpace).largestAckedByPeer); + getAckState(connection, pnSpace).largestAckedByPeer, + connection.version.value_or(*connection.originalVersion)); pktBuilder.setCipherOverhead(cipherOverhead); auto result = scheduler.scheduleFramesForPacket(std::move(pktBuilder), writableBytes); diff --git a/quic/api/test/QuicTransportTest.cpp b/quic/api/test/QuicTransportTest.cpp index fe39362da..2618bea5f 100644 --- a/quic/api/test/QuicTransportTest.cpp +++ b/quic/api/test/QuicTransportTest.cpp @@ -882,7 +882,7 @@ TEST_F(QuicTransportTest, StopSending) { getLastOutstandingPacket( transport_->getConnectionState(), PacketNumberSpace::AppData) ->packet; - EXPECT_EQ(16, packet.frames.size()); + EXPECT_EQ(14, packet.frames.size()); bool foundStopSending = false; for (auto& simpleFrame : all_frames(packet.frames)) { folly::variant_match( diff --git a/quic/client/QuicClientTransport.cpp b/quic/client/QuicClientTransport.cpp index 988ce1a0a..4b1248a77 100644 --- a/quic/client/QuicClientTransport.cpp +++ b/quic/client/QuicClientTransport.cpp @@ -42,8 +42,8 @@ QuicClientTransport::QuicClientTransport( clientConn_->initialDestinationConnectionId = ConnectionId(connIdData); conn_->readCodec = std::make_unique(QuicNodeType::Client); conn_->readCodec->setClientConnectionId(*conn_->clientConnectionId); - conn_->readCodec->setCodecParameters( - CodecParameters(conn_->peerAckDelayExponent)); + conn_->readCodec->setCodecParameters(CodecParameters( + conn_->peerAckDelayExponent, conn_->originalVersion.value())); // TODO: generate this once we can generate the packet sequence number // correctly. // conn_->nextSequenceNum = folly::Random::secureRandom(); diff --git a/quic/client/state/ClientStateMachine.cpp b/quic/client/state/ClientStateMachine.cpp index 07a72aadd..b53cdb78d 100644 --- a/quic/client/state/ClientStateMachine.cpp +++ b/quic/client/state/ClientStateMachine.cpp @@ -44,8 +44,8 @@ std::unique_ptr undoAllClientStateCommon( newConn->initialWriteCipher = std::move(conn->initialWriteCipher); newConn->readCodec = std::make_unique(QuicNodeType::Client); newConn->readCodec->setClientConnectionId(*conn->clientConnectionId); - newConn->readCodec->setCodecParameters( - CodecParameters(conn->peerAckDelayExponent)); + newConn->readCodec->setCodecParameters(CodecParameters( + conn->peerAckDelayExponent, conn->originalVersion.value())); return newConn; } diff --git a/quic/codec/Decode.cpp b/quic/codec/Decode.cpp index 7654cddb4..3599fb567 100644 --- a/quic/codec/Decode.cpp +++ b/quic/codec/Decode.cpp @@ -191,7 +191,9 @@ ReadAckFrame decodeAckFrameWithECN( return readAckFrame; } -RstStreamFrame decodeRstStreamFrame(folly::io::Cursor& cursor) { +RstStreamFrame decodeRstStreamFrame( + folly::io::Cursor& cursor, + const CodecParameters& params) { auto streamId = decodeQuicInteger(cursor); if (UNLIKELY(!streamId)) { throw QuicTransportException( @@ -199,8 +201,21 @@ RstStreamFrame decodeRstStreamFrame(folly::io::Cursor& cursor) { quic::TransportErrorCode::FRAME_ENCODING_ERROR, quic::FrameType::RST_STREAM); } - auto errorCode = - static_cast(cursor.readBE()); + ApplicationErrorCode errorCode; + if (params.version == QuicVersion::MVFST_OLD) { + errorCode = static_cast( + cursor.readBE()); + } else { + auto varCode = decodeQuicInteger(cursor); + if (varCode) { + errorCode = static_cast(varCode->first); + } else { + throw QuicTransportException( + "Cannot decode error code", + quic::TransportErrorCode::FRAME_ENCODING_ERROR, + quic::FrameType::RST_STREAM); + } + } auto offset = decodeQuicInteger(cursor); if (UNLIKELY(!offset)) { throw QuicTransportException( @@ -212,7 +227,9 @@ RstStreamFrame decodeRstStreamFrame(folly::io::Cursor& cursor) { folly::to(streamId->first), errorCode, offset->first); } -StopSendingFrame decodeStopSendingFrame(folly::io::Cursor& cursor) { +StopSendingFrame decodeStopSendingFrame( + folly::io::Cursor& cursor, + const CodecParameters& params) { auto streamId = decodeQuicInteger(cursor); if (UNLIKELY(!streamId)) { throw QuicTransportException( @@ -220,14 +237,21 @@ StopSendingFrame decodeStopSendingFrame(folly::io::Cursor& cursor) { quic::TransportErrorCode::FRAME_ENCODING_ERROR, quic::FrameType::STOP_SENDING); } - if (!cursor.canAdvance(sizeof(ApplicationErrorCode))) { - throw QuicTransportException( - "Not enough input bytes to read application error code.", - quic::TransportErrorCode::FRAME_ENCODING_ERROR, - quic::FrameType::STOP_SENDING); + ApplicationErrorCode errorCode; + if (params.version == QuicVersion::MVFST_OLD) { + errorCode = static_cast( + cursor.readBE()); + } else { + auto varCode = decodeQuicInteger(cursor); + if (varCode) { + errorCode = static_cast(varCode->first); + } else { + throw QuicTransportException( + "Cannot decode error code", + quic::TransportErrorCode::FRAME_ENCODING_ERROR, + quic::FrameType::STOP_SENDING); + } } - auto errorCode = - static_cast(cursor.readBE()); return StopSendingFrame(folly::to(streamId->first), errorCode); } @@ -512,10 +536,25 @@ PathResponseFrame decodePathResponseFrame(folly::io::Cursor& cursor) { return PathResponseFrame(pathData); } -ConnectionCloseFrame decodeConnectionCloseFrame(folly::io::Cursor& cursor) { - auto detailedCode = - cursor.readBE::type>(); - auto errorCode = static_cast(detailedCode); +ConnectionCloseFrame decodeConnectionCloseFrame( + folly::io::Cursor& cursor, + const CodecParameters& params) { + TransportErrorCode errorCode{}; + if (params.version == QuicVersion::MVFST_OLD) { + auto detailedCode = + cursor.readBE::type>(); + errorCode = static_cast(detailedCode); + } else { + auto varCode = decodeQuicInteger(cursor); + if (varCode) { + errorCode = static_cast(varCode->first); + } else { + throw QuicTransportException( + "Failed to parse error code.", + quic::TransportErrorCode::FRAME_ENCODING_ERROR, + quic::FrameType::CONNECTION_CLOSE); + } + } auto frameTypeField = decodeQuicInteger(cursor); if (UNLIKELY(!frameTypeField || frameTypeField->second != sizeof(uint8_t))) { throw QuicTransportException( @@ -539,9 +578,24 @@ ConnectionCloseFrame decodeConnectionCloseFrame(folly::io::Cursor& cursor) { errorCode, std::move(reasonPhrase), triggeringFrameType); } -ApplicationCloseFrame decodeApplicationCloseFrame(folly::io::Cursor& cursor) { - auto detailedCode = cursor.readBE(); - auto errorCode = static_cast(detailedCode); +ApplicationCloseFrame decodeApplicationCloseFrame( + folly::io::Cursor& cursor, + const CodecParameters& params) { + ApplicationErrorCode errorCode{}; + if (params.version == QuicVersion::MVFST_OLD) { + auto detailedCode = cursor.readBE(); + errorCode = static_cast(detailedCode); + } else { + auto varCode = decodeQuicInteger(cursor); + if (varCode) { + errorCode = static_cast(varCode->first); + } else { + throw QuicTransportException( + "Failed to parse error code.", + quic::TransportErrorCode::FRAME_ENCODING_ERROR, + quic::FrameType::APPLICATION_CLOSE); + } + } auto reasonPhraseLength = decodeQuicInteger(cursor); if (UNLIKELY( !reasonPhraseLength || @@ -630,9 +684,9 @@ QuicFrame parseFrame( case FrameType::ACK_ECN: return QuicFrame(decodeAckFrameWithECN(cursor, header, params)); case FrameType::RST_STREAM: - return QuicFrame(decodeRstStreamFrame(cursor)); + return QuicFrame(decodeRstStreamFrame(cursor, params)); case FrameType::STOP_SENDING: - return QuicFrame(decodeStopSendingFrame(cursor)); + return QuicFrame(decodeStopSendingFrame(cursor, params)); case FrameType::CRYPTO_FRAME: return QuicFrame(decodeCryptoFrame(cursor)); case FrameType::NEW_TOKEN: @@ -665,9 +719,9 @@ QuicFrame parseFrame( case FrameType::PATH_RESPONSE: return QuicFrame(decodePathResponseFrame(cursor)); case FrameType::CONNECTION_CLOSE: - return QuicFrame(decodeConnectionCloseFrame(cursor)); + return QuicFrame(decodeConnectionCloseFrame(cursor, params)); case FrameType::APPLICATION_CLOSE: - return QuicFrame(decodeApplicationCloseFrame(cursor)); + return QuicFrame(decodeApplicationCloseFrame(cursor, params)); case FrameType::MIN_STREAM_DATA: return QuicFrame(decodeMinStreamDataFrame(cursor)); case FrameType::EXPIRED_STREAM_DATA: diff --git a/quic/codec/Decode.h b/quic/codec/Decode.h index e2850a01d..535216af6 100644 --- a/quic/codec/Decode.h +++ b/quic/codec/Decode.h @@ -21,11 +21,12 @@ namespace quic { struct CodecParameters { // This must not be set to zero. uint8_t peerAckDelayExponent{kDefaultAckDelayExponent}; + QuicVersion version{QuicVersion::MVFST}; CodecParameters() = default; - explicit CodecParameters(uint8_t peerAckDelayExponentIn) - : peerAckDelayExponent(peerAckDelayExponentIn) {} + CodecParameters(uint8_t peerAckDelayExponentIn, QuicVersion versionIn) + : peerAckDelayExponent(peerAckDelayExponentIn), version(versionIn) {} }; struct ParsedLongHeaderInvariant { @@ -73,11 +74,17 @@ QuicFrame parseFrame( */ PaddingFrame decodePaddingFrame(folly::io::Cursor&); -RstStreamFrame decodeRstStreamFrame(folly::io::Cursor& cursor); +RstStreamFrame decodeRstStreamFrame( + folly::io::Cursor& cursor, + const CodecParameters& params); -ConnectionCloseFrame decodeConnectionCloseFrame(folly::io::Cursor& cursor); +ConnectionCloseFrame decodeConnectionCloseFrame( + folly::io::Cursor& cursor, + const CodecParameters& params); -ApplicationCloseFrame decodeApplicationCloseFrame(folly::io::Cursor& cursor); +ApplicationCloseFrame decodeApplicationCloseFrame( + folly::io::Cursor& cursor, + const CodecParameters& params); MaxDataFrame decodeMaxDataFrame(folly::io::Cursor& cursor); @@ -105,7 +112,9 @@ NewConnectionIdFrame decodeNewConnectionIdFrame(folly::io::Cursor& cursor); NoopFrame decodeRetireConnectionIdFrame(folly::io::Cursor& cursor); -StopSendingFrame decodeStopSendingFrame(folly::io::Cursor& cursor); +StopSendingFrame decodeStopSendingFrame( + folly::io::Cursor& cursor, + const CodecParameters& params); PathChallengeFrame decodePathChallengeFrame(folly::io::Cursor& cursor); diff --git a/quic/codec/QuicPacketBuilder.cpp b/quic/codec/QuicPacketBuilder.cpp index 0116c240a..9d0953833 100644 --- a/quic/codec/QuicPacketBuilder.cpp +++ b/quic/codec/QuicPacketBuilder.cpp @@ -122,11 +122,13 @@ PacketNumEncodingResult encodeLongHeaderHelper( RegularQuicPacketBuilder::RegularQuicPacketBuilder( uint32_t remainingBytes, PacketHeader header, - PacketNum largestAckedPacketNum) + PacketNum largestAckedPacketNum, + QuicVersion version) : remainingBytes_(remainingBytes), packet_(std::move(header)), headerAppender_(&header_, kLongHeaderHeaderSize), - bodyAppender_(&outputQueue_, kAppenderGrowthSize) { + bodyAppender_(&outputQueue_, kAppenderGrowthSize), + version_(version) { writeHeaderBytes(largestAckedPacketNum); } @@ -276,6 +278,10 @@ void RegularQuicPacketBuilder::setCipherOverhead(uint8_t overhead) noexcept { cipherOverhead_ = overhead; } +QuicVersion RegularQuicPacketBuilder::getVersion() const { + return version_; +} + StatelessResetPacketBuilder::StatelessResetPacketBuilder( uint16_t maxPacketSize, const StatelessResetToken& resetToken) { diff --git a/quic/codec/QuicPacketBuilder.h b/quic/codec/QuicPacketBuilder.h index 3a0166ead..c40b3e818 100644 --- a/quic/codec/QuicPacketBuilder.h +++ b/quic/codec/QuicPacketBuilder.h @@ -63,6 +63,11 @@ class PacketBuilderInterface { // Returns the packet header for the current packet. virtual const PacketHeader& getPacketHeader() const = 0; + + // Horrible hack, remove when we stop having to support MVFST_OLD + virtual QuicVersion getVersion() const { + return QuicVersion::MVFST; + } }; class RegularQuicPacketBuilder : public PacketBuilderInterface { @@ -85,7 +90,8 @@ class RegularQuicPacketBuilder : public PacketBuilderInterface { RegularQuicPacketBuilder( uint32_t remainingBytes, PacketHeader header, - PacketNum largestAckedPacketNum); + PacketNum largestAckedPacketNum, + QuicVersion version = QuicVersion::MVFST_OLD); uint32_t getHeaderBytes() const; @@ -116,6 +122,8 @@ class RegularQuicPacketBuilder : public PacketBuilderInterface { void setCipherOverhead(uint8_t overhead) noexcept; + QuicVersion getVersion() const override; + private: void writeHeaderBytes(PacketNum largestAckedPacketNum); void encodeLongHeader( @@ -135,6 +143,7 @@ class RegularQuicPacketBuilder : public PacketBuilderInterface { folly::io::QueueAppender bodyAppender_; uint32_t cipherOverhead_{0}; folly::Optional packetNumberEncoding_; + QuicVersion version_; }; class VersionNegotiationPacketBuilder { @@ -245,6 +254,10 @@ class PacketBuilderWrapper : public PacketBuilderInterface { return builder.getPacketHeader(); } + QuicVersion getVersion() const override { + return builder.getVersion(); + } + private: PacketBuilderInterface& builder; uint32_t diff; diff --git a/quic/codec/QuicWriteCodec.cpp b/quic/codec/QuicWriteCodec.cpp index fe981590d..fdcea4788 100644 --- a/quic/codec/QuicWriteCodec.cpp +++ b/quic/codec/QuicWriteCodec.cpp @@ -293,13 +293,23 @@ size_t writeSimpleFrame( [&](StopSendingFrame& stopSendingFrame) { QuicInteger intFrameType(static_cast(FrameType::STOP_SENDING)); QuicInteger streamId(stopSendingFrame.streamId); - auto stopSendingFrameSize = intFrameType.getSize() + - streamId.getSize() + sizeof(ApplicationErrorCode); + QuicInteger errorCode( + static_cast(stopSendingFrame.errorCode)); + auto version = builder.getVersion(); + size_t errorSize = version == QuicVersion::MVFST_OLD + ? sizeof(ApplicationErrorCode) + : errorCode.getSize(); + auto stopSendingFrameSize = + intFrameType.getSize() + streamId.getSize() + errorSize; if (packetSpaceCheck(spaceLeft, stopSendingFrameSize)) { builder.write(intFrameType); builder.write(streamId); - builder.writeBE( - static_cast(stopSendingFrame.errorCode)); + if (version == QuicVersion::MVFST_OLD) { + builder.writeBE( + static_cast(stopSendingFrame.errorCode)); + } else { + builder.write(errorCode); + } builder.appendFrame(std::move(stopSendingFrame)); return stopSendingFrameSize; } @@ -425,14 +435,22 @@ size_t writeFrame(QuicWriteFrame&& frame, PacketBuilderInterface& builder) { QuicInteger intFrameType(static_cast(FrameType::RST_STREAM)); QuicInteger streamId(rstStreamFrame.streamId); QuicInteger offset(rstStreamFrame.offset); - auto rstStreamFrameSize = intFrameType.getSize() + - sizeof(ApplicationErrorCode) + streamId.getSize() + - offset.getSize(); + QuicInteger errorCode(static_cast(rstStreamFrame.errorCode)); + auto version = builder.getVersion(); + size_t errorSize = version == QuicVersion::MVFST_OLD + ? sizeof(ApplicationErrorCode) + : errorCode.getSize(); + auto rstStreamFrameSize = intFrameType.getSize() + errorSize + + streamId.getSize() + offset.getSize(); if (packetSpaceCheck(spaceLeft, rstStreamFrameSize)) { builder.write(intFrameType); builder.write(streamId); - builder.writeBE( - static_cast(rstStreamFrame.errorCode)); + if (version == QuicVersion::MVFST_OLD) { + builder.writeBE( + static_cast(rstStreamFrame.errorCode)); + } else { + builder.write(errorCode); + } builder.write(offset); builder.appendFrame(std::move(rstStreamFrame)); return rstStreamFrameSize; @@ -538,17 +556,26 @@ size_t writeFrame(QuicWriteFrame&& frame, PacketBuilderInterface& builder) { QuicInteger intFrameType( static_cast(FrameType::CONNECTION_CLOSE)); QuicInteger reasonLength(connectionCloseFrame.reasonPhrase.size()); - auto connCloseFrameSize = intFrameType.getSize() + - sizeof(TransportErrorCode) + - sizeof(connectionCloseFrame.closingFrameType) + - reasonLength.getSize() + connectionCloseFrame.reasonPhrase.size(); + QuicInteger closingFrameType( + static_cast(connectionCloseFrame.closingFrameType)); + QuicInteger errorCode( + static_cast(connectionCloseFrame.errorCode)); + auto version = builder.getVersion(); + size_t errorSize = version == QuicVersion::MVFST_OLD + ? sizeof(TransportErrorCode) + : errorCode.getSize(); + auto connCloseFrameSize = intFrameType.getSize() + errorSize + + closingFrameType.getSize() + reasonLength.getSize() + + connectionCloseFrame.reasonPhrase.size(); if (packetSpaceCheck(spaceLeft, connCloseFrameSize)) { builder.write(intFrameType); - builder.writeBE( - static_cast::type>( - connectionCloseFrame.errorCode)); - QuicInteger closingFrameType(static_cast( - connectionCloseFrame.closingFrameType)); + if (version == QuicVersion::MVFST_OLD) { + builder.writeBE( + static_cast::type>( + connectionCloseFrame.errorCode)); + } else { + builder.write(errorCode); + } builder.write(closingFrameType); builder.write(reasonLength); builder.push( @@ -564,13 +591,22 @@ size_t writeFrame(QuicWriteFrame&& frame, PacketBuilderInterface& builder) { QuicInteger intFrameType( static_cast(FrameType::APPLICATION_CLOSE)); QuicInteger reasonLength(applicationCloseFrame.reasonPhrase.size()); - auto applicationCloseFrameSize = intFrameType.getSize() + - sizeof(ApplicationErrorCode) + reasonLength.getSize() + - applicationCloseFrame.reasonPhrase.size(); + QuicInteger errorCode( + static_cast(applicationCloseFrame.errorCode)); + auto version = builder.getVersion(); + size_t errorSize = version == QuicVersion::MVFST_OLD + ? sizeof(ApplicationErrorCode) + : errorCode.getSize(); + auto applicationCloseFrameSize = intFrameType.getSize() + errorSize + + reasonLength.getSize() + applicationCloseFrame.reasonPhrase.size(); if (packetSpaceCheck(spaceLeft, applicationCloseFrameSize)) { builder.write(intFrameType); - builder.writeBE(static_cast( - applicationCloseFrame.errorCode)); + if (version == QuicVersion::MVFST_OLD) { + builder.writeBE(static_cast( + applicationCloseFrame.errorCode)); + } else { + builder.write(errorCode); + } builder.write(reasonLength); builder.push( (const uint8_t*)applicationCloseFrame.reasonPhrase.data(), diff --git a/quic/codec/test/DecodeTest.cpp b/quic/codec/test/DecodeTest.cpp index b65c4a78c..54e3424ab 100644 --- a/quic/codec/test/DecodeTest.cpp +++ b/quic/codec/test/DecodeTest.cpp @@ -211,7 +211,9 @@ TEST_F(DecodeTest, ValidAckFrame) { ackBlocks); folly::io::Cursor cursor(result.get()); auto ackFrame = decodeAckFrame( - cursor, makeHeader(), CodecParameters(kDefaultAckDelayExponent)); + cursor, + makeHeader(), + CodecParameters(kDefaultAckDelayExponent, QuicVersion::MVFST)); EXPECT_EQ(ackFrame.ackBlocks.size(), 2); EXPECT_EQ(ackFrame.largestAcked, 1000); // Since 100 is the encoded value, we use the decoded value. @@ -233,7 +235,9 @@ TEST_F(DecodeTest, AckFrameLargestAckExceedsRange) { true); folly::io::Cursor cursor(result.get()); auto ackFrame = decodeAckFrame( - cursor, makeHeader(), CodecParameters(kDefaultAckDelayExponent)); + cursor, + makeHeader(), + CodecParameters(kDefaultAckDelayExponent, QuicVersion::MVFST)); // it will interpret this as a 8 byte range with the max value. EXPECT_EQ(ackFrame.largestAcked, 4611686018427387903); } @@ -254,7 +258,9 @@ TEST_F(DecodeTest, AckFrameLargestAckInvalid) { folly::io::Cursor cursor(result.get()); EXPECT_THROW( decodeAckFrame( - cursor, makeHeader(), CodecParameters(kDefaultAckDelayExponent)), + cursor, + makeHeader(), + CodecParameters(kDefaultAckDelayExponent, QuicVersion::MVFST)), QuicTransportException); } @@ -275,7 +281,9 @@ TEST_F(DecodeTest, AckFrameDelayEncodingInvalid) { folly::io::Cursor cursor(result.get()); EXPECT_THROW( decodeAckFrame( - cursor, makeHeader(), CodecParameters(kDefaultAckDelayExponent)), + cursor, + makeHeader(), + CodecParameters(kDefaultAckDelayExponent, QuicVersion::MVFST)), QuicTransportException); } @@ -290,7 +298,9 @@ TEST_F(DecodeTest, AckFrameDelayExceedsRange) { folly::io::Cursor cursor(result.get()); EXPECT_THROW( decodeAckFrame( - cursor, makeHeader(), CodecParameters(kDefaultAckDelayExponent)), + cursor, + makeHeader(), + CodecParameters(kDefaultAckDelayExponent, QuicVersion::MVFST)), QuicTransportException); } @@ -312,7 +322,9 @@ TEST_F(DecodeTest, AckFrameAdditionalBlocksUnderflow) { folly::io::Cursor cursor(result.get()); EXPECT_THROW( decodeAckFrame( - cursor, makeHeader(), CodecParameters(kDefaultAckDelayExponent)), + cursor, + makeHeader(), + CodecParameters(kDefaultAckDelayExponent, QuicVersion::MVFST)), QuicTransportException); } @@ -335,7 +347,9 @@ TEST_F(DecodeTest, AckFrameAdditionalBlocksOverflow) { ackBlocks); folly::io::Cursor cursor(result.get()); decodeAckFrame( - cursor, makeHeader(), CodecParameters(kDefaultAckDelayExponent)); + cursor, + makeHeader(), + CodecParameters(kDefaultAckDelayExponent, QuicVersion::MVFST)); } TEST_F(DecodeTest, AckFrameMissingFields) { @@ -359,7 +373,9 @@ TEST_F(DecodeTest, AckFrameMissingFields) { EXPECT_THROW( decodeAckFrame( - cursor1, header, CodecParameters(kDefaultAckDelayExponent)), + cursor1, + header, + CodecParameters(kDefaultAckDelayExponent, QuicVersion::MVFST)), QuicTransportException); auto result2 = createAckFrame( @@ -367,7 +383,9 @@ TEST_F(DecodeTest, AckFrameMissingFields) { folly::io::Cursor cursor2(result2.get()); EXPECT_THROW( decodeAckFrame( - cursor2, header, CodecParameters(kDefaultAckDelayExponent)), + cursor2, + header, + CodecParameters(kDefaultAckDelayExponent, QuicVersion::MVFST)), QuicTransportException); auto result3 = createAckFrame( @@ -375,7 +393,9 @@ TEST_F(DecodeTest, AckFrameMissingFields) { folly::io::Cursor cursor3(result3.get()); EXPECT_THROW( decodeAckFrame( - cursor3, header, CodecParameters(kDefaultAckDelayExponent)), + cursor3, + header, + CodecParameters(kDefaultAckDelayExponent, QuicVersion::MVFST)), QuicTransportException); auto result4 = createAckFrame( @@ -383,7 +403,9 @@ TEST_F(DecodeTest, AckFrameMissingFields) { folly::io::Cursor cursor4(result4.get()); EXPECT_THROW( decodeAckFrame( - cursor4, header, CodecParameters(kDefaultAckDelayExponent)), + cursor4, + header, + CodecParameters(kDefaultAckDelayExponent, QuicVersion::MVFST)), QuicTransportException); auto result5 = createAckFrame( @@ -391,7 +413,9 @@ TEST_F(DecodeTest, AckFrameMissingFields) { folly::io::Cursor cursor5(result5.get()); EXPECT_THROW( decodeAckFrame( - cursor5, header, CodecParameters(kDefaultAckDelayExponent)), + cursor5, + header, + CodecParameters(kDefaultAckDelayExponent, QuicVersion::MVFST)), QuicTransportException); } @@ -406,7 +430,9 @@ TEST_F(DecodeTest, AckFrameFirstBlockLengthInvalid) { folly::io::Cursor cursor(result.get()); EXPECT_THROW( decodeAckFrame( - cursor, makeHeader(), CodecParameters(kDefaultAckDelayExponent)), + cursor, + makeHeader(), + CodecParameters(kDefaultAckDelayExponent, QuicVersion::MVFST)), QuicTransportException); } @@ -429,7 +455,9 @@ TEST_F(DecodeTest, AckFrameBlockLengthInvalid) { folly::io::Cursor cursor(result.get()); EXPECT_THROW( decodeAckFrame( - cursor, makeHeader(), CodecParameters(kDefaultAckDelayExponent)), + cursor, + makeHeader(), + CodecParameters(kDefaultAckDelayExponent, QuicVersion::MVFST)), QuicTransportException); } @@ -452,7 +480,9 @@ TEST_F(DecodeTest, AckFrameBlockGapInvalid) { folly::io::Cursor cursor(result.get()); EXPECT_THROW( decodeAckFrame( - cursor, makeHeader(), CodecParameters(kDefaultAckDelayExponent)), + cursor, + makeHeader(), + CodecParameters(kDefaultAckDelayExponent, QuicVersion::MVFST)), QuicTransportException); } @@ -476,7 +506,9 @@ TEST_F(DecodeTest, AckFrameBlockLengthZero) { folly::io::Cursor cursor(result.get()); auto readAckFrame = decodeAckFrame( - cursor, makeHeader(), CodecParameters(kDefaultAckDelayExponent)); + cursor, + makeHeader(), + CodecParameters(kDefaultAckDelayExponent, QuicVersion::MVFST)); EXPECT_EQ(readAckFrame.ackBlocks[0].endPacket, 1000); EXPECT_EQ(readAckFrame.ackBlocks[0].startPacket, 990); EXPECT_EQ(readAckFrame.ackBlocks[1].endPacket, 978); diff --git a/quic/codec/test/QuicReadCodecTest.cpp b/quic/codec/test/QuicReadCodecTest.cpp index ed19b0cb6..ef7f87402 100644 --- a/quic/codec/test/QuicReadCodecTest.cpp +++ b/quic/codec/test/QuicReadCodecTest.cpp @@ -34,7 +34,8 @@ class QuicReadCodecTest : public Test {}; std::unique_ptr makeUnencryptedCodec() { auto codec = std::make_unique(QuicNodeType::Server); - codec->setCodecParameters(CodecParameters(kDefaultAckDelayExponent)); + codec->setCodecParameters( + CodecParameters(kDefaultAckDelayExponent, QuicVersion::MVFST)); return codec; } diff --git a/quic/codec/test/QuicWriteCodecTest.cpp b/quic/codec/test/QuicWriteCodecTest.cpp index f0bc4f0f3..2286efcab 100644 --- a/quic/codec/test/QuicWriteCodecTest.cpp +++ b/quic/codec/test/QuicWriteCodecTest.cpp @@ -31,7 +31,7 @@ QuicFrame parseQuicFrame(folly::io::Cursor& cursor) { return quic::parseFrame( cursor, buildTestShortHeader(), - CodecParameters(kDefaultAckDelayExponent)); + CodecParameters(kDefaultAckDelayExponent, QuicVersion::MVFST)); } namespace quic { @@ -259,14 +259,18 @@ TEST_F(QuicWriteCodecTest, WriteTwoStreamFrames) { auto wireBuf = std::move(builtOut.second); folly::io::Cursor cursor(wireBuf.get()); auto decodedStreamFrame1 = boost::get(quic::parseFrame( - cursor, regularPacket.header, CodecParameters(kDefaultAckDelayExponent))); + cursor, + regularPacket.header, + CodecParameters(kDefaultAckDelayExponent, QuicVersion::MVFST))); EXPECT_EQ(decodedStreamFrame1.streamId, streamId1); EXPECT_EQ(decodedStreamFrame1.offset, offset1); EXPECT_EQ(decodedStreamFrame1.data->computeChainDataLength(), 30); EXPECT_TRUE(folly::IOBufEqualTo()(inputBuf, decodedStreamFrame1.data)); // Read another one from wire output: auto decodedStreamFrame2 = boost::get(quic::parseFrame( - cursor, regularPacket.header, CodecParameters(kDefaultAckDelayExponent))); + cursor, + regularPacket.header, + CodecParameters(kDefaultAckDelayExponent, QuicVersion::MVFST))); EXPECT_EQ(decodedStreamFrame2.streamId, streamId2); EXPECT_EQ(decodedStreamFrame2.offset, offset2); EXPECT_EQ(decodedStreamFrame2.data->computeChainDataLength(), 40); @@ -395,7 +399,9 @@ TEST_F(QuicWriteCodecTest, WriteStreamSpaceForOneByte) { auto wireBuf = std::move(builtOut.second); folly::io::Cursor cursor(wireBuf.get()); auto decodedStreamFrame = boost::get(quic::parseFrame( - cursor, regularPacket.header, CodecParameters(kDefaultAckDelayExponent))); + cursor, + regularPacket.header, + CodecParameters(kDefaultAckDelayExponent, QuicVersion::MVFST))); EXPECT_EQ(decodedStreamFrame.streamId, streamId); EXPECT_EQ(decodedStreamFrame.offset, offset); EXPECT_EQ(decodedStreamFrame.data->computeChainDataLength(), 1); @@ -434,7 +440,9 @@ TEST_F(QuicWriteCodecTest, WriteFinToEmptyPacket) { auto wireBuf = std::move(builtOut.second); folly::io::Cursor cursor(wireBuf.get()); auto decodedStreamFrame = boost::get(quic::parseFrame( - cursor, regularPacket.header, CodecParameters(kDefaultAckDelayExponent))); + cursor, + regularPacket.header, + CodecParameters(kDefaultAckDelayExponent, QuicVersion::MVFST))); EXPECT_EQ(decodedStreamFrame.streamId, streamId); EXPECT_EQ(decodedStreamFrame.offset, offset); EXPECT_EQ( @@ -721,7 +729,9 @@ TEST_F(QuicWriteCodecTest, WriteWithDifferentAckDelayExponent) { auto wireBuf = std::move(builtOut.second); folly::io::Cursor cursor(wireBuf.get()); auto decodedAckFrame = boost::get(quic::parseFrame( - cursor, builtOut.first.header, CodecParameters(ackDelayExponent))); + cursor, + builtOut.first.header, + CodecParameters(ackDelayExponent, QuicVersion::MVFST))); EXPECT_EQ( decodedAckFrame.ackDelay.count(), computeExpectedDelay(ackMetadata.ackDelay, ackDelayExponent)); @@ -740,7 +750,9 @@ TEST_F(QuicWriteCodecTest, WriteExponentInLongHeaderPacket) { auto wireBuf = std::move(builtOut.second); folly::io::Cursor cursor(wireBuf.get()); auto decodedAckFrame = boost::get(quic::parseFrame( - cursor, builtOut.first.header, CodecParameters(ackDelayExponent))); + cursor, + builtOut.first.header, + CodecParameters(ackDelayExponent, QuicVersion::MVFST))); EXPECT_EQ( decodedAckFrame.ackDelay.count(), (uint64_t(ackMetadata.ackDelay.count()) >> ackDelayExponent) @@ -1015,8 +1027,8 @@ TEST_F(QuicWriteCodecTest, WriteConnClose) { auto builtOut = std::move(pktBuilder).buildPacket(); auto regularPacket = builtOut.first; - // 5 == ErrorCode(2) + FrameType(1) + reasonPhrase-len(2) - EXPECT_EQ(5 + reasonPhrase.size(), connCloseBytesWritten); + // 6 == ErrorCode(2) + FrameType(1) + reasonPhrase-len(2) + EXPECT_EQ(4 + reasonPhrase.size(), connCloseBytesWritten); auto resultConnCloseFrame = boost::get(regularPacket.frames[0]); EXPECT_EQ( @@ -1192,7 +1204,7 @@ TEST_F(QuicWriteCodecTest, WriteRstStream) { auto builtOut = std::move(pktBuilder).buildPacket(); auto regularPacket = builtOut.first; - EXPECT_EQ(11, rstStreamBytesWritten); + EXPECT_EQ(13, rstStreamBytesWritten); auto resultRstStreamFrame = boost::get(regularPacket.frames[0]); EXPECT_EQ(errorCode, resultRstStreamFrame.errorCode); @@ -1314,7 +1326,7 @@ TEST_F(QuicWriteCodecTest, WriteStopSending) { auto builtOut = std::move(pktBuilder).buildPacket(); auto regularPacket = builtOut.first; - EXPECT_EQ(bytesWritten, 4); + EXPECT_EQ(bytesWritten, 6); auto wireBuf = std::move(builtOut.second); folly::io::Cursor cursor(wireBuf.get()); diff --git a/quic/server/state/ServerStateMachine.cpp b/quic/server/state/ServerStateMachine.cpp index 804ac8699..af855bb80 100644 --- a/quic/server/state/ServerStateMachine.cpp +++ b/quic/server/state/ServerStateMachine.cpp @@ -494,7 +494,7 @@ void onServerReadDataFromOpen( conn.qLogger->dcid = clientConnectionId; } conn.readCodec->setCodecParameters( - CodecParameters(conn.peerAckDelayExponent)); + CodecParameters(conn.peerAckDelayExponent, version)); conn.initialWriteCipher = getServerInitialCipher( &fizzFactory, initialDestinationConnectionId, version); diff --git a/quic/server/test/QuicServerTransportTest.cpp b/quic/server/test/QuicServerTransportTest.cpp index 6e3f1113e..4b76f519a 100644 --- a/quic/server/test/QuicServerTransportTest.cpp +++ b/quic/server/test/QuicServerTransportTest.cpp @@ -412,7 +412,7 @@ class QuicServerTransportTest : public Test { clientReadCodec->setInitialHeaderCipher(makeServerInitialHeaderCipher( &fizzFactory, *initialDestinationConnectionId, QuicVersion::MVFST)); clientReadCodec->setCodecParameters( - CodecParameters(kDefaultAckDelayExponent)); + CodecParameters(kDefaultAckDelayExponent, QuicVersion::MVFST)); } virtual void expectWriteNewSessionTicket() { @@ -575,6 +575,8 @@ class QuicServerTransportTest : public Test { readCodec->setHandshakeReadCipher(test::createNoOpAead()); readCodec->setHandshakeHeaderCipher(test::createNoOpHeaderCipher()); readCodec->setClientConnectionId(*clientConnectionId); + readCodec->setCodecParameters( + CodecParameters(kDefaultAckDelayExponent, QuicVersion::MVFST)); if (handshakeCipher) { readCodec->setInitialReadCipher(getServerInitialCipher( &fizzFactory, *initialDestinationConnectionId, QuicVersion::MVFST)); @@ -1215,7 +1217,8 @@ TEST_F(QuicServerTransportTest, RecvRstStreamFrame) { RegularQuicPacketBuilder builder( server->getConn().udpSendPacketLen, std::move(header), - 0 /* largestAcked */); + 0 /* largestAcked */, + QuicVersion::MVFST); RstStreamFrame rstFrame( streamId, @@ -1269,7 +1272,8 @@ TEST_F(QuicServerTransportTest, RecvStopSendingFrame) { RegularQuicPacketBuilder builder( server->getConn().udpSendPacketLen, std::move(header), - 0 /* largestAcked */); + 0 /* largestAcked */, + QuicVersion::MVFST); StopSendingFrame stopSendingFrame( streamId, GenericApplicationErrorCode::UNKNOWN); @@ -1389,7 +1393,8 @@ TEST_F(QuicServerTransportTest, RecvStopSendingFrameAfterHalfCloseRemote) { RegularQuicPacketBuilder builder( server->getConn().udpSendPacketLen, std::move(header), - 0 /* largestAcked */); + 0 /* largestAcked */, + QuicVersion::MVFST); StopSendingFrame stopSendingFrame( streamId, GenericApplicationErrorCode::UNKNOWN); @@ -1419,7 +1424,8 @@ TEST_F(QuicServerTransportTest, RecvStopSendingBeforeStream) { RegularQuicPacketBuilder builder( server->getConn().udpSendPacketLen, std::move(header), - 0 /* largestAcked */); + 0 /* largestAcked */, + QuicVersion::MVFST); StopSendingFrame stopSendingFrame( streamId, GenericApplicationErrorCode::UNKNOWN); @@ -1471,7 +1477,8 @@ TEST_F(QuicServerTransportTest, RecvStopSendingFrameAfterReset) { RegularQuicPacketBuilder builder( server->getConn().udpSendPacketLen, std::move(header), - 0 /* largestAcked */); + 0 /* largestAcked */, + QuicVersion::MVFST); StopSendingFrame stopSendingFrame1( streamId1, GenericApplicationErrorCode::UNKNOWN); @@ -1500,7 +1507,8 @@ TEST_F(QuicServerTransportTest, StopSendingLoss) { RegularQuicPacketBuilder builder( server->getConn().udpSendPacketLen, std::move(header), - server->getConn().ackStates.appDataAckState.largestAckedByPeer); + server->getConn().ackStates.appDataAckState.largestAckedByPeer, + QuicVersion::MVFST); StopSendingFrame stopSendingFrame( streamId, GenericApplicationErrorCode::UNKNOWN); ASSERT_TRUE(builder.canBuildPacket()); @@ -1529,7 +1537,8 @@ TEST_F(QuicServerTransportTest, StopSendingLossAfterStreamClosed) { RegularQuicPacketBuilder builder( server->getConn().udpSendPacketLen, std::move(header), - server->getConn().ackStates.appDataAckState.largestAckedByPeer); + server->getConn().ackStates.appDataAckState.largestAckedByPeer, + QuicVersion::MVFST); StopSendingFrame stopSendingFrame( streamId, GenericApplicationErrorCode::UNKNOWN); ASSERT_TRUE(builder.canBuildPacket()); @@ -1692,7 +1701,8 @@ TEST_F(QuicServerTransportTest, ReceiveConnectionClose) { RegularQuicPacketBuilder builder( server->getConn().udpSendPacketLen, std::move(header), - 0 /* largestAcked */); + 0 /* largestAcked */, + QuicVersion::MVFST); std::string errMsg = "Stand clear of the closing doors, please"; ConnectionCloseFrame connClose(TransportErrorCode::NO_ERROR, errMsg); writeFrame(std::move(connClose), builder); @@ -1722,7 +1732,8 @@ TEST_F(QuicServerTransportTest, ReceiveApplicationClose) { RegularQuicPacketBuilder builder( server->getConn().udpSendPacketLen, std::move(header), - 0 /* largestAcked */); + 0 /* largestAcked */, + QuicVersion::MVFST); std::string errMsg = "Stand clear of the closing doors, please"; ApplicationCloseFrame appClose(GenericApplicationErrorCode::UNKNOWN, errMsg); writeFrame(std::move(appClose), builder); @@ -1757,7 +1768,8 @@ TEST_F(QuicServerTransportTest, ReceiveConnectionCloseTwice) { RegularQuicPacketBuilder builder( server->getConn().udpSendPacketLen, std::move(header), - 0 /* largestAcked */); + 0 /* largestAcked */, + QuicVersion::MVFST); std::string errMsg = "Mind the gap"; ConnectionCloseFrame connClose(TransportErrorCode::NO_ERROR, errMsg); writeFrame(std::move(connClose), builder);