diff --git a/quic/api/QuicTransportFunctions.cpp b/quic/api/QuicTransportFunctions.cpp index 1586c6b46..f5c5d96ff 100644 --- a/quic/api/QuicTransportFunctions.cpp +++ b/quic/api/QuicTransportFunctions.cpp @@ -261,7 +261,7 @@ DataPathResult continuousMemoryBuildScheduleEncrypt( } return DataPathResult::makeBuildFailure(); } - if (!packet->body) { + if (packet->body.empty()) { // No more space remaining. rollbackBuf(); ioBufBatch.flush(); @@ -274,8 +274,7 @@ DataPathResult continuousMemoryBuildScheduleEncrypt( auto headerLen = packet->header.length(); buf = connection.bufAccessor->obtain(); CHECK( - packet->body->data() > buf->data() && - packet->body->tail() <= buf->tail()); + packet->body.data() > buf->data() && packet->body.tail() <= buf->tail()); CHECK( packet->header.data() >= buf->data() && packet->header.tail() < buf->tail()); @@ -345,7 +344,7 @@ DataPathResult iobufChainBasedBuildScheduleEncrypt( } return DataPathResult::makeBuildFailure(); } - if (!packet->body) { + if (packet->body.empty()) { // No more space remaining. ioBufBatch.flush(); if (connection.loopDetectorCallback) { @@ -355,10 +354,10 @@ DataPathResult iobufChainBasedBuildScheduleEncrypt( } packet->header.coalesce(); auto headerLen = packet->header.length(); - auto bodyLen = packet->body->computeChainDataLength(); + auto bodyLen = packet->body.computeChainDataLength(); auto unencrypted = folly::IOBuf::createCombined( headerLen + bodyLen + aead.getCipherOverhead()); - auto bodyCursor = folly::io::Cursor(packet->body.get()); + auto bodyCursor = folly::io::Cursor(&packet->body); bodyCursor.pull(unencrypted->writableData() + headerLen, bodyLen); unencrypted->advance(headerLen); unencrypted->append(bodyLen); @@ -1263,20 +1262,21 @@ void writeCloseCommon( } auto packet = std::move(packetBuilder).buildPacket(); packet.header.coalesce(); - packet.body->reserve(0, aead.getCipherOverhead()); - CHECK_GE(packet.body->tailroom(), aead.getCipherOverhead()); - auto body = - aead.inplaceEncrypt(std::move(packet.body), &packet.header, packetNum); - body->coalesce(); + packet.body.reserve(0, aead.getCipherOverhead()); + CHECK_GE(packet.body.tailroom(), aead.getCipherOverhead()); + auto bufUniquePtr = packet.body.clone(); + bufUniquePtr = + aead.inplaceEncrypt(std::move(bufUniquePtr), &packet.header, packetNum); + bufUniquePtr->coalesce(); encryptPacketHeader( headerForm, packet.header.writableData(), packet.header.length(), - body->data(), - body->length(), + bufUniquePtr->data(), + bufUniquePtr->length(), headerCipher); folly::IOBuf packetBuf(std::move(packet.header)); - packetBuf.prependChain(std::move(body)); + packetBuf.prependChain(std::move(bufUniquePtr)); auto packetSize = packetBuf.computeChainDataLength(); if (connection.qLogger) { connection.qLogger->addPacket(packet.packet, packetSize); diff --git a/quic/api/test/QuicPacketSchedulerTest.cpp b/quic/api/test/QuicPacketSchedulerTest.cpp index b8ce89f11..3cd202dde 100644 --- a/quic/api/test/QuicPacketSchedulerTest.cpp +++ b/quic/api/test/QuicPacketSchedulerTest.cpp @@ -151,7 +151,7 @@ TEST_F(QuicPacketSchedulerTest, CryptoPaddingInitialPacket) { auto result = cryptoOnlyScheduler.scheduleFramesForPacket( std::move(builder), conn.udpSendPacketLen); auto packetLength = result.packet->header.computeChainDataLength() + - result.packet->body->computeChainDataLength(); + result.packet->body.computeChainDataLength(); EXPECT_EQ(conn.udpSendPacketLen, packetLength); } @@ -184,7 +184,7 @@ TEST_F(QuicPacketSchedulerTest, PaddingInitialPureAcks) { auto result = acksOnlyScheduler.scheduleFramesForPacket( std::move(builder), conn.udpSendPacketLen); auto packetLength = result.packet->header.computeChainDataLength() + - result.packet->body->computeChainDataLength(); + result.packet->body.computeChainDataLength(); EXPECT_EQ(conn.udpSendPacketLen, packetLength); } @@ -218,7 +218,7 @@ TEST_F(QuicPacketSchedulerTest, InitialPaddingDoesNotUseWrapper) { auto result = acksOnlyScheduler.scheduleFramesForPacket( std::move(builder), conn.udpSendPacketLen - cipherOverhead); auto packetLength = result.packet->header.computeChainDataLength() + - result.packet->body->computeChainDataLength(); + result.packet->body.computeChainDataLength(); EXPECT_EQ(conn.udpSendPacketLen, packetLength); } @@ -251,7 +251,7 @@ TEST_F(QuicPacketSchedulerTest, CryptoServerInitialPadded) { auto result = scheduler.scheduleFramesForPacket( std::move(builder1), conn.udpSendPacketLen); auto packetLength = result.packet->header.computeChainDataLength() + - result.packet->body->computeChainDataLength(); + result.packet->body.computeChainDataLength(); EXPECT_EQ(conn.udpSendPacketLen, packetLength); } @@ -284,7 +284,7 @@ TEST_F(QuicPacketSchedulerTest, PadTwoInitialPackets) { auto result = scheduler.scheduleFramesForPacket( std::move(builder1), conn.udpSendPacketLen); auto packetLength = result.packet->header.computeChainDataLength() + - result.packet->body->computeChainDataLength(); + result.packet->body.computeChainDataLength(); EXPECT_EQ(conn.udpSendPacketLen, packetLength); increaseNextPacketNum(conn, PacketNumberSpace::Initial); @@ -303,7 +303,7 @@ TEST_F(QuicPacketSchedulerTest, PadTwoInitialPackets) { auto result2 = scheduler.scheduleFramesForPacket( std::move(builder2), conn.udpSendPacketLen); packetLength = result2.packet->header.computeChainDataLength() + - result2.packet->body->computeChainDataLength(); + result2.packet->body.computeChainDataLength(); EXPECT_EQ(conn.udpSendPacketLen, packetLength); } @@ -335,7 +335,7 @@ TEST_F(QuicPacketSchedulerTest, CryptoPaddingRetransmissionClientInitial) { auto result = scheduler.scheduleFramesForPacket( std::move(builder), conn.udpSendPacketLen); auto packetLength = result.packet->header.computeChainDataLength() + - result.packet->body->computeChainDataLength(); + result.packet->body.computeChainDataLength(); EXPECT_EQ(conn.udpSendPacketLen, packetLength); } @@ -398,7 +398,7 @@ TEST_F(QuicPacketSchedulerTest, CryptoWritePartialLossBuffer) { auto result = cryptoOnlyScheduler.scheduleFramesForPacket( std::move(builder), conn.udpSendPacketLen); auto packetLength = result.packet->header.computeChainDataLength() + - result.packet->body->computeChainDataLength(); + result.packet->body.computeChainDataLength(); EXPECT_LE(packetLength, 25); EXPECT_TRUE(result.packet->packet.frames[0].asWriteCryptoFrame() != nullptr); EXPECT_FALSE(conn.cryptoState->initialStream.lossBuffer.empty()); @@ -806,7 +806,9 @@ TEST_F(QuicPacketSchedulerTest, CloneSchedulerUseNormalSchedulerFirst) { folly::IOBuf( folly::IOBuf::CopyBufferOp::COPY_BUFFER, "if you are the dealer"), - folly::IOBuf::copyBuffer("I'm out of the game")); + folly::IOBuf( + folly::IOBuf::CopyBufferOp::COPY_BUFFER, + "I'm out of the game")); return SchedulingResult(folly::none, std::move(builtPacket)); })); RegularQuicPacketBuilder builder( @@ -831,7 +833,7 @@ TEST_F(QuicPacketSchedulerTest, CloneSchedulerUseNormalSchedulerFirst) { *folly::IOBuf::copyBuffer("if you are the dealer"), result.packet->header)); EXPECT_TRUE(folly::IOBufEqualTo{}( - *folly::IOBuf::copyBuffer("I'm out of the game"), *result.packet->body)); + *folly::IOBuf::copyBuffer("I'm out of the game"), result.packet->body)); } TEST_F(QuicPacketSchedulerTest, CloneWillGenerateNewWindowUpdate) { @@ -992,7 +994,7 @@ TEST_F(QuicPacketSchedulerTest, CloningSchedulerWithInplaceBuilderFullPacket) { auto result = scheduler.scheduleFramesForPacket( std::move(builder), conn.udpSendPacketLen); auto bufferLength = result.packet->header.computeChainDataLength() + - result.packet->body->computeChainDataLength(); + result.packet->body.computeChainDataLength(); EXPECT_EQ(conn.udpSendPacketLen, bufferLength); updateConnection( conn, @@ -1063,7 +1065,7 @@ TEST_F(QuicPacketSchedulerTest, CloneLargerThanOriginalPacket) { conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0)); auto packetResult = scheduler.scheduleFramesForPacket( std::move(builder), conn.udpSendPacketLen - cipherOverhead); - auto encodedSize = packetResult.packet->body->computeChainDataLength() + + auto encodedSize = packetResult.packet->body.computeChainDataLength() + packetResult.packet->header.computeChainDataLength() + cipherOverhead; EXPECT_EQ(encodedSize, conn.udpSendPacketLen); updateConnection( @@ -2402,14 +2404,14 @@ TEST_F(QuicPacketSchedulerTest, ShortHeaderPaddingWithSpaceForPadding) { std::move(builder2), conn.udpSendPacketLen); auto headerLength1 = result1.packet->header.computeChainDataLength(); - auto bodyLength1 = result1.packet->body->computeChainDataLength(); + auto bodyLength1 = result1.packet->body.computeChainDataLength(); auto packetLength1 = headerLength1 + bodyLength1; auto expectedPadding1 = (conn.udpSendPacketLen - (inputDataLength1 + headerLength1)) % paddingModulo; auto headerLength2 = result2.packet->header.computeChainDataLength(); - auto bodyLength2 = result2.packet->body->computeChainDataLength(); + auto bodyLength2 = result2.packet->body.computeChainDataLength(); auto packetLength2 = headerLength2 + bodyLength2; auto expectedPadding2 = (conn.udpSendPacketLen - (inputDataLength2 + headerLength2)) % @@ -2460,7 +2462,7 @@ TEST_F(QuicPacketSchedulerTest, ShortHeaderPaddingNearMaxPacketLength) { std::move(builder), conn.udpSendPacketLen); auto headerLength = result.packet->header.computeChainDataLength(); - auto bodyLength = result.packet->body->computeChainDataLength(); + auto bodyLength = result.packet->body.computeChainDataLength(); auto packetLength = headerLength + bodyLength; @@ -2515,7 +2517,7 @@ TEST_F(QuicPacketSchedulerTest, ShortHeaderPaddingMaxPacketLength) { std::move(builder), conn.udpSendPacketLen); auto headerLength = result.packet->header.computeChainDataLength(); - auto bodyLength = result.packet->body->computeChainDataLength(); + auto bodyLength = result.packet->body.computeChainDataLength(); auto packetLength = headerLength + bodyLength; @@ -2554,7 +2556,7 @@ TEST_F(QuicPacketSchedulerTest, ImmediateAckFrameSchedulerOnRequest) { auto result = immediateAckOnlyScheduler.scheduleFramesForPacket( std::move(builder), conn.udpSendPacketLen); auto packetLength = result.packet->header.computeChainDataLength() + - result.packet->body->computeChainDataLength(); + result.packet->body.computeChainDataLength(); EXPECT_EQ(conn.udpSendPacketLen, packetLength); } @@ -2590,7 +2592,7 @@ TEST_F(QuicPacketSchedulerTest, ImmediateAckFrameSchedulerNotRequested) { auto result = immediateAckOnlyScheduler.scheduleFramesForPacket( std::move(builder), conn.udpSendPacketLen); auto packetLength = result.packet->header.computeChainDataLength() + - result.packet->body->computeChainDataLength(); + result.packet->body.computeChainDataLength(); // The immediate ACK scheduler was not triggered. This packet has no frames // and it shouldn't get padded. EXPECT_LT(packetLength, conn.udpSendPacketLen); diff --git a/quic/api/test/QuicTransportFunctionsTest.cpp b/quic/api/test/QuicTransportFunctionsTest.cpp index 01fc756fd..2ee084fad 100644 --- a/quic/api/test/QuicTransportFunctionsTest.cpp +++ b/quic/api/test/QuicTransportFunctionsTest.cpp @@ -149,8 +149,8 @@ uint64_t getEncodedSize(const RegularQuicPacketBuilder::Packet& packet) { if (!packet.header.empty()) { encodedSize += packet.header.computeChainDataLength(); } - if (packet.body) { - encodedSize += packet.body->computeChainDataLength(); + if (!packet.body.empty()) { + encodedSize += packet.body.computeChainDataLength(); } return encodedSize; } @@ -158,8 +158,8 @@ uint64_t getEncodedSize(const RegularQuicPacketBuilder::Packet& packet) { uint64_t getEncodedBodySize(const RegularQuicPacketBuilder::Packet& packet) { // calculate size as the plaintext size uint32_t encodedBodySize = 0; - if (packet.body) { - encodedBodySize += packet.body->computeChainDataLength(); + if (!packet.body.empty()) { + encodedBodySize += packet.body.computeChainDataLength(); } return encodedBodySize; } @@ -1177,7 +1177,8 @@ TEST_F(QuicTransportFunctionsTest, TestUpdateConnectionHandshakeCounter) { auto packet = buildEmptyPacket(*conn, PacketNumberSpace::Handshake); auto packetEncodedSize = !packet.header.empty() ? packet.header.computeChainDataLength() : 0; - packetEncodedSize += packet.body ? packet.body->computeChainDataLength() : 0; + packetEncodedSize += + !packet.body.empty() ? packet.body.computeChainDataLength() : 0; packet.packet.frames.push_back(WriteCryptoFrame(0, 0)); updateConnection( @@ -1194,8 +1195,9 @@ TEST_F(QuicTransportFunctionsTest, TestUpdateConnectionHandshakeCounter) { packetEncodedSize = !nonHandshake.header.empty() ? nonHandshake.header.computeChainDataLength() : 0; - packetEncodedSize += - nonHandshake.body ? nonHandshake.body->computeChainDataLength() : 0; + packetEncodedSize += !nonHandshake.body.empty() + ? nonHandshake.body.computeChainDataLength() + : 0; auto stream1 = conn->streamManager->createNextBidirectionalStream().value(); writeDataToQuicStream(*stream1, nullptr, true); diff --git a/quic/codec/QuicPacketBuilder.cpp b/quic/codec/QuicPacketBuilder.cpp index 95c376b2f..88ab292d6 100644 --- a/quic/codec/QuicPacketBuilder.cpp +++ b/quic/codec/QuicPacketBuilder.cpp @@ -131,9 +131,9 @@ RegularQuicPacketBuilder::RegularQuicPacketBuilder( largestAckedPacketNum_(largestAckedPacketNum), packet_(std::move(header)), header_(folly::IOBuf::CreateOp::CREATE, kLongHeaderHeaderSize), - body_(folly::IOBuf::create(kAppenderGrowthSize)), + body_(folly::IOBuf::CreateOp::CREATE, kAppenderGrowthSize), headerAppender_(&header_, kLongHeaderHeaderSize), - bodyAppender_(body_.get(), kAppenderGrowthSize) { + bodyAppender_(&body_, kAppenderGrowthSize) { if (frameHint) { packet_.frames.reserve(frameHint); } @@ -236,7 +236,7 @@ RegularQuicPacketBuilder::Packet RegularQuicPacketBuilder::buildPacket() && { size_t minBodySize = kMaxPacketNumEncodingSize - packetNumberEncoding_->length + sizeof(Sample); size_t extraDataWritten = 0; - size_t bodyLength = body_->computeChainDataLength(); + size_t bodyLength = body_.computeChainDataLength(); while (bodyLength + extraDataWritten + cipherOverhead_ < minBodySize && !packet_.empty && remainingBytes_ > kMaxPacketLenSize) { // We can add padding frames, but we don't need to store them. @@ -246,7 +246,7 @@ RegularQuicPacketBuilder::Packet RegularQuicPacketBuilder::buildPacket() && { } if (longHeader && longHeader->getHeaderType() != LongHeader::Types::Retry) { QuicInteger pktLen( - packetNumberEncoding_->length + body_->computeChainDataLength() + + packetNumberEncoding_->length + body_.computeChainDataLength() + cipherOverhead_); pktLen.encode([&](auto val) { headerAppender_.writeBE(val); }); appendBytes( @@ -389,7 +389,7 @@ RegularSizeEnforcedPacketBuilder::RegularSizeEnforcedPacketBuilder( : packet_(std::move(packet.packet)), header_(std::move(packet.header)), body_(std::move(packet.body)), - bodyAppender_(body_.get(), kAppenderGrowthSize), + bodyAppender_(&body_, kAppenderGrowthSize), enforcedSize_(enforcedSize), cipherOverhead_(cipherOverhead) {} @@ -399,7 +399,7 @@ bool RegularSizeEnforcedPacketBuilder::canBuildPacket() const noexcept { const ShortHeader* shortHeader = packet_.header.asShort(); // We also don't want to send packets longer than kDefaultMaxUDPPayload return shortHeader && enforcedSize_ <= kDefaultMaxUDPPayload && - (body_->computeChainDataLength() + header_.computeChainDataLength() + + (body_.computeChainDataLength() + header_.computeChainDataLength() + cipherOverhead_ < enforcedSize_); } @@ -408,7 +408,7 @@ PacketBuilderInterface::Packet RegularSizeEnforcedPacketBuilder::buildPacket() && { // Store counters on the stack to overhead from function calls size_t extraDataWritten = 0; - size_t bodyLength = body_->computeChainDataLength(); + size_t bodyLength = body_.computeChainDataLength(); size_t headerLength = header_.computeChainDataLength(); while (extraDataWritten + bodyLength + headerLength + cipherOverhead_ < enforcedSize_) { @@ -435,7 +435,7 @@ InplaceSizeEnforcedPacketBuilder::InplaceSizeEnforcedPacketBuilder( bool InplaceSizeEnforcedPacketBuilder::canBuildPacket() const noexcept { const ShortHeader* shortHeader = packet_.header.asShort(); size_t encryptedPacketSize = - header_.length() + body_->length() + cipherOverhead_; + header_.length() + body_.length() + cipherOverhead_; size_t delta = enforcedSize_ - encryptedPacketSize; return shortHeader && enforcedSize_ <= kDefaultMaxUDPPayload && encryptedPacketSize < enforcedSize_ && iobuf_->tailroom() >= delta; @@ -445,13 +445,13 @@ PacketBuilderInterface::Packet InplaceSizeEnforcedPacketBuilder::buildPacket() && { // Create bodyWriter size_t encryptedPacketSize = - header_.length() + body_->length() + cipherOverhead_; + header_.length() + body_.length() + cipherOverhead_; size_t paddingSize = enforcedSize_ - encryptedPacketSize; BufWriter bodyWriter(*iobuf_, paddingSize); // Store counters on the stack to overhead from function calls size_t extraDataWritten = 0; - size_t bodyLength = body_->computeChainDataLength(); + size_t bodyLength = body_.computeChainDataLength(); size_t headerLength = header_.computeChainDataLength(); while (extraDataWritten + bodyLength + headerLength + cipherOverhead_ < enforcedSize_) { @@ -463,7 +463,8 @@ InplaceSizeEnforcedPacketBuilder::buildPacket() && { PacketBuilderInterface::Packet builtPacket( std::move(packet_), std::move(header_), - folly::IOBuf::wrapBuffer(body_->data(), iobuf_->tail() - body_->data())); + folly::IOBuf::wrapBufferAsValue( + body_.data(), iobuf_->tail() - body_.data())); // Release internal iobuf bufAccessor_.release(std::move(iobuf_)); @@ -731,9 +732,9 @@ PacketBuilderInterface::Packet InplaceQuicPacketBuilder::buildPacket() && { (bodyStart_ ? folly::IOBuf::wrapBufferAsValue( headerStart_, (bodyStart_ - headerStart_)) : folly::IOBuf()), - (bodyStart_ - ? folly::IOBuf::wrapBuffer(bodyStart_, iobuf_->tail() - bodyStart_) - : nullptr)); + (bodyStart_ ? folly::IOBuf::wrapBufferAsValue( + bodyStart_, iobuf_->tail() - bodyStart_) + : folly::IOBuf())); releaseOutputBufferInternal(); return builtPacket; } diff --git a/quic/codec/QuicPacketBuilder.h b/quic/codec/QuicPacketBuilder.h index 0d5803ee9..5b64ceac0 100644 --- a/quic/codec/QuicPacketBuilder.h +++ b/quic/codec/QuicPacketBuilder.h @@ -48,9 +48,12 @@ class PacketBuilderInterface { struct Packet { RegularQuicWritePacket packet; folly::IOBuf header; - Buf body; + folly::IOBuf body; - Packet(RegularQuicWritePacket packetIn, folly::IOBuf headerIn, Buf bodyIn) + Packet( + RegularQuicWritePacket packetIn, + folly::IOBuf headerIn, + folly::IOBuf&& bodyIn) : packet(std::move(packetIn)), header(std::move(headerIn)), body(std::move(bodyIn)) {} @@ -255,7 +258,7 @@ class RegularQuicPacketBuilder final : public PacketBuilderInterface { PacketNum largestAckedPacketNum_; RegularQuicWritePacket packet_; folly::IOBuf header_; - std::unique_ptr body_; + folly::IOBuf body_; BufAppender headerAppender_; BufAppender bodyAppender_; @@ -304,7 +307,7 @@ class RegularSizeEnforcedPacketBuilder : public WrapperPacketBuilderInterface { private: RegularQuicWritePacket packet_; folly::IOBuf header_; - Buf body_; + folly::IOBuf body_; BufAppender bodyAppender_; uint64_t enforcedSize_; uint32_t cipherOverhead_; @@ -340,7 +343,7 @@ class InplaceSizeEnforcedPacketBuilder : public WrapperPacketBuilderInterface { Buf iobuf_; RegularQuicWritePacket packet_; folly::IOBuf header_; - Buf body_; + folly::IOBuf body_; uint64_t enforcedSize_; uint32_t cipherOverhead_; }; diff --git a/quic/codec/test/QuicPacketBuilderTest.cpp b/quic/codec/test/QuicPacketBuilderTest.cpp index 75fbd8f67..0e0208367 100644 --- a/quic/codec/test/QuicPacketBuilderTest.cpp +++ b/quic/codec/test/QuicPacketBuilderTest.cpp @@ -35,8 +35,8 @@ Buf packetToBuf( buf->prependChain(packet.header.clone()); } std::unique_ptr body = folly::IOBuf::create(0); - if (packet.body) { - body = packet.body->clone(); + if (!packet.body.empty()) { + body = packet.body.clone(); } if (aead && !packet.header.empty()) { auto bodySize = body->computeChainDataLength(); @@ -257,7 +257,7 @@ TEST_P(QuicPacketBuilderTest, ShortHeaderRegularPacket) { size_t expectedOutputSize = sizeof(Sample) + kMaxPacketNumEncodingSize - encodedPacketNum.length; // We wrote less than sample bytes into the packet, so we'll pad it to sample - EXPECT_EQ(builtOut.body->computeChainDataLength(), expectedOutputSize); + EXPECT_EQ(builtOut.body.computeChainDataLength(), expectedOutputSize); auto resultBuf = packetToBuf(builtOut); auto& resultShortHeader = *resultRegularPacket.header.asShort(); @@ -314,27 +314,25 @@ TEST_P(QuicPacketBuilderTest, EnforcePacketSizeWithCipherOverhead) { auto param = GetParam(); if (param == TestFlavor::Regular) { - EXPECT_EQ(builtOut.body->isManagedOne(), true); + EXPECT_EQ(builtOut.body.isManagedOne(), true); RegularSizeEnforcedPacketBuilder sizeEnforcedBuilder( std::move(builtOut), enforcedSize, cipherOverhead); EXPECT_TRUE(sizeEnforcedBuilder.canBuildPacket()); auto out = std::move(sizeEnforcedBuilder).buildPacket(); EXPECT_EQ( - out.header.computeChainDataLength() + - out.body->computeChainDataLength(), + out.header.computeChainDataLength() + out.body.computeChainDataLength(), enforcedSize - cipherOverhead); auto buf = packetToBuf(out, aead_); EXPECT_EQ(buf->computeChainDataLength(), enforcedSize); } else { - EXPECT_EQ(builtOut.body->isManagedOne(), false); + EXPECT_EQ(builtOut.body.isManagedOne(), false); InplaceSizeEnforcedPacketBuilder sizeEnforcedBuilder( *simpleBufAccessor_, std::move(builtOut), enforcedSize, cipherOverhead); EXPECT_TRUE(sizeEnforcedBuilder.canBuildPacket()); auto out = std::move(sizeEnforcedBuilder).buildPacket(); EXPECT_EQ( - out.header.computeChainDataLength() + - out.body->computeChainDataLength(), + out.header.computeChainDataLength() + out.body.computeChainDataLength(), enforcedSize - cipherOverhead); auto buf = packetToBuf(out, aead_); EXPECT_EQ(buf->computeChainDataLength(), enforcedSize); @@ -396,7 +394,7 @@ TEST_P(QuicPacketBuilderTest, TestPaddingAccountsForCipherOverhead) { sizeof(Sample) + kMaxPacketNumEncodingSize - encodedPacketNum.length; EXPECT_EQ(resultRegularPacket.frames.size(), 1); EXPECT_EQ( - builtOut.body->computeChainDataLength(), + builtOut.body.computeChainDataLength(), expectedOutputSize - cipherOverhead); } @@ -422,7 +420,7 @@ TEST_P(QuicPacketBuilderTest, TestPaddingRespectsRemainingBytes) { // We should have padded the remaining bytes with Padding frames. EXPECT_EQ(resultRegularPacket.frames.size(), 1); EXPECT_EQ( - builtOut.body->computeChainDataLength(), totalPacketSize - headerSize); + builtOut.body.computeChainDataLength(), totalPacketSize - headerSize); } TEST_F(QuicPacketBuilderTest, PacketBuilderWrapper) { @@ -547,7 +545,7 @@ TEST_F(QuicPacketBuilderTest, BuildTwoInplaces) { EXPECT_EQ(builtOut2.packet.frames[0].asPaddingFrame()->numFrames, 40); EXPECT_EQ(builtOut2.header.length(), builtOut1.header.length()); - EXPECT_EQ(20, builtOut2.body->length() - builtOut1.body->length()); + EXPECT_EQ(20, builtOut2.body.length() - builtOut1.body.length()); } TEST_F(QuicPacketBuilderTest, InplaceBuilderShorterHeaderBytes) { diff --git a/quic/codec/test/QuicPacketRebuilderTest.cpp b/quic/codec/test/QuicPacketRebuilderTest.cpp index 380786581..908dc8e6a 100644 --- a/quic/codec/test/QuicPacketRebuilderTest.cpp +++ b/quic/codec/test/QuicPacketRebuilderTest.cpp @@ -57,7 +57,7 @@ TEST_F(QuicPacketRebuilderTest, RebuildEmpty) { auto packet = std::move(regularBuilder).buildPacket(); EXPECT_TRUE(packet.packet.frames.empty()); EXPECT_FALSE(packet.header.empty()); - EXPECT_TRUE(packet.body->empty()); + EXPECT_TRUE(packet.body.empty()); } TEST_F(QuicPacketRebuilderTest, RebuildSmallInitial) { @@ -86,7 +86,7 @@ TEST_F(QuicPacketRebuilderTest, RebuildSmallInitial) { auto outstanding = makeDummyOutstandingPacket(packet.packet, 1000); EXPECT_FALSE(packet.header.empty()); ASSERT_EQ(packet.packet.frames.size(), 2); - EXPECT_FALSE(packet.body->empty()); + EXPECT_FALSE(packet.body.empty()); regularBuilder2.encodePacketHeader(); ASSERT_TRUE(rebuilder.rebuildFromPacket(outstanding).has_value()); auto rebuilt = std::move(regularBuilder2).buildPacket(); @@ -95,8 +95,8 @@ TEST_F(QuicPacketRebuilderTest, RebuildSmallInitial) { auto padding = rebuilt.packet.frames.back().asPaddingFrame(); ASSERT_TRUE(padding != nullptr); EXPECT_GT(padding->numFrames, 1000); - EXPECT_FALSE(rebuilt.body->empty()); - EXPECT_GT(rebuilt.body->computeChainDataLength(), 1200); + EXPECT_FALSE(rebuilt.body.empty()); + EXPECT_GT(rebuilt.body.computeChainDataLength(), 1200); } TEST_F(QuicPacketRebuilderTest, RebuildPacket) { @@ -335,7 +335,7 @@ TEST_F(QuicPacketRebuilderTest, FinOnlyStreamRebuild) { EXPECT_TRUE(folly::IOBufEqualTo()(packet1.header, packet2.header)); // Once we start to use the correct ack delay value in AckFrames, this needs // to be changed: - EXPECT_TRUE(folly::IOBufEqualTo()(*packet1.body, *packet2.body)); + EXPECT_TRUE(folly::IOBufEqualTo()(packet1.body, packet2.body)); } TEST_F(QuicPacketRebuilderTest, RebuildDataStreamAndEmptyCryptoStream) { @@ -488,7 +488,7 @@ TEST_F(QuicPacketRebuilderTest, CannotRebuild) { ProtectionType::KeyPhaseZero, getTestConnectionId(), 0); RegularQuicPacketBuilder regularBuilder2( (packet1.header.computeChainDataLength() + - packet1.body->computeChainDataLength()) / + packet1.body.computeChainDataLength()) / 2, std::move(shortHeader2), 0 /* largestAcked */); diff --git a/quic/common/test/TestUtils.cpp b/quic/common/test/TestUtils.cpp index 21fb17b64..6b8a034d6 100644 --- a/quic/common/test/TestUtils.cpp +++ b/quic/common/test/TestUtils.cpp @@ -392,8 +392,8 @@ RegularQuicPacketBuilder::Packet createCryptoPacket( Buf packetToBuf(const RegularQuicPacketBuilder::Packet& packet) { auto packetBuf = packet.header.clone(); - if (packet.body) { - packetBuf->prependChain(packet.body->clone()); + if (!packet.body.empty()) { + packetBuf->prependChain(packet.body.clone()); } return packetBuf; } @@ -407,9 +407,9 @@ Buf packetToBufCleartext( << folly::hexlify(packet.header.clone()->moveToFbString()); auto packetBuf = packet.header.clone(); Buf body; - if (packet.body) { - packet.body->coalesce(); - body = packet.body->clone(); + if (!packet.body.empty()) { + packet.body.coalesce(); + body = packet.body.clone(); } else { body = folly::IOBuf::create(0); } @@ -739,7 +739,7 @@ writeCryptoFrame(uint64_t offsetIn, Buf data, PacketBuilderInterface& builder) { void overridePacketWithToken( PacketBuilderInterface::Packet& packet, const StatelessResetToken& token) { - overridePacketWithToken(*packet.body, token); + overridePacketWithToken(packet.body, token); } void overridePacketWithToken( diff --git a/quic/dsr/backend/DSRPacketizer.cpp b/quic/dsr/backend/DSRPacketizer.cpp index 0c0d9957a..fb361878e 100644 --- a/quic/dsr/backend/DSRPacketizer.cpp +++ b/quic/dsr/backend/DSRPacketizer.cpp @@ -69,7 +69,7 @@ bool writeSingleQuicPacket( ioBufBatch.flush(); return false; } - if (!packet.body) { + if (packet.body.empty()) { LOG(ERROR) << "DSR Send failed: Build empty body buffer"; rollbackBuf(); ioBufBatch.flush(); @@ -80,8 +80,8 @@ bool writeSingleQuicPacket( auto headerLen = packet.header.length(); buildBuf = accessor.obtain(); CHECK( - packet.body->data() > buildBuf->data() && - packet.body->tail() <= buildBuf->tail()); + packet.body.data() > buildBuf->data() && + packet.body.tail() <= buildBuf->tail()); CHECK( packet.header.data() >= buildBuf->data() && packet.header.tail() < buildBuf->tail()); diff --git a/quic/fizz/client/test/QuicClientTransportTest.cpp b/quic/fizz/client/test/QuicClientTransportTest.cpp index a372070e5..ed6eea641 100644 --- a/quic/fizz/client/test/QuicClientTransportTest.cpp +++ b/quic/fizz/client/test/QuicClientTransportTest.cpp @@ -2644,8 +2644,7 @@ TEST_F(QuicClientTransportAfterStartTest, ShortHeaderPacketWithNoFrames) { 0 /* largestAcked */); builder.encodePacketHeader(); ASSERT_TRUE(builder.canBuildPacket()); - auto packet = std::move(builder).buildPacket(); - auto buf = packetToBuf(packet); + Buf buf = packetToBuf(std::move(builder).buildPacket()); buf->coalesce(); buf->reserve(0, 200); buf->append(20); diff --git a/quic/loss/test/QuicLossFunctionsTest.cpp b/quic/loss/test/QuicLossFunctionsTest.cpp index 22813aa3b..9421d3522 100644 --- a/quic/loss/test/QuicLossFunctionsTest.cpp +++ b/quic/loss/test/QuicLossFunctionsTest.cpp @@ -254,9 +254,9 @@ PacketNum QuicLossFunctionsTest::sendPacket( if (!packet.header.empty()) { encodedSize += packet.header.computeChainDataLength(); } - if (packet.body) { - encodedSize += packet.body->computeChainDataLength(); - encodedBodySize += packet.body->computeChainDataLength(); + if (!packet.body.empty()) { + encodedSize += packet.body.computeChainDataLength(); + encodedBodySize += packet.body.computeChainDataLength(); } auto outstandingPacket = OutstandingPacketWrapper( packet.packet, diff --git a/quic/server/test/QuicServerTransportTest.cpp b/quic/server/test/QuicServerTransportTest.cpp index 9636ce304..0ab185326 100644 --- a/quic/server/test/QuicServerTransportTest.cpp +++ b/quic/server/test/QuicServerTransportTest.cpp @@ -1707,8 +1707,8 @@ TEST_F(QuicServerTransportTest, ShortHeaderPacketWithNoFrames) { 0 /* largestAcked */); builder.encodePacketHeader(); ASSERT_TRUE(builder.canBuildPacket()); - auto packet = std::move(builder).buildPacket(); - auto buf = packetToBuf(packet); + Buf buf = packetToBuf(std::move(builder).buildPacket()); + buf->coalesce(); buf->reserve(0, 200); buf->append(dummyDataLen); @@ -1758,8 +1758,7 @@ TEST_F(QuicServerTransportTest, ShortHeaderPacketWithNoFramesAfterClose) { 0 /* largestAcked */); builder.encodePacketHeader(); ASSERT_TRUE(builder.canBuildPacket()); - auto packet = std::move(builder).buildPacket(); - auto buf = packetToBuf(packet); + Buf buf = packetToBuf(std::move(builder).buildPacket()); buf->coalesce(); buf->reserve(0, 200); buf->append(dummyDataLen); diff --git a/quic/state/test/AckHandlersTest.cpp b/quic/state/test/AckHandlersTest.cpp index b5c81a9f1..a13621aef 100644 --- a/quic/state/test/AckHandlersTest.cpp +++ b/quic/state/test/AckHandlersTest.cpp @@ -4415,8 +4415,8 @@ class AckEventForAppDataTest : public Test { if (!packet.header.empty()) { encodedSize += packet.header.computeChainDataLength(); } - if (packet.body) { - encodedSize += packet.body->computeChainDataLength(); + if (!packet.body.empty()) { + encodedSize += packet.body.computeChainDataLength(); } return encodedSize; } @@ -4424,8 +4424,8 @@ class AckEventForAppDataTest : public Test { uint64_t getEncodedBodySize(const RegularQuicPacketBuilder::Packet& packet) { // calculate size as the plaintext size uint32_t encodedBodySize = 0; - if (packet.body) { - encodedBodySize += packet.body->computeChainDataLength(); + if (!packet.body.empty()) { + encodedBodySize += packet.body.computeChainDataLength(); } return encodedBodySize; }