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

HQ codecs: log parse-error context

Browse Source 
Summary:
Same attribution for HTTP/3, and the largest gap of the three.
`HQFramedCodec::checkConnectionError` built every frame-layer `HTTPException`
from the hardcoded literal `Connection error`, so `HQControlCodec`'s five
indistinguishable `HTTP_FRAME_UNEXPECTED` returns and `HQFramer`'s message-less
returns all collapsed into one string. Only the numeric HTTP/3 code
differentiated them.

  Connection error [File]=HQControlCodec.cpp
  [Context]=settings-not-first-frame

The frame loop lives in the base class while the checks live in the derived
codecs, so `setParseErrorContext` takes the caller's file and the origin points
at the real rejecting file rather than the base. The base clears the context
before `checkFrameAllowed` and falls back to a generic reason only when the
derived codec named none, so a specific attribution is never overwritten.

Instrumented `HQFramedCodec`, `HQControlCodec` and `HQStreamCodec`.

Reviewed By: afrind

Differential Revision: D114943895

fbshipit-source-id: 47123a6e08d1ecca411f505f500f3bee3f285680
This commit is contained in:
Sujay Patel
2026-09-21 11:57:47 -07:00
committed by meta-codesync[bot]
parent e1aae3eb6e
commit 8ec08dceb0
7 changed files with 69 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files

18
proxygen/lib/http/codec/HQControlCodec.cpp
View File

