lib/proxygen
1
0
Fork 0
mirror of https://github.com/facebook/proxygen.git synced 2026-10-24 01:33:54 +03:00
Code Activity

HTTP2Codec: log parse-error context

Browse Source 
Summary:
Same attribution for HTTP/2. `checkConnectionError` reported every framer
failure as the single literal `Connection error`, so roughly thirty distinct
rejection paths in `HTTP2Framer` were indistinguishable in `debug_data`.

Frame parsing now records a reason via `setParseErrorContext`:

  GOAWAY error: excessively large frame len=32768
  [File]=HTTP2Codec.cpp [Context]=frame-exceeds-max-size

Reviewed By: afrind

Differential Revision: D114943825

fbshipit-source-id: e2e0ae29fed562b96e8fabaeb38641eef558cc62
This commit is contained in:
Sujay Patel
2026-09-21 10:02:08 -07:00
committed by meta-codesync[bot]
parent d1a1362316
commit df727477da
7 changed files with 211 additions and 130 deletions
Download Patch File Download Diff File Expand all files Collapse all files

245
proxygen/lib/http/codec/HTTP2Codec.cpp
View File

@@ -10,6 +10,7 @@
#include <folly/base64.h>
#include <proxygen/lib/http/HTTPPriorityFunctions.h>
#include <proxygen/lib/http/codec/CodecUtil.h>
#include <proxygen/lib/http/codec/HTTP2Constants.h>
#include <proxygen/lib/utils/Logging.h>
@@ -28,6 +29,14 @@ namespace {
const size_t kDefaultGrowth = 4000;
} // namespace
#define RETURN_IF_ERROR_WITH_CTX(errExpr, context) \
do { \
if (auto ctxErr = (errExpr); ctxErr != ErrorCode::NO_ERROR) { \
setParseErrorContext(context); \
return ctxErr; \
} \
} while (0)
namespace proxygen {
HTTP2Codec::HTTP2Codec(TransportDirection direction)
@@ -66,8 +75,7 @@ size_t HTTP2Codec::onIngress(const folly::IOBuf& buf) noexcept {
auto test = cursor.readFixedString(http2::kConnectionPreface.length());
parsed += http2::kConnectionPreface.length();
if (test != http2::kConnectionPreface) {
goawayErrorMessage_ = "missing connection preface";
VLOG(4) << goawayErrorMessage_;
setParseErrorContext("missing-connection-preface");
connError = ErrorCode::PROTOCOL_ERROR;
}
frameState_ = FrameState::EXPECT_FIRST_SETTINGS;
@@ -79,22 +87,27 @@ size_t HTTP2Codec::onIngress(const folly::IOBuf& buf) noexcept {
// Waiting to parse the common frame header
if (remaining >= http2::kFrameHeaderSize) {
connError = parseFrameHeader(cursor, curHeader_);
if (connError != ErrorCode::NO_ERROR) {
setParseErrorContext("invalid-frame-header");
}
parsed += http2::kFrameHeaderSize;
if (frameState_ == FrameState::EXPECT_FIRST_SETTINGS &&
curHeader_.type != http2::FrameType::SETTINGS) {
goawayErrorMessage_ = folly::to<string>(
"GOAWAY error: got invalid connection preface frame type=",
getFrameTypeString(curHeader_.type),
"(",
curHeader_.type,
")",
" for streamID=",
curHeader_.stream);
VLOG(4) << goawayErrorMessage_;
setParseErrorContext(
"invalid-connection-preface-frame-type",
folly::to<string>("type=",
getFrameTypeString(curHeader_.type),
"(",
curHeader_.type,
") stream=",
curHeader_.stream));
connError = ErrorCode::PROTOCOL_ERROR;
}
if (curHeader_.length > maxRecvFrameSize()) {
VLOG(4) << "Excessively large frame len=" << curHeader_.length;
setParseErrorContext(
"frame-exceeds-max-size",
folly::to<string>(
"len=", curHeader_.length, " max=", maxRecvFrameSize()));
connError = ErrorCode::FRAME_SIZE_ERROR;
}
@@ -152,25 +165,25 @@ size_t HTTP2Codec::onIngress(const folly::IOBuf& buf) noexcept {
ErrorCode HTTP2Codec::parseFrame(folly::io::Cursor& cursor) {
FOLLY_SCOPED_TRACE_SECTION("HTTP2Codec - parseFrame");
parseErrorContext_.clear();
parseErrorAdditionalInfo_.clear();
if (expectedContinuationStream_ != 0 &&
(curHeader_.type != http2::FrameType::CONTINUATION ||
expectedContinuationStream_ != curHeader_.stream)) {
goawayErrorMessage_ = folly::to<string>(
"GOAWAY error: while expected CONTINUATION with stream=",
expectedContinuationStream_,
", received streamID=",
curHeader_.stream,
" of type=",
getFrameTypeString(curHeader_.type));
VLOG(4) << goawayErrorMessage_;
setParseErrorContext(
"continuation-stream-mismatch",
folly::to<string>("expected=",
expectedContinuationStream_,
" received=",
curHeader_.stream,
" type=",
getFrameTypeString(curHeader_.type)));
return ErrorCode::PROTOCOL_ERROR;
}
if (expectedContinuationStream_ == 0 &&
curHeader_.type == http2::FrameType::CONTINUATION) {
goawayErrorMessage_ = folly::to<string>(
"GOAWAY error: unexpected CONTINUATION received with streamID=",
curHeader_.stream);
VLOG(4) << goawayErrorMessage_;
setParseErrorContext("unexpected-continuation-frame",
folly::to<string>("stream=", curHeader_.stream));
return ErrorCode::PROTOCOL_ERROR;
}
if (frameAffectsCompression(curHeader_.type) &&
@@ -185,18 +198,20 @@ ErrorCode HTTP2Codec::parseFrame(folly::io::Cursor& cursor) {
// block to keep the connection state consistent without consuming
// memory, and fail just the request per the HTTP/2 spec (section
// 10.3)
goawayErrorMessage_ = folly::to<string>(
"Failing connection due to excessively large headers");
LOG(ERROR) << goawayErrorMessage_;
setParseErrorContext(
"header-block-exceeds-max-size",
folly::to<string>(
"len=",
curHeaderBlock_.chainLength() + curHeader_.length,
" max=",
egressSettings_.getSetting(SettingsId::MAX_HEADER_LIST_SIZE, 0)));
return ErrorCode::PROTOCOL_ERROR;
}
if (curHeader_.type == http2::FrameType::CONTINUATION) {
if (continuationFramesLeftInHeaderBlock_ == 0) {
goawayErrorMessage_ = folly::to<string>(
"GOAWAY error: too many CONTINUATION frames for stream=",
curHeader_.stream);
LOG(ERROR) << goawayErrorMessage_;
setParseErrorContext("too-many-continuation-frames",
folly::to<string>("stream=", curHeader_.stream));
return ErrorCode::ENHANCE_YOUR_CALM;
}
--continuationFramesLeftInHeaderBlock_;
@@ -284,7 +299,7 @@ ErrorCode HTTP2Codec::parseAllData(Cursor& cursor) {
VLOG(10) << "parsing all frame DATA bytes for stream=" << curHeader_.stream
<< " length=" << curHeader_.length;
auto ret = http2::parseData(cursor, curHeader_, outData, padding);
RETURN_IF_ERROR(ret);
RETURN_IF_ERROR_WITH_CTX(ret, "parse-data-frame");
if (callback_ && (padding > 0 || (outData && !outData->empty()))) {
if (!outData) {
@@ -324,7 +339,7 @@ ErrorCode HTTP2Codec::parseDataFrameData(Cursor& cursor,
return ErrorCode::NO_ERROR;
}
const auto ret = http2::parseDataBegin(cursor, curHeader_, parsed, padding);
RETURN_IF_ERROR(ret);
RETURN_IF_ERROR_WITH_CTX(ret, "parse-data-frame-begin");
if (padding > 0) {
pendingDataFramePaddingBytes_ = padding - 1;
pendingDataFrameBytes_--;
@@ -357,7 +372,7 @@ ErrorCode HTTP2Codec::parseDataFrameData(Cursor& cursor,
size_t toSkip = 0;
auto ret = http2::parseDataEnd(
cursor, bufLen, pendingDataFramePaddingBytes_, toSkip);
RETURN_IF_ERROR(ret);
RETURN_IF_ERROR_WITH_CTX(ret, "parse-data-frame-end");
pendingDataFrameBytes_ -= toSkip;
pendingDataFramePaddingBytes_ -= toSkip;
parsed += toSkip;
@@ -387,10 +402,11 @@ ErrorCode HTTP2Codec::parseHeaders(Cursor& cursor) {
VLOG(4) << "parsing HEADERS frame for stream=" << curHeader_.stream
<< " length=" << curHeader_.length;
auto err = http2::parseHeaders(cursor, curHeader_, headerBuf);
RETURN_IF_ERROR(err);
RETURN_IF_ERROR_WITH_CTX(err, "parse-headers-frame");
if (isDownstream(transportDirection_)) {
RETURN_IF_ERROR(
checkNewStream(curHeader_.stream, true /* trailersAllowed */));
auto newStreamErr =
checkNewStream(curHeader_.stream, true /* trailersAllowed */);
RETURN_IF_ERROR(newStreamErr);
}
err = parseHeadersImpl(cursor, std::move(headerBuf), folly::none);
return err;
@@ -401,7 +417,7 @@ ErrorCode HTTP2Codec::parseContinuation(Cursor& cursor) {
VLOG(4) << "parsing CONTINUATION frame for stream=" << curHeader_.stream
<< " length=" << curHeader_.length;
auto err = http2::parseContinuation(cursor, curHeader_, headerBuf);
RETURN_IF_ERROR(err);
RETURN_IF_ERROR_WITH_CTX(err, "parse-continuation-frame");
err = parseHeadersImpl(cursor, std::move(headerBuf), folly::none);
return err;
}
@@ -502,9 +518,8 @@ ErrorCode HTTP2Codec::parseHeadersImpl(
} else {
if (isUpstream(transportDirection_) && curHeader_.stream & 0x01 &&
curHeader_.stream >= nextEgressStreamID_) {
goawayErrorMessage_ = folly::to<std::string>(
"HEADERS on idle upstream stream=", curHeader_.stream);
LOG(ERROR) << goawayErrorMessage_;
setParseErrorContext("headers-on-idle-upstream-stream",
folly::to<string>("stream=", curHeader_.stream));
return ErrorCode::PROTOCOL_ERROR;
}
deliverCallbackIfAllowed(&HTTPCodec::Callback::onHeadersComplete,
@@ -538,22 +553,21 @@ HTTP2Codec::parseHeadersDecodeFrames() {
auto g = folly::makeGuard([this] { curHeaderBlock_.move(); });
// Check decoding error
if (decodeInfo_.decodeError != HPACK::DecodeError::NONE) {
constexpr std::string_view kDecodeErrorMessage =
"Failed decoding header block for stream=";
// Avoid logging header blocks that have failed decoding due to being
// excessively large.
if (decodeInfo_.decodeError != HPACK::DecodeError::HEADERS_TOO_LARGE) {
goawayErrorMessage_ =
folly::to<std::string>(kDecodeErrorMessage,
curHeader_.stream,
": decompression error=",
uint32_t(decodeInfo_.decodeError));
LOG(ERROR) << goawayErrorMessage_
setParseErrorContext(
"header-block-decode-failed",
folly::to<string>("stream=",
curHeader_.stream,
" decompression-error=",
uint32_t(decodeInfo_.decodeError)));
LOG(ERROR) << "Failed decoding header block " << parseErrorAdditionalInfo_
<< (VLOG_IS_ON(3) ? ", header block=" : "");
VLOG(3) << IOBufPrinter::printHexFolly(curHeaderBlock_.front(), true);
} else {
goawayErrorMessage_ = folly::to<std::string>(
kDecodeErrorMessage, curHeader_.stream, ": headers too large");
setParseErrorContext("header-block-too-large",
folly::to<string>("stream=", curHeader_.stream));
if (debugLevel_ > 0 && msg) {
LOG(ERROR) << "HPACK Headers too large"
<< CodecUtil::debugString(*msg, debugLevel_)
@@ -599,7 +613,9 @@ HTTP2Codec::parseHeadersDecodeFrames() {
ErrorCode::PROTOCOL_ERROR,
false,
folly::to<string>("Field section parsing failed txn=",
curHeader_.stream),
curHeader_.stream,
" error: ",
decodeInfo_.parsingError),
std::move(msg)));
}
}
@@ -701,7 +717,7 @@ ErrorCode HTTP2Codec::parseRFC9218Priority(Cursor& cursor) {
std::string pri;
uint32_t priStream = 0;
auto err = http2::parseRFC9218Priority(cursor, curHeader_, priStream, pri);
RETURN_IF_ERROR(err);
RETURN_IF_ERROR_WITH_CTX(err, "parse-priority-frame");
auto httpPri = httpPriorityFromString(pri);
auto onPriFunc =
static_cast<void (HTTPCodec::Callback::*)(StreamID, const HTTPPriority&)>(
@@ -718,7 +734,7 @@ ErrorCode HTTP2Codec::parseRstStream(Cursor& cursor) {
upgradedStreams_.erase(curHeader_.stream);
ErrorCode statusCode = ErrorCode::NO_ERROR;
auto err = http2::parseRstStream(cursor, curHeader_, statusCode);
RETURN_IF_ERROR(err);
RETURN_IF_ERROR_WITH_CTX(err, "parse-rst-stream-frame");
if (statusCode == ErrorCode::PROTOCOL_ERROR) {
VLOG(3) << "RST_STREAM with code=" << getErrorCodeString(statusCode)
<< " for streamID=" << curHeader_.stream
@@ -734,7 +750,7 @@ ErrorCode HTTP2Codec::parseSettings(Cursor& cursor) {
<< " length=" << curHeader_.length;
std::deque<SettingPair> settings;
auto err = http2::parseSettings(cursor, curHeader_, settings);
RETURN_IF_ERROR(err);
RETURN_IF_ERROR_WITH_CTX(err, "parse-settings-frame");
if (curHeader_.flags & http2::ACK) {
handleSettingsAck();
return ErrorCode::NO_ERROR;
@@ -779,48 +795,40 @@ ErrorCode HTTP2Codec::handleSettings(const std::deque<SettingPair>& settings) {
case SettingsId::ENABLE_PUSH:
if ((setting.second != 0 && setting.second != 1) ||
(setting.second == 1 && isUpstream(transportDirection_))) {
goawayErrorMessage_ =
folly::to<string>("GOAWAY error: ENABLE_PUSH invalid setting=",
setting.second,
" for streamID=",
curHeader_.stream);
VLOG(4) << goawayErrorMessage_;
setParseErrorContext(
"settings-invalid-enable-push",
folly::to<string>(
"value=", setting.second, " stream=", curHeader_.stream));
return ErrorCode::PROTOCOL_ERROR;
}
break;
case SettingsId::INITIAL_WINDOW_SIZE:
if (setting.second == 0 ||
setting.second > http2::kMaxWindowUpdateSize) {
goawayErrorMessage_ = folly::to<string>(
"GOAWAY error: INITIAL_WINDOW_SIZE invalid size=",
setting.second,
" for streamID=",
curHeader_.stream);
VLOG(4) << goawayErrorMessage_;
setParseErrorContext(
"settings-invalid-initial-window-size",
folly::to<string>(
"value=", setting.second, " stream=", curHeader_.stream));
return ErrorCode::PROTOCOL_ERROR;
}
break;
case SettingsId::MAX_FRAME_SIZE:
if (setting.second < http2::kMaxFramePayloadLengthMin ||
setting.second > http2::kMaxFramePayloadLength) {
goawayErrorMessage_ =
folly::to<string>("GOAWAY error: MAX_FRAME_SIZE invalid size=",
setting.second,
" for streamID=",
curHeader_.stream);
VLOG(4) << goawayErrorMessage_;
setParseErrorContext(
"settings-invalid-max-frame-size",
folly::to<string>(
"value=", setting.second, " stream=", curHeader_.stream));
return ErrorCode::PROTOCOL_ERROR;
}
ingressSettings_.setSetting(SettingsId::MAX_FRAME_SIZE, setting.second);
break;
case SettingsId::ENABLE_CONNECT_PROTOCOL:
if (setting.second > 1) {
goawayErrorMessage_ = folly::to<string>(
"GOAWAY error: ENABLE_CONNECT_PROTOCOL invalid number=",
setting.second,
" for streamID=",
curHeader_.stream);
VLOG(4) << goawayErrorMessage_;
setParseErrorContext(
"settings-invalid-enable-connect-protocol",
folly::to<string>(
"value=", setting.second, " stream=", curHeader_.stream));
return ErrorCode::PROTOCOL_ERROR;
}
break;
@@ -855,15 +863,11 @@ ErrorCode HTTP2Codec::parsePushPromise(Cursor& cursor) {
created before the RST_STREAM frame is received and processed.
*/
if (isDownstream(transportDirection_)) {
goawayErrorMessage_ =
folly::to<string>("Received PUSH_PROMISE on DOWNSTREAM codec");
VLOG(2) << goawayErrorMessage_;
setParseErrorContext("received-push-promise-on-downstream-codec");
return ErrorCode::PROTOCOL_ERROR;
}
if (egressSettings_.getSetting(SettingsId::ENABLE_PUSH, -1) != 1) {
goawayErrorMessage_ =
folly::to<string>("Received PUSH_PROMISE on codec with push disabled");
VLOG(2) << goawayErrorMessage_;
setParseErrorContext("received-push-promise-with-push-disabled");
return ErrorCode::PROTOCOL_ERROR;
}
VLOG(4) << "parsing PUSH_PROMISE frame for stream=" << curHeader_.stream
@@ -872,8 +876,10 @@ ErrorCode HTTP2Codec::parsePushPromise(Cursor& cursor) {
std::unique_ptr<IOBuf> headerBlockFragment;
auto err = http2::parsePushPromise(
cursor, curHeader_, promisedStream, headerBlockFragment);
RETURN_IF_ERROR(err);
RETURN_IF_ERROR(checkNewStream(promisedStream, false /* trailersAllowed */));
RETURN_IF_ERROR_WITH_CTX(err, "parse-push-promise-frame");
auto newStreamErr =
checkNewStream(promisedStream, false /* trailersAllowed */);
RETURN_IF_ERROR(newStreamErr);
err =
parseHeadersImpl(cursor, std::move(headerBlockFragment), promisedStream);
return err;
@@ -883,7 +889,7 @@ ErrorCode HTTP2Codec::parsePing(Cursor& cursor) {
VLOG(4) << "parsing PING frame length=" << curHeader_.length;
uint64_t opaqueData = 0;
auto err = http2::parsePing(cursor, curHeader_, opaqueData);
RETURN_IF_ERROR(err);
RETURN_IF_ERROR_WITH_CTX(err, "parse-ping-frame");
if (callback_) {
if (curHeader_.flags & http2::ACK) {
callback_->onPingReply(opaqueData);
@@ -908,7 +914,7 @@ ErrorCode HTTP2Codec::parseGoaway(Cursor& cursor) {
<< " debugData="
<< ((debugData) ? debugData->to<std::string>() : empty_string);
}
RETURN_IF_ERROR(err);
RETURN_IF_ERROR_WITH_CTX(err, "parse-goaway-frame");
if (lastGoodStream < ingressGoawayAck_) {
ingressGoawayAck_ = lastGoodStream;
// Drain all streams <= lastGoodStream
@@ -927,22 +933,22 @@ ErrorCode HTTP2Codec::parseWindowUpdate(Cursor& cursor) {
<< " length=" << curHeader_.length;
uint32_t delta = 0;
auto err = http2::parseWindowUpdate(cursor, curHeader_, delta);
RETURN_IF_ERROR(err);
RETURN_IF_ERROR_WITH_CTX(err, "parse-window-update-frame");
if (delta == 0) {
VLOG(4) << "Invalid 0 length delta for stream=" << curHeader_.stream;
if (curHeader_.stream == 0) {
goawayErrorMessage_ = folly::to<string>(
"GOAWAY error: invalid/0 length delta for streamID=",
curHeader_.stream);
setParseErrorContext("window-update-zero-delta-connection", "stream=0");
return ErrorCode::PROTOCOL_ERROR;
} else {
// Parsing a zero delta window update should cause a protocol error
// and send a rst stream
goawayErrorMessage_ = folly::to<std::string>(
"streamID=", curHeader_.stream, " with window update delta=", delta);
VLOG(4) << goawayErrorMessage_;
streamError(goawayErrorMessage_, ErrorCode::PROTOCOL_ERROR);
auto additionalInfo = folly::to<string>("stream=", curHeader_.stream);
streamError(
folly::to<string>("window-update-zero-delta-stream ", additionalInfo),
ErrorCode::PROTOCOL_ERROR);
// Stream error and protocol error
setParseErrorContext("window-update-zero-delta-stream",
std::move(additionalInfo));
return ErrorCode::PROTOCOL_ERROR;
}
}
@@ -957,13 +963,14 @@ ErrorCode HTTP2Codec::parseWindowUpdate(Cursor& cursor) {
ErrorCode HTTP2Codec::checkNewStream(uint32_t streamId, bool trailersAllowed) {
bool existingStream = (streamId <= lastStreamID_);
if (streamId == 0 || (!trailersAllowed && existingStream)) {
goawayErrorMessage_ =
folly::to<string>("GOAWAY error: received streamID=",
streamId,
" as invalid new stream for lastStreamID_=",
lastStreamID_);
VLOG(4) << goawayErrorMessage_;
if (streamId == 0) {
setParseErrorContext("new-stream-zero-id");
return ErrorCode::PROTOCOL_ERROR;
}
if (!trailersAllowed && existingStream) {
setParseErrorContext(
"new-stream-not-monotonic",
folly::to<string>("stream=", streamId, " last=", lastStreamID_));
return ErrorCode::PROTOCOL_ERROR;
}
parsingDownstreamTrailers_ = trailersAllowed && existingStream;
@@ -977,9 +984,8 @@ ErrorCode HTTP2Codec::checkNewStream(uint32_t streamId, bool trailersAllowed) {
if (isInitiatedStream(streamId)) {
// this stream should be initiated by us, not by peer
goawayErrorMessage_ = folly::to<string>(
"GOAWAY error: invalid new stream received with streamID=", streamId);
VLOG(4) << goawayErrorMessage_;
setParseErrorContext("new-stream-wrong-initiator",
folly::to<string>("stream=", streamId));
return ErrorCode::PROTOCOL_ERROR;
} else {
return ErrorCode::NO_ERROR;
@@ -1537,14 +1543,22 @@ size_t HTTP2Codec::generatePriority(folly::IOBufQueue& writeBuf,
bool HTTP2Codec::checkConnectionError(ErrorCode err, const folly::IOBuf* buf) {
if (err != ErrorCode::NO_ERROR) {
std::string errorDescription =
goawayErrorMessage_.empty() ? "Connection error" : goawayErrorMessage_;
std::string context;
if (!parseErrorContext_.empty()) {
context = folly::to<std::string>("[Context]=", parseErrorContext_);
if (!parseErrorAdditionalInfo_.empty()) {
context =
folly::to<std::string>(context, " ", parseErrorAdditionalInfo_);
}
}
LOG(ERROR) << "Connection error " << getErrorCodeString(err) << " "
<< errorDescription << " with ingress=";
<< context << (context.empty() ? "" : " ") << "with ingress=";
VLOG(3) << IOBufPrinter::printHexFolly(buf, true);
if (callback_) {
HTTPException ex(HTTPException::Direction::INGRESS_AND_EGRESS,
errorDescription);
context.empty() ? std::string("Connection error")
: folly::to<std::string>(
"Connection error ", context));
ex.setCodecStatusCode(err);
callback_->onError(0, ex, false);
}
@@ -1553,6 +1567,15 @@ bool HTTP2Codec::checkConnectionError(ErrorCode err, const folly::IOBuf* buf) {
return false;
}
void HTTP2Codec::setParseErrorContext(std::string context,
std::string additionalInfo) {
parseErrorContext_ = std::move(context);
parseErrorAdditionalInfo_ = std::move(additionalInfo);
VLOG(4) << "parse error context=" << parseErrorContext_
<< (parseErrorAdditionalInfo_.empty() ? "" : " ")
<< parseErrorAdditionalInfo_;
}
void HTTP2Codec::streamError(const std::string& msg,
ErrorCode code,
bool newTxn,

4
proxygen/lib/http/codec/HTTP2Codec.h
View File

@@ -251,6 +251,8 @@ class HTTP2Codec
ErrorCode handleEndStream();
ErrorCode checkNewStream(uint32_t stream, bool trailersAllowed);
bool checkConnectionError(ErrorCode, const folly::IOBuf* buf);
void setParseErrorContext(std::string context,
std::string additionalInfo = {});
ErrorCode handleSettings(const std::deque<SettingPair>& settings);
void handleSettingsAck();
size_t maxSendFrameSize() const {
@@ -273,6 +275,8 @@ class HTTP2Codec
// Current frame state
http2::FrameHeader curHeader_;
std::string parseErrorContext_;
std::string parseErrorAdditionalInfo_;
StreamID expectedContinuationStream_{0};
// Used for parsing PUSH_PROMISE+CONTINUATION
folly::Optional<StreamID> promisedStream_;

1
proxygen/lib/http/codec/HTTPParallelCodec.h
View File

@@ -99,7 +99,6 @@ class HTTPParallelCodec : public HTTPCodec {
HTTPCodec::Callback* callback_{nullptr};
StreamID ingressGoawayAck_{std::numeric_limits<uint32_t>::max()};
StreamID egressGoawayAck_{std::numeric_limits<uint32_t>::max()};
std::string goawayErrorMessage_;
enum ClosingState {
OPEN = 0,

63
proxygen/lib/http/codec/HeaderDecodeInfo.cpp
View File

@@ -31,8 +31,11 @@ bool HeaderDecodeInfo::onHeader(const HPACKHeaderName& name,
if (nameSp.startsWith(':')) {
pseudoHeaderSeen_ = true;
if (firstPseudoHeader_.empty()) {
firstPseudoHeader_ = nameSp.str();
}
if (regularHeaderSeen_) {
parsingError = folly::to<string>("Illegal pseudo header name=", nameSp);
parsingError = folly::to<string>("illegal-pseudo-header name=", nameSp);
return false;
}
if (isRequest_) {
@@ -69,7 +72,8 @@ bool HeaderDecodeInfo::onHeader(const HPACKHeaderName& name,
}
break;
default:
parsingError = folly::to<string>("Invalid req header name=", nameSp);
parsingError =
folly::to<string>("invalid-request-pseudo-header name=", nameSp);
return false;
}
if (!ok) {
@@ -78,7 +82,10 @@ bool HeaderDecodeInfo::onHeader(const HPACKHeaderName& name,
} else {
if (headerCode == HTTP_HEADER_COLON_STATUS) {
if (hasStatus_) {
parsingError = string("Duplicate status");
parsingError = folly::to<string>("duplicate-status existing=",
msg->getStatusCode(),
" new=",
valueSp);
return false;
}
hasStatus_ = true;
@@ -89,11 +96,12 @@ bool HeaderDecodeInfo::onHeader(const HPACKHeaderName& name,
msg->setStatusCode(code);
msg->setStatusMessage(HTTPMessage::getDefaultReason(code));
} else {
parsingError = folly::to<string>("Malformed status code=", valueSp);
parsingError = folly::to<string>("malformed-status code=", valueSp);
return false;
}
} else {
parsingError = folly::to<string>("Invalid resp header name=", nameSp);
parsingError =
folly::to<string>("invalid-response-pseudo-header name=", nameSp);
return false;
}
}
@@ -101,14 +109,16 @@ bool HeaderDecodeInfo::onHeader(const HPACKHeaderName& name,
regularHeaderSeen_ = true;
switch (headerCode) {
case HTTP_HEADER_CONNECTION:
parsingError = string("HTTP/2 Message with Connection header");
parsingError =
folly::to<string>("connection-header-forbidden value=", valueSp);
return false;
case HTTP_HEADER_CONTENT_LENGTH: {
const auto cl = headers.getSingleOrNullptr(HTTP_HEADER_CONTENT_LENGTH);
if (cl) {
bool ok = *cl == valueSp;
if (!ok) {
parsingError = string("Multiple content-length headers");
parsingError = folly::to<string>(
"multiple-content-length existing=", *cl, " new=", valueSp);
}
return ok; // skips adding if already present and equal
}
@@ -118,7 +128,11 @@ bool HeaderDecodeInfo::onHeader(const HPACKHeaderName& name,
if (verifier.hasAuthority()) { // HTTP_HEADER_HOST already present
bool ok = headers.getSingleOrEmpty(HTTP_HEADER_HOST) == valueSp;
if (!ok) {
parsingError = ":authority/Host header mismatch";
parsingError =
folly::to<string>("authority-host-mismatch existing=",
headers.getSingleOrEmpty(HTTP_HEADER_HOST),
" new=",
valueSp);
}
return ok; // skips adding if already present and equal
}
@@ -133,16 +147,30 @@ bool HeaderDecodeInfo::onHeader(const HPACKHeaderName& name,
nameSp,
strictValidation_ ? CodecUtil::HEADER_NAME_STRICT
: CodecUtil::HEADER_NAME_STRICT_COMPAT);
bool valueOk =
!validate_ ||
CodecUtil::validateHeaderValue(
valueSp,
strictValidation_ ? CodecUtil::CtlEscapeMode::STRICT
: CodecUtil::CtlEscapeMode::STRICT_COMPAT);
if (!nameOk || !valueOk) {
auto valueError =
validate_
? CodecUtil::validateHeaderValueDetail(
valueSp,
strictValidation_ ? CodecUtil::CtlEscapeMode::STRICT
: CodecUtil::CtlEscapeMode::STRICT_COMPAT)
: CodecUtil::HeaderValueError::None;
if (!nameOk || valueError != CodecUtil::HeaderValueError::None) {
proxygenError =
nameSp.empty() ? kErrorParseHeader : kErrorHeaderContentValidation;
parsingError = folly::to<string>("Invalid header name=", nameSp);
std::string reason =
nameSp.empty() ? "empty-name" : (!nameOk ? "invalid-name-char" : "");
if (valueError != CodecUtil::HeaderValueError::None) {
if (!reason.empty()) {
reason += ",";
}
reason += CodecUtil::describeHeaderValueError(valueError);
}
parsingError = folly::to<string>(isRequestTrailers_ ? "invalid-trailer "
: "invalid-header ",
"name=",
nameSp,
" reason=",
reason);
headerErrorValue = valueSp;
return false;
}
@@ -170,7 +198,8 @@ void HeaderDecodeInfo::onHeadersComplete(HTTPHeaderSize decodedSize) {
bool isResponseTrailers = (!isRequest_ && !hasStatus_);
if ((isRequestTrailers_ || isResponseTrailers) && pseudoHeaderSeen_) {
parsingError = "Pseudo headers forbidden in trailers.";
parsingError = folly::to<string>("pseudo-header-in-trailers name=",
firstPseudoHeader_);
return;
}

2
proxygen/lib/http/codec/HeaderDecodeInfo.h
View File

@@ -36,6 +36,7 @@ class HeaderDecodeInfo {
pseudoHeaderSeen_ = false;
parsingError.clear();
headerErrorValue.clear();
firstPseudoHeader_.clear();
decodeError = HPACK::DecodeError::NONE;
strictValidation_ = strictValidation;
allowEmptyPath_ = allowEmptyPath;
@@ -67,6 +68,7 @@ class HeaderDecodeInfo {
// Default to false for now to match existing behavior
bool strictValidation_{false};
bool allowEmptyPath_{false};
std::string firstPseudoHeader_;
};
} // namespace proxygen

23
proxygen/lib/http/codec/test/HTTP2CodecTest.cpp
View File

@@ -176,6 +176,27 @@ TEST_F(HTTP2CodecTestOmitParsePreface, OmitSettingsAfterConnPrefaceError) {
EXPECT_EQ(callbacks_.sessionErrors, 1);
EXPECT_EQ(callbacks_.lastParseError->getCodecStatusCode(),
ErrorCode::PROTOCOL_ERROR);
EXPECT_THAT(callbacks_.lastParseError->what(),
HasSubstr("[Context]=invalid-connection-preface-frame-type"));
// The numeric type is what identifies a frame the name mapping doesn't know.
EXPECT_THAT(callbacks_.lastParseError->what(), HasSubstr("type=HEADERS(1)"));
}
TEST_F(HTTP2CodecTestOmitParsePreface, OversizedNonSettingsPrefaceFrame) {
// The preface check and the frame-size check are independent ifs, so a first
// frame that is both non-SETTINGS and oversized trips both. The reported
// message and context must describe the same failure.
writeFrameHeaderManual(
output_, 1 << 15, (uint8_t)http2::FrameType::HEADERS, 0, 1);
parse();
EXPECT_EQ(callbacks_.sessionErrors, 1);
EXPECT_EQ(callbacks_.lastParseError->getCodecStatusCode(),
ErrorCode::FRAME_SIZE_ERROR);
EXPECT_THAT(callbacks_.lastParseError->what(),
HasSubstr("[Context]=frame-exceeds-max-size"));
EXPECT_THAT(callbacks_.lastParseError->what(),
Not(HasSubstr("invalid connection preface")));
}
TEST_F(HTTP2CodecTest, BadHeaders) {
@@ -954,6 +975,8 @@ TEST_F(HTTP2CodecTest, FrameTooLarge) {
EXPECT_TRUE(callbacks_.lastParseError->hasCodecStatusCode());
EXPECT_EQ(callbacks_.lastParseError->getCodecStatusCode(),
ErrorCode::FRAME_SIZE_ERROR);
EXPECT_THAT(callbacks_.lastParseError->what(),
HasSubstr("[Context]=frame-exceeds-max-size"));
}
TEST_F(HTTP2CodecTest, DataFrameZeroLengthWithEOM) {

3
proxygen/lib/http/session/test/HTTPDownstreamSessionTest.cpp
View File

@@ -3033,7 +3033,8 @@ TEST_F(HTTP2DownstreamSessionTest, ZeroDeltaWindowUpdate) {
handler->expectHeaders();
handler->expectError([&](const HTTPException& ex) {
ASSERT_EQ(ex.getCodecStatusCode(), ErrorCode::PROTOCOL_ERROR);
ASSERT_EQ("streamID=1 with window update delta=0", std::string(ex.what()));
ASSERT_EQ("window-update-zero-delta-stream stream=1",
std::string(ex.what()));
});
handler->expectDetachTransaction();
expectDetachSession();