diff --git a/quic/api/QuicTransportFunctions.cpp b/quic/api/QuicTransportFunctions.cpp index 63833b14b..1586c6b46 100644 --- a/quic/api/QuicTransportFunctions.cpp +++ b/quic/api/QuicTransportFunctions.cpp @@ -270,21 +270,21 @@ DataPathResult continuousMemoryBuildScheduleEncrypt( } return DataPathResult::makeBuildFailure(); } - CHECK(!packet->header->isChained()); - auto headerLen = packet->header->length(); + CHECK(!packet->header.isChained()); + auto headerLen = packet->header.length(); buf = connection.bufAccessor->obtain(); CHECK( packet->body->data() > buf->data() && packet->body->tail() <= buf->tail()); CHECK( - packet->header->data() >= buf->data() && - packet->header->tail() < buf->tail()); + packet->header.data() >= buf->data() && + packet->header.tail() < buf->tail()); // Trim off everything before the current packet, and the header length, so // buf's data starts from the body part of buf. buf->trimStart(prevSize + headerLen); // buf and packetBuf is actually the same. auto packetBuf = - aead.inplaceEncrypt(std::move(buf), packet->header.get(), packetNum); + aead.inplaceEncrypt(std::move(buf), &packet->header, packetNum); CHECK(packetBuf->headroom() == headerLen + prevSize); // Include header back. packetBuf->prepend(headerLen); @@ -353,8 +353,8 @@ DataPathResult iobufChainBasedBuildScheduleEncrypt( } return DataPathResult::makeBuildFailure(); } - packet->header->coalesce(); - auto headerLen = packet->header->length(); + packet->header.coalesce(); + auto headerLen = packet->header.length(); auto bodyLen = packet->body->computeChainDataLength(); auto unencrypted = folly::IOBuf::createCombined( headerLen + bodyLen + aead.getCipherOverhead()); @@ -362,11 +362,11 @@ DataPathResult iobufChainBasedBuildScheduleEncrypt( bodyCursor.pull(unencrypted->writableData() + headerLen, bodyLen); unencrypted->advance(headerLen); unencrypted->append(bodyLen); - auto packetBuf = aead.inplaceEncrypt( - std::move(unencrypted), packet->header.get(), packetNum); + auto packetBuf = + aead.inplaceEncrypt(std::move(unencrypted), &packet->header, packetNum); DCHECK(packetBuf->headroom() == headerLen); packetBuf->clear(); - auto headerCursor = folly::io::Cursor(packet->header.get()); + auto headerCursor = folly::io::Cursor(&packet->header); headerCursor.pull(packetBuf->writableData(), headerLen); packetBuf->append(headerLen + bodyLen + aead.getCipherOverhead()); @@ -1262,22 +1262,22 @@ void writeCloseCommon( return; } auto packet = std::move(packetBuilder).buildPacket(); - packet.header->coalesce(); + 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.get(), packetNum); + auto body = + aead.inplaceEncrypt(std::move(packet.body), &packet.header, packetNum); body->coalesce(); encryptPacketHeader( headerForm, - packet.header->writableData(), - packet.header->length(), + packet.header.writableData(), + packet.header.length(), body->data(), body->length(), headerCipher); - auto packetBuf = std::move(packet.header); - packetBuf->prependChain(std::move(body)); - auto packetSize = packetBuf->computeChainDataLength(); + folly::IOBuf packetBuf(std::move(packet.header)); + packetBuf.prependChain(std::move(body)); + auto packetSize = packetBuf.computeChainDataLength(); if (connection.qLogger) { connection.qLogger->addPacket(packet.packet, packetSize); } @@ -1287,7 +1287,9 @@ void writeCloseCommon( // Increment the sequence number. increaseNextPacketNum(connection, pnSpace); // best effort writing to the socket, ignore any errors. - auto ret = sock.write(connection.peerAddress, packetBuf); + + Buf packetBufPtr = packetBuf.clone(); + auto ret = sock.write(connection.peerAddress, packetBufPtr); connection.lossState.totalBytesSent += packetSize; if (ret < 0) { VLOG(4) << "Error writing connection close " << folly::errnoStr(errno) diff --git a/quic/api/test/QuicPacketSchedulerTest.cpp b/quic/api/test/QuicPacketSchedulerTest.cpp index 24355cb9b..b8ce89f11 100644 --- a/quic/api/test/QuicPacketSchedulerTest.cpp +++ b/quic/api/test/QuicPacketSchedulerTest.cpp @@ -150,7 +150,7 @@ TEST_F(QuicPacketSchedulerTest, CryptoPaddingInitialPacket) { conn.cryptoState->initialStream, folly::IOBuf::copyBuffer("chlo")); auto result = cryptoOnlyScheduler.scheduleFramesForPacket( std::move(builder), conn.udpSendPacketLen); - auto packetLength = result.packet->header->computeChainDataLength() + + auto packetLength = result.packet->header.computeChainDataLength() + result.packet->body->computeChainDataLength(); EXPECT_EQ(conn.udpSendPacketLen, packetLength); } @@ -183,7 +183,7 @@ TEST_F(QuicPacketSchedulerTest, PaddingInitialPureAcks) { .build(); auto result = acksOnlyScheduler.scheduleFramesForPacket( std::move(builder), conn.udpSendPacketLen); - auto packetLength = result.packet->header->computeChainDataLength() + + auto packetLength = result.packet->header.computeChainDataLength() + result.packet->body->computeChainDataLength(); EXPECT_EQ(conn.udpSendPacketLen, packetLength); } @@ -217,7 +217,7 @@ TEST_F(QuicPacketSchedulerTest, InitialPaddingDoesNotUseWrapper) { .build(); auto result = acksOnlyScheduler.scheduleFramesForPacket( std::move(builder), conn.udpSendPacketLen - cipherOverhead); - auto packetLength = result.packet->header->computeChainDataLength() + + auto packetLength = result.packet->header.computeChainDataLength() + result.packet->body->computeChainDataLength(); EXPECT_EQ(conn.udpSendPacketLen, packetLength); } @@ -250,7 +250,7 @@ TEST_F(QuicPacketSchedulerTest, CryptoServerInitialPadded) { conn.cryptoState->initialStream, folly::IOBuf::copyBuffer("shlo")); auto result = scheduler.scheduleFramesForPacket( std::move(builder1), conn.udpSendPacketLen); - auto packetLength = result.packet->header->computeChainDataLength() + + auto packetLength = result.packet->header.computeChainDataLength() + result.packet->body->computeChainDataLength(); EXPECT_EQ(conn.udpSendPacketLen, packetLength); } @@ -283,7 +283,7 @@ TEST_F(QuicPacketSchedulerTest, PadTwoInitialPackets) { conn.cryptoState->initialStream, folly::IOBuf::copyBuffer("shlo")); auto result = scheduler.scheduleFramesForPacket( std::move(builder1), conn.udpSendPacketLen); - auto packetLength = result.packet->header->computeChainDataLength() + + auto packetLength = result.packet->header.computeChainDataLength() + result.packet->body->computeChainDataLength(); EXPECT_EQ(conn.udpSendPacketLen, packetLength); @@ -302,7 +302,7 @@ TEST_F(QuicPacketSchedulerTest, PadTwoInitialPackets) { conn.cryptoState->initialStream, folly::IOBuf::copyBuffer("shlo again")); auto result2 = scheduler.scheduleFramesForPacket( std::move(builder2), conn.udpSendPacketLen); - packetLength = result2.packet->header->computeChainDataLength() + + packetLength = result2.packet->header.computeChainDataLength() + result2.packet->body->computeChainDataLength(); EXPECT_EQ(conn.udpSendPacketLen, packetLength); } @@ -334,7 +334,7 @@ TEST_F(QuicPacketSchedulerTest, CryptoPaddingRetransmissionClientInitial) { StreamBuffer{folly::IOBuf::copyBuffer("chlo"), 0, false}); auto result = scheduler.scheduleFramesForPacket( std::move(builder), conn.udpSendPacketLen); - auto packetLength = result.packet->header->computeChainDataLength() + + auto packetLength = result.packet->header.computeChainDataLength() + result.packet->body->computeChainDataLength(); EXPECT_EQ(conn.udpSendPacketLen, packetLength); } @@ -397,7 +397,7 @@ TEST_F(QuicPacketSchedulerTest, CryptoWritePartialLossBuffer) { false}); auto result = cryptoOnlyScheduler.scheduleFramesForPacket( std::move(builder), conn.udpSendPacketLen); - auto packetLength = result.packet->header->computeChainDataLength() + + auto packetLength = result.packet->header.computeChainDataLength() + result.packet->body->computeChainDataLength(); EXPECT_LE(packetLength, 25); EXPECT_TRUE(result.packet->packet.frames[0].asWriteCryptoFrame() != nullptr); @@ -803,7 +803,9 @@ TEST_F(QuicPacketSchedulerTest, CloneSchedulerUseNormalSchedulerFirst) { packet.frames.push_back(MaxDataFrame(2832)); RegularQuicPacketBuilder::Packet builtPacket( std::move(packet), - folly::IOBuf::copyBuffer("if you are the dealer"), + folly::IOBuf( + folly::IOBuf::CopyBufferOp::COPY_BUFFER, + "if you are the dealer"), folly::IOBuf::copyBuffer("I'm out of the game")); return SchedulingResult(folly::none, std::move(builtPacket)); })); @@ -827,7 +829,7 @@ TEST_F(QuicPacketSchedulerTest, CloneSchedulerUseNormalSchedulerFirst) { EXPECT_EQ(2832, maxDataFrame->maximumData); EXPECT_TRUE(folly::IOBufEqualTo{}( *folly::IOBuf::copyBuffer("if you are the dealer"), - *result.packet->header)); + result.packet->header)); EXPECT_TRUE(folly::IOBufEqualTo{}( *folly::IOBuf::copyBuffer("I'm out of the game"), *result.packet->body)); } @@ -989,7 +991,7 @@ TEST_F(QuicPacketSchedulerTest, CloningSchedulerWithInplaceBuilderFullPacket) { ASSERT_TRUE(scheduler.hasData()); auto result = scheduler.scheduleFramesForPacket( std::move(builder), conn.udpSendPacketLen); - auto bufferLength = result.packet->header->computeChainDataLength() + + auto bufferLength = result.packet->header.computeChainDataLength() + result.packet->body->computeChainDataLength(); EXPECT_EQ(conn.udpSendPacketLen, bufferLength); updateConnection( @@ -1062,7 +1064,7 @@ TEST_F(QuicPacketSchedulerTest, CloneLargerThanOriginalPacket) { auto packetResult = scheduler.scheduleFramesForPacket( std::move(builder), conn.udpSendPacketLen - cipherOverhead); auto encodedSize = packetResult.packet->body->computeChainDataLength() + - packetResult.packet->header->computeChainDataLength() + cipherOverhead; + packetResult.packet->header.computeChainDataLength() + cipherOverhead; EXPECT_EQ(encodedSize, conn.udpSendPacketLen); updateConnection( conn, @@ -2399,14 +2401,14 @@ TEST_F(QuicPacketSchedulerTest, ShortHeaderPaddingWithSpaceForPadding) { auto result2 = scheduler.scheduleFramesForPacket( std::move(builder2), conn.udpSendPacketLen); - auto headerLength1 = result1.packet->header->computeChainDataLength(); + auto headerLength1 = result1.packet->header.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 headerLength2 = result2.packet->header.computeChainDataLength(); auto bodyLength2 = result2.packet->body->computeChainDataLength(); auto packetLength2 = headerLength2 + bodyLength2; auto expectedPadding2 = @@ -2457,7 +2459,7 @@ TEST_F(QuicPacketSchedulerTest, ShortHeaderPaddingNearMaxPacketLength) { auto result = scheduler.scheduleFramesForPacket( std::move(builder), conn.udpSendPacketLen); - auto headerLength = result.packet->header->computeChainDataLength(); + auto headerLength = result.packet->header.computeChainDataLength(); auto bodyLength = result.packet->body->computeChainDataLength(); auto packetLength = headerLength + bodyLength; @@ -2512,7 +2514,7 @@ TEST_F(QuicPacketSchedulerTest, ShortHeaderPaddingMaxPacketLength) { auto result = scheduler.scheduleFramesForPacket( std::move(builder), conn.udpSendPacketLen); - auto headerLength = result.packet->header->computeChainDataLength(); + auto headerLength = result.packet->header.computeChainDataLength(); auto bodyLength = result.packet->body->computeChainDataLength(); auto packetLength = headerLength + bodyLength; @@ -2551,7 +2553,7 @@ TEST_F(QuicPacketSchedulerTest, ImmediateAckFrameSchedulerOnRequest) { auto result = immediateAckOnlyScheduler.scheduleFramesForPacket( std::move(builder), conn.udpSendPacketLen); - auto packetLength = result.packet->header->computeChainDataLength() + + auto packetLength = result.packet->header.computeChainDataLength() + result.packet->body->computeChainDataLength(); EXPECT_EQ(conn.udpSendPacketLen, packetLength); } @@ -2587,7 +2589,7 @@ TEST_F(QuicPacketSchedulerTest, ImmediateAckFrameSchedulerNotRequested) { auto result = immediateAckOnlyScheduler.scheduleFramesForPacket( std::move(builder), conn.udpSendPacketLen); - auto packetLength = result.packet->header->computeChainDataLength() + + auto packetLength = result.packet->header.computeChainDataLength() + result.packet->body->computeChainDataLength(); // The immediate ACK scheduler was not triggered. This packet has no frames // and it shouldn't get padded. diff --git a/quic/api/test/QuicTransportFunctionsTest.cpp b/quic/api/test/QuicTransportFunctionsTest.cpp index 195123825..01fc756fd 100644 --- a/quic/api/test/QuicTransportFunctionsTest.cpp +++ b/quic/api/test/QuicTransportFunctionsTest.cpp @@ -146,8 +146,8 @@ auto buildEmptyPacket( uint64_t getEncodedSize(const RegularQuicPacketBuilder::Packet& packet) { // calculate size as the plaintext size uint32_t encodedSize = 0; - if (packet.header) { - encodedSize += packet.header->computeChainDataLength(); + if (!packet.header.empty()) { + encodedSize += packet.header.computeChainDataLength(); } if (packet.body) { encodedSize += packet.body->computeChainDataLength(); @@ -1176,7 +1176,7 @@ TEST_F(QuicTransportFunctionsTest, TestUpdateConnectionHandshakeCounter) { auto packet = buildEmptyPacket(*conn, PacketNumberSpace::Handshake); auto packetEncodedSize = - packet.header ? packet.header->computeChainDataLength() : 0; + !packet.header.empty() ? packet.header.computeChainDataLength() : 0; packetEncodedSize += packet.body ? packet.body->computeChainDataLength() : 0; packet.packet.frames.push_back(WriteCryptoFrame(0, 0)); @@ -1191,8 +1191,9 @@ TEST_F(QuicTransportFunctionsTest, TestUpdateConnectionHandshakeCounter) { EXPECT_EQ(1, conn->outstandings.packetCount[PacketNumberSpace::Handshake]); auto nonHandshake = buildEmptyPacket(*conn, PacketNumberSpace::AppData); - packetEncodedSize = - nonHandshake.header ? nonHandshake.header->computeChainDataLength() : 0; + packetEncodedSize = !nonHandshake.header.empty() + ? nonHandshake.header.computeChainDataLength() + : 0; packetEncodedSize += nonHandshake.body ? nonHandshake.body->computeChainDataLength() : 0; auto stream1 = conn->streamManager->createNextBidirectionalStream().value(); diff --git a/quic/codec/QuicPacketBuilder.cpp b/quic/codec/QuicPacketBuilder.cpp index 31cdbea98..95c376b2f 100644 --- a/quic/codec/QuicPacketBuilder.cpp +++ b/quic/codec/QuicPacketBuilder.cpp @@ -130,9 +130,9 @@ RegularQuicPacketBuilder::RegularQuicPacketBuilder( : remainingBytes_(remainingBytes), largestAckedPacketNum_(largestAckedPacketNum), packet_(std::move(header)), - header_(folly::IOBuf::create(kLongHeaderHeaderSize)), + header_(folly::IOBuf::CreateOp::CREATE, kLongHeaderHeaderSize), body_(folly::IOBuf::create(kAppenderGrowthSize)), - headerAppender_(header_.get(), kLongHeaderHeaderSize), + headerAppender_(&header_, kLongHeaderHeaderSize), bodyAppender_(body_.get(), kAppenderGrowthSize) { if (frameHint) { packet_.frames.reserve(frameHint); @@ -143,7 +143,7 @@ uint32_t RegularQuicPacketBuilder::getHeaderBytes() const { bool isLongHeader = packet_.header.getHeaderForm() == HeaderForm::Long; CHECK(packetNumberEncoding_) << "packetNumberEncoding_ should be valid after ctor"; - return folly::to(header_->computeChainDataLength()) + + return folly::to(header_.computeChainDataLength()) + (isLongHeader ? packetNumberEncoding_->length + kMaxPacketLenSize : 0); } @@ -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_); } @@ -409,7 +409,7 @@ RegularSizeEnforcedPacketBuilder::buildPacket() && { // Store counters on the stack to overhead from function calls size_t extraDataWritten = 0; size_t bodyLength = body_->computeChainDataLength(); - size_t headerLength = header_->computeChainDataLength(); + size_t headerLength = header_.computeChainDataLength(); while (extraDataWritten + bodyLength + headerLength + cipherOverhead_ < enforcedSize_) { QuicInteger paddingType(static_cast(FrameType::PADDING)); @@ -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,14 +445,14 @@ 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 headerLength = header_->computeChainDataLength(); + size_t headerLength = header_.computeChainDataLength(); while (extraDataWritten + bodyLength + headerLength + cipherOverhead_ < enforcedSize_) { QuicInteger paddingType(static_cast(FrameType::PADDING)); @@ -728,9 +728,9 @@ PacketBuilderInterface::Packet InplaceQuicPacketBuilder::buildPacket() && { // for encryption. PacketBuilderInterface::Packet builtPacket( std::move(packet_), - (bodyStart_ - ? folly::IOBuf::wrapBuffer(headerStart_, (bodyStart_ - headerStart_)) - : nullptr), + (bodyStart_ ? folly::IOBuf::wrapBufferAsValue( + headerStart_, (bodyStart_ - headerStart_)) + : folly::IOBuf()), (bodyStart_ ? folly::IOBuf::wrapBuffer(bodyStart_, iobuf_->tail() - bodyStart_) : nullptr)); diff --git a/quic/codec/QuicPacketBuilder.h b/quic/codec/QuicPacketBuilder.h index 21a8c9ce4..0d5803ee9 100644 --- a/quic/codec/QuicPacketBuilder.h +++ b/quic/codec/QuicPacketBuilder.h @@ -47,10 +47,10 @@ class PacketBuilderInterface { // and body into a continuous memory. struct Packet { RegularQuicWritePacket packet; - Buf header; + folly::IOBuf header; Buf body; - Packet(RegularQuicWritePacket packetIn, Buf headerIn, Buf bodyIn) + Packet(RegularQuicWritePacket packetIn, folly::IOBuf headerIn, Buf bodyIn) : packet(std::move(packetIn)), header(std::move(headerIn)), body(std::move(bodyIn)) {} @@ -254,7 +254,7 @@ class RegularQuicPacketBuilder final : public PacketBuilderInterface { uint32_t remainingBytes_; PacketNum largestAckedPacketNum_; RegularQuicWritePacket packet_; - std::unique_ptr header_; + folly::IOBuf header_; std::unique_ptr body_; BufAppender headerAppender_; BufAppender bodyAppender_; @@ -303,7 +303,7 @@ class RegularSizeEnforcedPacketBuilder : public WrapperPacketBuilderInterface { private: RegularQuicWritePacket packet_; - Buf header_; + folly::IOBuf header_; Buf body_; BufAppender bodyAppender_; uint64_t enforcedSize_; @@ -339,7 +339,7 @@ class InplaceSizeEnforcedPacketBuilder : public WrapperPacketBuilderInterface { BufAccessor& bufAccessor_; Buf iobuf_; RegularQuicWritePacket packet_; - Buf header_; + folly::IOBuf header_; Buf body_; uint64_t enforcedSize_; uint32_t cipherOverhead_; diff --git a/quic/codec/test/QuicHeaderCodecTest.cpp b/quic/codec/test/QuicHeaderCodecTest.cpp index 11aa81473..f672d6911 100644 --- a/quic/codec/test/QuicHeaderCodecTest.cpp +++ b/quic/codec/test/QuicHeaderCodecTest.cpp @@ -52,7 +52,7 @@ TEST_F(QuicHeaderCodecTest, ShortHeaderTest) { 0 /* largestAcked */); builder.encodePacketHeader(); auto packet = std::move(builder).buildPacket(); - auto result = parseHeader(*packet.header); + auto result = parseHeader(packet.header); auto& header = result->parsedHeader; LongHeader* longHeader = header->asLong(); if (longHeader) { diff --git a/quic/codec/test/QuicPacketBuilderTest.cpp b/quic/codec/test/QuicPacketBuilderTest.cpp index 926f29fda..75fbd8f67 100644 --- a/quic/codec/test/QuicPacketBuilderTest.cpp +++ b/quic/codec/test/QuicPacketBuilderTest.cpp @@ -31,16 +31,16 @@ Buf packetToBuf( auto buf = folly::IOBuf::create(0); // This does not matter. PacketNum num = 10; - if (packet.header) { - buf->prependChain(packet.header->clone()); + if (!packet.header.empty()) { + buf->prependChain(packet.header.clone()); } std::unique_ptr body = folly::IOBuf::create(0); if (packet.body) { body = packet.body->clone(); } - if (aead && packet.header) { + if (aead && !packet.header.empty()) { auto bodySize = body->computeChainDataLength(); - body = aead->inplaceEncrypt(std::move(body), packet.header.get(), num); + body = aead->inplaceEncrypt(std::move(body), &packet.header, num); EXPECT_GT(body->computeChainDataLength(), bodySize); } if (body) { @@ -320,7 +320,7 @@ TEST_P(QuicPacketBuilderTest, EnforcePacketSizeWithCipherOverhead) { EXPECT_TRUE(sizeEnforcedBuilder.canBuildPacket()); auto out = std::move(sizeEnforcedBuilder).buildPacket(); EXPECT_EQ( - out.header->computeChainDataLength() + + out.header.computeChainDataLength() + out.body->computeChainDataLength(), enforcedSize - cipherOverhead); auto buf = packetToBuf(out, aead_); @@ -333,7 +333,7 @@ TEST_P(QuicPacketBuilderTest, EnforcePacketSizeWithCipherOverhead) { EXPECT_TRUE(sizeEnforcedBuilder.canBuildPacket()); auto out = std::move(sizeEnforcedBuilder).buildPacket(); EXPECT_EQ( - out.header->computeChainDataLength() + + out.header.computeChainDataLength() + out.body->computeChainDataLength(), enforcedSize - cipherOverhead); auto buf = packetToBuf(out, aead_); @@ -462,7 +462,7 @@ TEST_P(QuicPacketBuilderTest, LongHeaderBytesCounting) { estimatedHeaderBytes, expectedWrittenHeaderFieldLen + kMaxPacketLenSize); writeFrame(PaddingFrame(), *builder); EXPECT_LE( - std::move(*builder).buildPacket().header->computeChainDataLength(), + std::move(*builder).buildPacket().header.computeChainDataLength(), estimatedHeaderBytes); } @@ -480,7 +480,7 @@ TEST_P(QuicPacketBuilderTest, ShortHeaderBytesCounting) { auto headerBytes = builder->getHeaderBytes(); writeFrame(PaddingFrame(), *builder); EXPECT_EQ( - std::move(*builder).buildPacket().header->computeChainDataLength(), + std::move(*builder).buildPacket().header.computeChainDataLength(), headerBytes); } @@ -546,7 +546,7 @@ TEST_F(QuicPacketBuilderTest, BuildTwoInplaces) { ASSERT_TRUE(builtOut2.packet.frames[0].asPaddingFrame()); EXPECT_EQ(builtOut2.packet.frames[0].asPaddingFrame()->numFrames, 40); - EXPECT_EQ(builtOut2.header->length(), builtOut1.header->length()); + EXPECT_EQ(builtOut2.header.length(), builtOut1.header.length()); EXPECT_EQ(20, builtOut2.body->length() - builtOut1.body->length()); } diff --git a/quic/codec/test/QuicPacketRebuilderTest.cpp b/quic/codec/test/QuicPacketRebuilderTest.cpp index 163fa33eb..380786581 100644 --- a/quic/codec/test/QuicPacketRebuilderTest.cpp +++ b/quic/codec/test/QuicPacketRebuilderTest.cpp @@ -56,7 +56,7 @@ TEST_F(QuicPacketRebuilderTest, RebuildEmpty) { PacketRebuilder rebuilder(regularBuilder, conn); auto packet = std::move(regularBuilder).buildPacket(); EXPECT_TRUE(packet.packet.frames.empty()); - EXPECT_FALSE(packet.header->empty()); + EXPECT_FALSE(packet.header.empty()); EXPECT_TRUE(packet.body->empty()); } @@ -84,13 +84,13 @@ TEST_F(QuicPacketRebuilderTest, RebuildSmallInitial) { PacketRebuilder rebuilder(regularBuilder2, conn); auto packet = std::move(regularBuilder1).buildPacket(); auto outstanding = makeDummyOutstandingPacket(packet.packet, 1000); - EXPECT_FALSE(packet.header->empty()); + EXPECT_FALSE(packet.header.empty()); ASSERT_EQ(packet.packet.frames.size(), 2); EXPECT_FALSE(packet.body->empty()); regularBuilder2.encodePacketHeader(); ASSERT_TRUE(rebuilder.rebuildFromPacket(outstanding).has_value()); auto rebuilt = std::move(regularBuilder2).buildPacket(); - EXPECT_FALSE(rebuilt.header->empty()); + EXPECT_FALSE(rebuilt.header.empty()); ASSERT_EQ(rebuilt.packet.frames.size(), 3); auto padding = rebuilt.packet.frames.back().asPaddingFrame(); ASSERT_TRUE(padding != nullptr); @@ -253,7 +253,7 @@ TEST_F(QuicPacketRebuilderTest, RebuildPacket) { EXPECT_TRUE(false); /* should never happen*/ } } - EXPECT_TRUE(folly::IOBufEqualTo()(*packet1.header, *packet2.header)); + EXPECT_TRUE(folly::IOBufEqualTo()(packet1.header, packet2.header)); // TODO: I don't have a good way to verify body without decode them } @@ -332,7 +332,7 @@ TEST_F(QuicPacketRebuilderTest, FinOnlyStreamRebuild) { packet1.packet.frames.data(), packet2.packet.frames.data(), packet1.packet.frames.size())); - EXPECT_TRUE(folly::IOBufEqualTo()(*packet1.header, *packet2.header)); + 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)); @@ -399,7 +399,7 @@ TEST_F(QuicPacketRebuilderTest, RebuildDataStreamAndEmptyCryptoStream) { EXPECT_EQ(buf->computeChainDataLength(), streamFrame->len); EXPECT_EQ(true, streamFrame->fin); } - EXPECT_TRUE(folly::IOBufEqualTo()(*packet1.header, *packet2.header)); + EXPECT_TRUE(folly::IOBufEqualTo()(packet1.header, packet2.header)); } TEST_F(QuicPacketRebuilderTest, CannotRebuildEmptyCryptoStream) { @@ -487,7 +487,7 @@ TEST_F(QuicPacketRebuilderTest, CannotRebuild) { ShortHeader shortHeader2( ProtectionType::KeyPhaseZero, getTestConnectionId(), 0); RegularQuicPacketBuilder regularBuilder2( - (packet1.header->computeChainDataLength() + + (packet1.header.computeChainDataLength() + packet1.body->computeChainDataLength()) / 2, std::move(shortHeader2), diff --git a/quic/codec/test/QuicReadCodecTest.cpp b/quic/codec/test/QuicReadCodecTest.cpp index b9db22ee5..a83acd531 100644 --- a/quic/codec/test/QuicReadCodecTest.cpp +++ b/quic/codec/test/QuicReadCodecTest.cpp @@ -390,7 +390,7 @@ TEST_F(QuicReadCodecTest, BadResetFirstTwoBits) { true, ProtectionType::KeyPhaseZero); overridePacketWithToken(streamPacket, tok); - uint8_t* packetHeaderBuffer = streamPacket.header.get()->writableData(); + uint8_t* packetHeaderBuffer = streamPacket.header.writableData(); while (*packetHeaderBuffer & 0x40) { uint8_t randomByte; folly::Random::secureRandom(&randomByte, 1); @@ -437,7 +437,7 @@ TEST_F(QuicReadCodecTest, RandomizedShortHeaderLeadsToReset) { true, ProtectionType::KeyPhaseZero); overridePacketWithToken(streamPacket, tok); - uint8_t* packetHeaderBuffer = streamPacket.header.get()->writableData(); + uint8_t* packetHeaderBuffer = streamPacket.header.writableData(); uint8_t randomByte; folly::Random::secureRandom(&randomByte, 1); *packetHeaderBuffer = 0x40 | (randomByte & 0b00111111); diff --git a/quic/codec/test/TypesTest.cpp b/quic/codec/test/TypesTest.cpp index a0c61b3ed..e27282399 100644 --- a/quic/codec/test/TypesTest.cpp +++ b/quic/codec/test/TypesTest.cpp @@ -27,7 +27,7 @@ std::pair encodeShortHeader(const ShortHeader& header) { builder.encodePacketHeader(); auto packet = std::move(builder).buildPacket(); Buf out; - folly::io::Cursor cursor(packet.header.get()); + folly::io::Cursor cursor(&packet.header); auto initialByte = cursor.readBE(); cursor.clone(out, cursor.totalLength()); return std::make_pair(initialByte, std::move(out)); diff --git a/quic/common/test/TestUtils.cpp b/quic/common/test/TestUtils.cpp index f9ba107d1..21fb17b64 100644 --- a/quic/common/test/TestUtils.cpp +++ b/quic/common/test/TestUtils.cpp @@ -391,7 +391,7 @@ RegularQuicPacketBuilder::Packet createCryptoPacket( } Buf packetToBuf(const RegularQuicPacketBuilder::Packet& packet) { - auto packetBuf = packet.header->clone(); + auto packetBuf = packet.header.clone(); if (packet.body) { packetBuf->prependChain(packet.body->clone()); } @@ -399,13 +399,13 @@ Buf packetToBuf(const RegularQuicPacketBuilder::Packet& packet) { } Buf packetToBufCleartext( - const RegularQuicPacketBuilder::Packet& packet, + RegularQuicPacketBuilder::Packet& packet, const Aead& cleartextCipher, const PacketNumberCipher& headerCipher, PacketNum packetNum) { VLOG(10) << __func__ << " packet header: " - << folly::hexlify(packet.header->clone()->moveToFbString()); - auto packetBuf = packet.header->clone(); + << folly::hexlify(packet.header.clone()->moveToFbString()); + auto packetBuf = packet.header.clone(); Buf body; if (packet.body) { packet.body->coalesce(); @@ -414,19 +414,19 @@ Buf packetToBufCleartext( body = folly::IOBuf::create(0); } auto headerForm = packet.packet.header.getHeaderForm(); - packet.header->coalesce(); + packet.header.coalesce(); auto tagLen = cleartextCipher.getCipherOverhead(); if (body->tailroom() < tagLen) { body->prependChain(folly::IOBuf::create(tagLen)); } body->coalesce(); auto encryptedBody = cleartextCipher.inplaceEncrypt( - std::move(body), packet.header.get(), packetNum); + std::move(body), &packet.header, packetNum); encryptedBody->coalesce(); encryptPacketHeader( headerForm, - packet.header->writableData(), - packet.header->length(), + packet.header.writableData(), + packet.header.length(), encryptedBody->data(), encryptedBody->length(), headerCipher); @@ -434,6 +434,14 @@ Buf packetToBufCleartext( return packetBuf; } +Buf packetToBufCleartext( + RegularQuicPacketBuilder::Packet&& packet, + const Aead& cleartextCipher, + const PacketNumberCipher& headerCipher, + PacketNum packetNum) { + return packetToBufCleartext(packet, cleartextCipher, headerCipher, packetNum); +} + uint64_t computeExpectedDelay( std::chrono::microseconds ackDelay, uint8_t ackDelayExponent) { diff --git a/quic/common/test/TestUtils.h b/quic/common/test/TestUtils.h index 2de5178f6..537a92846 100644 --- a/quic/common/test/TestUtils.h +++ b/quic/common/test/TestUtils.h @@ -119,7 +119,13 @@ RegularQuicPacketBuilder::Packet createCryptoPacket( Buf packetToBuf(const RegularQuicPacketBuilder::Packet& packet); Buf packetToBufCleartext( - const RegularQuicPacketBuilder::Packet& packet, + RegularQuicPacketBuilder::Packet& packet, + const Aead& cleartextCipher, + const PacketNumberCipher& headerCipher, + PacketNum packetNum); + +Buf packetToBufCleartext( + RegularQuicPacketBuilder::Packet&& packet, const Aead& cleartextCipher, const PacketNumberCipher& headerCipher, PacketNum packetNum); diff --git a/quic/dsr/backend/DSRPacketizer.cpp b/quic/dsr/backend/DSRPacketizer.cpp index 77a26eca7..0c0d9957a 100644 --- a/quic/dsr/backend/DSRPacketizer.cpp +++ b/quic/dsr/backend/DSRPacketizer.cpp @@ -75,22 +75,22 @@ bool writeSingleQuicPacket( ioBufBatch.flush(); return false; } - CHECK(!packet.header->isChained()); + CHECK(!packet.header.isChained()); - auto headerLen = packet.header->length(); + auto headerLen = packet.header.length(); buildBuf = accessor.obtain(); CHECK( packet.body->data() > buildBuf->data() && packet.body->tail() <= buildBuf->tail()); CHECK( - packet.header->data() >= buildBuf->data() && - packet.header->tail() < buildBuf->tail()); + packet.header.data() >= buildBuf->data() && + packet.header.tail() < buildBuf->tail()); // Trim off everything before the current packet, and the header length, so // buildBuf's data starts from the body part of buildBuf. buildBuf->trimStart(prevSize + headerLen); // buildBuf and packetbuildBuf is actually the same. auto packetbuildBuf = - aead.inplaceEncrypt(std::move(buildBuf), packet.header.get(), packetNum); + aead.inplaceEncrypt(std::move(buildBuf), &packet.header, packetNum); CHECK_EQ(packetbuildBuf->headroom(), headerLen + prevSize); // Include header back. packetbuildBuf->prepend(headerLen); diff --git a/quic/fizz/client/test/QuicClientTransportTest.cpp b/quic/fizz/client/test/QuicClientTransportTest.cpp index 8c5dd00ff..a372070e5 100644 --- a/quic/fizz/client/test/QuicClientTransportTest.cpp +++ b/quic/fizz/client/test/QuicClientTransportTest.cpp @@ -5802,7 +5802,7 @@ TEST_P(QuicProcessDataTest, ProcessDataHeaderOnly) { aead, 0 /* largestAcked */); - deliverData(serverAddr, packet.header->coalesce()); + deliverData(serverAddr, packet.header.coalesce()); EXPECT_EQ( getAckState(client->getConn(), PacketNumberSpace::Handshake) .largestRecvdPacketNum, diff --git a/quic/loss/test/QuicLossFunctionsTest.cpp b/quic/loss/test/QuicLossFunctionsTest.cpp index 7d860f1fa..22813aa3b 100644 --- a/quic/loss/test/QuicLossFunctionsTest.cpp +++ b/quic/loss/test/QuicLossFunctionsTest.cpp @@ -251,8 +251,8 @@ PacketNum QuicLossFunctionsTest::sendPacket( } uint32_t encodedSize = 0; uint32_t encodedBodySize = 0; - if (packet.header) { - encodedSize += packet.header->computeChainDataLength(); + if (!packet.header.empty()) { + encodedSize += packet.header.computeChainDataLength(); } if (packet.body) { encodedSize += packet.body->computeChainDataLength(); diff --git a/quic/state/test/AckHandlersTest.cpp b/quic/state/test/AckHandlersTest.cpp index b4cc3d2b1..b5c81a9f1 100644 --- a/quic/state/test/AckHandlersTest.cpp +++ b/quic/state/test/AckHandlersTest.cpp @@ -4412,8 +4412,8 @@ class AckEventForAppDataTest : public Test { uint64_t getEncodedSize(const RegularQuicPacketBuilder::Packet& packet) { // calculate size as the plaintext size uint32_t encodedSize = 0; - if (packet.header) { - encodedSize += packet.header->computeChainDataLength(); + if (!packet.header.empty()) { + encodedSize += packet.header.computeChainDataLength(); } if (packet.body) { encodedSize += packet.body->computeChainDataLength();