@@ -6,6 +6,7 @@
* LICENSE file in the root directory of this source tree.
*/
#include <proxygen/lib/http/codec/CodecUtil.h>
#include <proxygen/lib/http/codec/HQControlCodec.h>
#include <proxygen/lib/http/HTTP3ErrorCode.h>
@@ -36,6 +37,9 @@ ParseResult HQControlCodec::checkFrameAllowed(FrameType type) {
case hq::FrameType::HEADERS:
case hq::FrameType::PUSH_PROMISE:
case hq::FrameType::WEBTRANSPORT_BIDI:
setParseErrorContext(
"data-frame-on-control-stream",
folly::to<std::string>("type=", getFrameTypeString(type)));
return HTTP3::ErrorCode::HTTP_FRAME_UNEXPECTED;
default:
break;
@@ -44,16 +48,21 @@ ParseResult HQControlCodec::checkFrameAllowed(FrameType type) {
if (getStreamType() == hq::UnidirectionalStreamType::CONTROL) {
// SETTINGS MUST be the first frame on an HQ Control Stream
if (!receivedSettings_ && type != hq::FrameType::SETTINGS) {
setParseErrorContext(
"settings-not-first-frame",
folly::to<std::string>("type=", getFrameTypeString(type)));
return HTTP3::ErrorCode::HTTP_MISSING_SETTINGS;
}
// multiple SETTINGS frames are not allowed
if (receivedSettings_ && type == hq::FrameType::SETTINGS) {
setParseErrorContext("duplicate-settings");
return HTTP3::ErrorCode::HTTP_FRAME_UNEXPECTED;
}
// A client MUST treat the receipt of a MAX_PUSH_ID frame as a connection
// error of type HTTP_FRAME_UNEXPECTED
if (transportDirection_ == TransportDirection::UPSTREAM &&
type == hq::FrameType::MAX_PUSH_ID) {
setParseErrorContext("max-push-id-on-upstream-control-stream");
return HTTP3::ErrorCode::HTTP_FRAME_UNEXPECTED;
}
@@ -63,6 +72,9 @@ ParseResult HQControlCodec::checkFrameAllowed(FrameType type) {
type == hq::FrameType::PUSH_PRIORITY_UPDATE ||
type == hq::FrameType::FB_PUSH_PRIORITY_UPDATE ||
type == hq::FrameType::FB_PRIORITY_UPDATE)) {
setParseErrorContext(
"priority-update-on-upstream-control-stream",
folly::to<std::string>("type=", getFrameTypeString(type)));
return HTTP3::ErrorCode::HTTP_FRAME_UNEXPECTED;
}
}
@@ -107,6 +119,12 @@ ParseResult HQControlCodec::parseSettings(Cursor& cursor,
case hq::SettingId::WT_ENABLED:
// only 0/1 are legal
if (setting.second > 1) {
setParseErrorContext(
"boolean-setting-out-of-range",
folly::to<std::string>("id=",
static_cast<uint64_t>(setting.first),
" value=",
setting.second));
return HTTP3::ErrorCode::HTTP_SETTINGS_ERROR;
}
break;

25
proxygen/lib/http/codec/HQFramedCodec.cpp
View File

@@ -76,11 +76,20 @@ size_t HQFramedCodec::onFramedIngress(const IOBuf& buf) {
}
curHeader_.type = FrameType(type->first);
parsed += type->second;
parseErrorContext_ = {};
parseErrorAdditionalInfo_ = {};
auto res = checkFrameAllowed(curHeader_.type);
if (res) {
VLOG(4) << "Frame not allowed: 0x" << std::setfill('0')
<< std::setw(sizeof(uint64_t) * 2) << std::hex
<< (uint64_t)curHeader_.type << " on streamID=" << streamId_;
// Only generic if the derived codec did not name a specific reason.
if (parseErrorContext_.empty()) {
setParseErrorContext(
"frame-not-allowed",
folly::to<std::string>("type=",
getFrameTypeString(curHeader_.type)));
}
connError_ = res;
break;
}
@@ -157,6 +166,9 @@ bool HQFramedCodec::onFramedIngressEOF() {
return false;
} else if (frameState_ != FrameState::FRAME_HEADER_TYPE) {
VLOG(3) << "Stream ended in the middle of a frame type=" << curHeader_.type;
setParseErrorContext(
"eof-in-middle-of-frame",
folly::to<std::string>("type=", getFrameTypeString(curHeader_.type)));
connError_ = HTTP3::ErrorCode::HTTP_FRAME_ERROR;
checkConnectionError(connError_, nullptr);
return false;
@@ -168,14 +180,23 @@ bool HQFramedCodec::onFramedIngressEOF() {
bool HQFramedCodec::checkConnectionError(ParseResult err,
const folly::IOBuf* buf) {
if (err != folly::none) {
LOG(ERROR) << "Connection error with ingress=";
std::string errorMessage("Connection error");
if (!parseErrorContext_.empty()) {
errorMessage = folly::to<std::string>(
errorMessage, " [Context]=", parseErrorContext_);
if (!parseErrorAdditionalInfo_.empty()) {
errorMessage = folly::to<std::string>(
errorMessage, " ", parseErrorAdditionalInfo_);
}
}
LOG(ERROR) << errorMessage << " with ingress=";
if (buf) {
VLOG(3) << IOBufPrinter::printHexFolly(buf, true);
}
setParserPaused(true);
if (callback_) {
HTTPException ex(HTTPException::Direction::INGRESS_AND_EGRESS,
"Connection error");
errorMessage);
ex.setHttp3ErrorCode(err.value());
callback_->onError(kSessionStreamId, ex, false);
}

16
proxygen/lib/http/codec/HQFramedCodec.h
View File

@@ -264,6 +264,20 @@ class HQFramedCodec : public HTTPCodec {
}
protected:
/**
* Record why framing was rejected. HQ frame errors reach
* checkConnectionError as a bare HTTP3::ErrorCode, so without this every
* failure in the frame layer reports the same "Connection error" text.
*/
void setParseErrorContext(std::string context,
std::string additionalInfo = {}) {
parseErrorContext_ = std::move(context);
parseErrorAdditionalInfo_ = std::move(additionalInfo);
VLOG(4) << "parse error context=" << parseErrorContext_
<< (parseErrorAdditionalInfo_.empty() ? "" : " ")
<< parseErrorAdditionalInfo_;
}
virtual ParseResult checkFrameAllowed(FrameType type) = 0;
virtual ParseResult parseData(folly::io::Cursor& /*cursor*/,
@@ -352,6 +366,8 @@ class HQFramedCodec : public HTTPCodec {
};
FrameState frameState_ : 3;
ParseResult connError_{folly::none};
std::string parseErrorContext_;
std::string parseErrorAdditionalInfo_;
uint64_t totalBytesParsed_{0};
folly::Function<void()> resumeHook_;
};

2
proxygen/lib/http/codec/HQFramer.cpp
View File

@@ -387,7 +387,7 @@ WriteResult writeStreamPreface(folly::IOBufQueue& writeBuf,
return *streamPrefaceSize;
}
static const char* getFrameTypeString(FrameType type) {
const char* getFrameTypeString(FrameType type) {
switch (type) {
case FrameType::DATA:
return "DATA";

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

@@ -86,6 +86,8 @@ enum class FrameType : uint64_t {
};
const char* getFrameTypeString(FrameType type);
std::ostream& operator<<(std::ostream& os, FrameType type);
struct FrameHeader {

7
proxygen/lib/http/codec/HQStreamCodec.cpp
View File

@@ -42,6 +42,9 @@ HQStreamCodec::~HQStreamCodec() = default;
ParseResult HQStreamCodec::checkFrameAllowed(FrameType type) {
if (isConnect_ && type != hq::FrameType::DATA) {
setParseErrorContext(
"non-data-frame-on-connect",
folly::to<std::string>("type=", getFrameTypeString(type)));
return HTTP3::ErrorCode::HTTP_FRAME_UNEXPECTED;
}
switch (type) {
@@ -54,9 +57,13 @@ ParseResult HQStreamCodec::checkFrameAllowed(FrameType type) {
case hq::FrameType::FB_PRIORITY_UPDATE:
case hq::FrameType::FB_PUSH_PRIORITY_UPDATE:
case hq::FrameType::WEBTRANSPORT_BIDI:
setParseErrorContext(
"control-frame-on-request-stream",
folly::to<std::string>("type=", getFrameTypeString(type)));
return HTTP3::ErrorCode::HTTP_FRAME_UNEXPECTED;
case hq::FrameType::PUSH_PROMISE:
if (transportDirection_ == TransportDirection::DOWNSTREAM) {
setParseErrorContext("received-push-promise-on-downstream-codec");
return HTTP3::ErrorCode::HTTP_FRAME_UNEXPECTED;
}
break;

2
proxygen/lib/http/codec/test/HQCodecTest.cpp
View File

@@ -1143,6 +1143,8 @@ TEST_P(HQCodecTestFrameBeforeSettings, FrameAllowedOnControlCodec) {
} else {
EXPECT_EQ(callbacks_.lastParseError->getHttp3ErrorCode(),
HTTP3::ErrorCode::HTTP_MISSING_SETTINGS);
EXPECT_THAT(callbacks_.lastParseError->what(),
HasSubstr("[Context]=settings-not-first-frame"));
}
}
}