Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
129 changes: 99 additions & 30 deletions proxygen/lib/http/codec/HQFramedCodec.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,68 @@
#include <folly/io/Cursor.h>
#include <folly/tracing/ScopedTraceSection.h>

#include <algorithm>
#include <iomanip>
#include <limits>

namespace proxygen::hq {

using namespace folly::io;
using namespace folly;

namespace {
// CANCEL_PUSH, GOAWAY and MAX_PUSH_ID carry a single varint.
constexpr uint64_t kMaxIdFrameLength = 8;
constexpr uint64_t kMaxControlFrameLength = 4096;
// Partial frames in longer chains are copied to keep rescans short.
constexpr size_t kMaxPendingChainElements = 10;
constexpr size_t kPendingPayloadAllocSize = 4096;

// folly::none for frame types that are skipped.
folly::Optional<uint64_t> maxFrameLength(FrameType type) {
switch (type) {
case FrameType::DATA:
case FrameType::HEADERS:
case FrameType::PUSH_PROMISE:
return std::numeric_limits<uint64_t>::max();
case FrameType::CANCEL_PUSH:
case FrameType::GOAWAY:
case FrameType::MAX_PUSH_ID:
return kMaxIdFrameLength;
case FrameType::SETTINGS:
case FrameType::PRIORITY_UPDATE:
case FrameType::PUSH_PRIORITY_UPDATE:
case FrameType::FB_PRIORITY_UPDATE:
case FrameType::FB_PUSH_PRIORITY_UPDATE:
return kMaxControlFrameLength;
case FrameType::PADDING:
case FrameType::WEBTRANSPORT_BIDI:
default:
return folly::none;
}
}
} // namespace

ParseResult HQFramedCodec::checkFrameLength(const FrameHeader& header) {
if (header.length > maxFrameLength(header.type).value_or(0)) {
setParseErrorContext(
"frame-too-large",
folly::to<std::string>("type=", getFrameTypeString(header.type)));
return HTTP3::ErrorCode::HTTP_FRAME_ERROR;
}
return folly::none;
}

size_t HQFramedCodec::bufferPayload(Cursor& cursor, size_t bufLen) {
const uint64_t missing = curHeader_.length - pendingPayload_.chainLength();
const size_t toCopy = std::min<uint64_t>(missing, bufLen);
QueueAppender appender(&pendingPayload_,
std::min<uint64_t>(missing, kPendingPayloadAllocSize));
appender.push(cursor, toCopy);
cursor.skip(toCopy);
return toCopy;
}

ParseResult HQFramedCodec::parseFrame(Cursor& cursor) {
switch (curHeader_.type) {
case hq::FrameType::DATA:
Expand All @@ -43,25 +98,15 @@ ParseResult HQFramedCodec::parseFrame(Cursor& cursor) {
case hq::FrameType::FB_PUSH_PRIORITY_UPDATE:
return parsePriorityUpdate(cursor, curHeader_);
default:
// Implementations MUST ignore and discard any frame that has a
// type that is unknown
if (callback_) {
callback_->onUnknownFrame(streamId_,
static_cast<uint64_t>(curHeader_.type));
}
break;
LOG(DFATAL) << "Unparsed frame type=" << curHeader_.type;
cursor.skip(curHeader_.length);
return folly::none;
}

VLOG(3) << "Skipping frame (type=" << (uint64_t)curHeader_.type << ")";
cursor.skip(curHeader_.length);
return folly::none;
}

size_t HQFramedCodec::onFramedIngress(const IOBuf& buf) {
FOLLY_SCOPED_TRACE_SECTION("HQFramedCodec - onFramedIngress");
// if for some reason onFramedIngress gets called again after erroring out,
// skip parsing
if (connError_ != folly::none) {
if (connError_ != folly::none || parserPaused_) {
return 0;
}
Cursor cursor(&buf);
Expand Down Expand Up @@ -111,40 +156,62 @@ size_t HQFramedCodec::onFramedIngress(const IOBuf& buf) {
receivedFrameCount_++;
#endif
pendingDataFrameBytes_ = curHeader_.length;
if (curHeader_.length == 0) {
if (!maxFrameLength(curHeader_.type)) {
// Implementations MUST ignore and discard any frame that has a
// type that is unknown
if (callback_) {
callback_->onUnknownFrame(streamId_,
static_cast<uint64_t>(curHeader_.type));
}
frameState_ = curHeader_.length == 0
? FrameState::FRAME_HEADER_TYPE
: FrameState::FRAME_PAYLOAD_STREAMING;
} else if (curHeader_.length == 0) {
// If the frame length is zero, call parseFrame immediately. It is up
// to each frame to determine whether length 0 is valid.
connError_ = parseFrame(cursor);
frameState_ = FrameState::FRAME_HEADER_TYPE;
} else if (curHeader_.type == FrameType::DATA) {
frameState_ = FrameState::FRAME_PAYLOAD_STREAMING;
} else {
// For DATA frames, move to the streaming state
if (curHeader_.type == FrameType::DATA) {
frameState_ = FrameState::FRAME_PAYLOAD_STREAMING;
} else {
frameState_ = FrameState::FRAME_PAYLOAD;
}
connError_ = checkFrameLength(curHeader_);
frameState_ = FrameState::FRAME_PAYLOAD;
}
} else if (frameState_ == FrameState::FRAME_PAYLOAD) {
// Already parsed the common frame header
const auto frameLen = curHeader_.length;
if (bufLen >= frameLen) {
if (pendingPayload_.empty() && bufLen >= frameLen) {
connError_ = parseFrame(cursor);
// if connError_ is set, it means there was a frame error,
// so it doesn't really matter whether we have actually parsed all the
// data or not
parsed += curHeader_.length;
frameState_ = FrameState::FRAME_HEADER_TYPE;
} else {
} else if (pendingPayload_.empty() &&
buf.countChainElements() <= kMaxPendingChainElements) {
break;
} else {
parsed += bufferPayload(cursor, bufLen);
if (pendingPayload_.chainLength() == frameLen) {
auto payload = pendingPayload_.move();
Cursor payloadCursor(payload.get());
connError_ = parseFrame(payloadCursor);
frameState_ = FrameState::FRAME_HEADER_TYPE;
}
}
} else if (bufLen > 0 &&
frameState_ == FrameState::FRAME_PAYLOAD_STREAMING) {
FrameHeader auxDataFrameHeader;
auxDataFrameHeader.type = FrameType::DATA;
auxDataFrameHeader.length = std::min(pendingDataFrameBytes_, bufLen);
connError_ = parseData(cursor, auxDataFrameHeader);
parsed += auxDataFrameHeader.length;
pendingDataFrameBytes_ -= auxDataFrameHeader.length;
const auto chunkLen = std::min(pendingDataFrameBytes_, bufLen);
if (curHeader_.type == FrameType::DATA) {
FrameHeader auxDataFrameHeader;
auxDataFrameHeader.type = FrameType::DATA;
auxDataFrameHeader.length = chunkLen;
connError_ = parseData(cursor, auxDataFrameHeader);
} else {
cursor.skip(chunkLen);
}
parsed += chunkLen;
pendingDataFrameBytes_ -= chunkLen;
if (pendingDataFrameBytes_ == 0) {
frameState_ = FrameState::FRAME_HEADER_TYPE;
}
Expand All @@ -154,7 +221,9 @@ size_t HQFramedCodec::onFramedIngress(const IOBuf& buf) {
parsedTot += parsed;
totalBytesParsed_ += parsed;
}
checkConnectionError(connError_, &buf);
if (!checkConnectionError(connError_, &buf) && parserPaused_ && pauseHook_) {
pauseHook_();
}
return parsedTot;
}

Expand Down
11 changes: 11 additions & 0 deletions proxygen/lib/http/codec/HQFramedCodec.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include <proxygen/lib/http/codec/HQFramer.h>
#include <proxygen/lib/http/codec/HTTPCodec.h>

#include <folly/io/IOBufQueue.h>
#include <folly/lang/Assume.h>

namespace proxygen::hq {
Expand Down Expand Up @@ -71,6 +72,11 @@ class HQFramedCodec : public HTTPCodec {
resumeHook_ = std::move(resumeHook);
}

// Invoked when ingress parsing stops with the parser paused.
void setPauseHook(folly::Function<void()> pauseHook) {
pauseHook_ = std::move(pauseHook);
}

bool isParserPaused() const override {
return parserPaused_;
}
Expand Down Expand Up @@ -265,6 +271,7 @@ class HQFramedCodec : public HTTPCodec {
}

virtual ParseResult checkFrameAllowed(FrameType type) = 0;
virtual ParseResult checkFrameLength(const FrameHeader& header);

virtual ParseResult parseData(folly::io::Cursor& /*cursor*/,
const FrameHeader& /*header*/) {
Expand Down Expand Up @@ -337,8 +344,11 @@ class HQFramedCodec : public HTTPCodec {
ParseResult parseFrame(folly::io::Cursor& cursor);
bool checkConnectionError(ParseResult err, const folly::IOBuf* buf);

size_t bufferPayload(folly::io::Cursor& cursor, size_t bufLen);

// Current frame state
size_t pendingDataFrameBytes_{0};
folly::IOBufQueue pendingPayload_{folly::IOBufQueue::cacheChainLength()};

#ifndef NDEBUG
uint64_t receivedFrameCount_{0};
Expand All @@ -356,6 +366,7 @@ class HQFramedCodec : public HTTPCodec {
std::string parseErrorAdditionalInfo_;
uint64_t totalBytesParsed_{0};
folly::Function<void()> resumeHook_;
folly::Function<void()> pauseHook_;
};

} // namespace proxygen::hq
18 changes: 18 additions & 0 deletions proxygen/lib/http/codec/HQStreamCodec.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
#include <proxygen/lib/http/codec/HQUtils.h>
#include <proxygen/lib/http/codec/compress/QPACKCodec.h>

#include <limits>

namespace proxygen::hq {

using namespace folly;
Expand Down Expand Up @@ -101,6 +103,22 @@ ParseResult HQStreamCodec::checkFrameAllowed(FrameType type) {
return folly::none;
}

ParseResult HQStreamCodec::checkFrameLength(const FrameHeader& header) {
if (header.type != hq::FrameType::HEADERS &&
header.type != hq::FrameType::PUSH_PROMISE) {
return HQFramedCodec::checkFrameLength(header);
}
if (header.length > headerCodec_.getMaxUncompressed()) {
if (header.type == hq::FrameType::HEADERS && !finalIngressHeadersSeen_ &&
callback_) {
callback_->onMessageBegin(streamId_, nullptr);
}
setParserPaused(true);
onDecodeError(HPACK::DecodeError::HEADERS_TOO_LARGE);
}
return folly::none;
}

ParseResult HQStreamCodec::parseData(Cursor& cursor,
const FrameHeader& header) {
// NOTE: If an error path is added to this method, it needs to setParserPaused
Expand Down
1 change: 1 addition & 0 deletions proxygen/lib/http/codec/HQStreamCodec.h
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ class HQStreamCodec

protected:
ParseResult checkFrameAllowed(FrameType type) override;
ParseResult checkFrameLength(const FrameHeader& header) override;
ParseResult parseData(folly::io::Cursor& cursor,
const FrameHeader& header) override;
ParseResult parseHeaders(folly::io::Cursor& cursor,
Expand Down
8 changes: 4 additions & 4 deletions proxygen/lib/http/codec/compress/HPACKDecodeBuffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -69,15 +69,15 @@ DecodeError HPACKDecodeBuffer::decodeLiteral(uint8_t nbit,
EOB_LOG("Could not decode literal size", result);
return result;
}
if (size > maxLiteralSize_) {
LOG(ERROR) << "Literal too large, size=" << size;
return DecodeError::LITERAL_TOO_LARGE;
}
if (size > remainingBytes_) {
EOB_LOG(folly::to<std::string>(
"size(", size, ") > remainingBytes_(", remainingBytes_, ")"));
return DecodeError::BUFFER_UNDERFLOW;
}
if (size > maxLiteralSize_) {
LOG(ERROR) << "Literal too large, size=" << size;
return DecodeError::LITERAL_TOO_LARGE;
}
const uint8_t* data;
unique_ptr<IOBuf> tmpbuf;
// handle the case where the buffer spans multiple buffers
Expand Down
21 changes: 17 additions & 4 deletions proxygen/lib/http/codec/compress/QPACKDecoder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@

#include <proxygen/lib/http/codec/compress/HPACKEncodeBuffer.h>

#include <algorithm>
#include <limits>

using folly::io::Cursor;

namespace {
Expand Down Expand Up @@ -201,10 +204,14 @@ HPACK::DecodeError QPACKDecoder::decodeEncoderStream(
std::unique_ptr<folly::IOBuf> buf) {
ingress_.append(std::move(buf));
Cursor cursor(ingress_.front());
HPACKDecodeBuffer dbuf(cursor,
ingress_.chainLength(),
maxUncompressed_,
/* endOfBufferIsError=*/false);
// Every inserted literal must fit in the dynamic table. A Huffman code is at
// most 30 bits, so the encoded form can be up to 4x the decoded size.
HPACKDecodeBuffer dbuf(
cursor,
ingress_.chainLength(),
uint32_t(std::min<uint64_t>(uint64_t(maxTableSize_) * 4,
std::numeric_limits<uint32_t>::max())),
/* endOfBufferIsError=*/false);

VLOG(6) << "Decoding control block";
baseIndex_ = 0;
Expand Down Expand Up @@ -321,6 +328,12 @@ uint32_t QPACKDecoder::decodeLiteralHeaderQ(
LOG(ERROR) << "Error decoding header name err_=" << err_;
return 0;
}
if (indexing &&
headerName.size() + HPACKHeader::kMinLength > table_.capacity()) {
LOG(ERROR) << "Encoder inserted a name larger than capacity";
err_ = HPACK::DecodeError::INSERT_TOO_LARGE;
return 0;
}
partial->header.name = HPACKHeaderName{headerName};
}
partial->state = Partial::VALUE;
Expand Down
24 changes: 24 additions & 0 deletions proxygen/lib/http/codec/compress/test/QPACKContextTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -909,6 +909,30 @@ TEST(QPACKContextTests, RejectsOverflowingRequiredInsertCount) {
decoder, headerBlock.release(), HPACK::DecodeError::INVALID_INDEX);
}

TEST(QPACKContextTests, EncoderStreamLiteralLargerThanTable) {
QPACKDecoder decoder(HPACK::kTableSize);
HPACKEncodeBuffer partial(128, false);
partial.encodeInteger(100, HPACK::Q_INSERT_NO_NAME_REF);
EXPECT_EQ(decoder.decodeEncoderStream(partial.release()),
HPACK::DecodeError::NONE);

QPACKDecoder decoder2(HPACK::kTableSize);
HPACKEncodeBuffer tooLarge(128, false);
tooLarge.encodeInteger(4 * HPACK::kTableSize + 1,
HPACK::Q_INSERT_NO_NAME_REF);
EXPECT_EQ(decoder2.decodeEncoderStream(tooLarge.release()),
HPACK::DecodeError::LITERAL_TOO_LARGE);

QPACKDecoder decoder3(HPACK::kTableSize);
HPACKEncodeBuffer bigName(128, false);
bigName.encodeLiteral(
HPACK::Q_INSERT_NO_NAME_REF.code,
HPACK::Q_INSERT_NO_NAME_REF.prefixLength,
string(HPACK::kTableSize - HPACKHeader::kMinLength + 1, 'x'));
EXPECT_EQ(decoder3.decodeEncoderStream(bigName.release()),
HPACK::DecodeError::INSERT_TOO_LARGE);
}

TEST(QPACKContextTests, DecodeErrors) {
QPACKDecoder decoder(128);
unique_ptr<IOBuf> buf = IOBuf::create(128);
Expand Down
Loading
Loading