[hpack] Dynamically allocate error for hpack parser (#34889)

And cache-align InterSliceState.
pull/34901/head
Alisha Nanda 1 year ago committed by GitHub
parent d3828ebfbd
commit a9291f59fb
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 2
      BUILD
  2. 64
      src/core/ext/transport/chttp2/transport/hpack_parse_result.cc
  3. 97
      src/core/ext/transport/chttp2/transport/hpack_parse_result.h
  4. 8
      src/core/ext/transport/chttp2/transport/hpack_parser.h

@ -3861,8 +3861,10 @@ grpc_cc_library(
deps = [
"gpr",
"grpc_base",
"ref_counted_ptr",
"//src/core:error",
"//src/core:hpack_constants",
"//src/core:ref_counted",
"//src/core:slice",
"//src/core:status_helper",
],

@ -59,13 +59,21 @@ absl::Status MakeStreamError(absl::Status error) {
} // namespace
absl::Status HpackParseResult::Materialize() const {
if (materialized_status_.has_value()) return *materialized_status_;
materialized_status_ = BuildMaterialized();
return *materialized_status_;
if (state_ != nullptr && state_->materialized_status.has_value()) {
return *state_->materialized_status;
}
absl::Status materialized_status = BuildMaterialized();
if (!materialized_status.ok()) {
// We can safely assume state_ is not null here, since BuildMaterialized
// returns ok if it is.
state_->materialized_status = materialized_status;
}
return materialized_status;
}
absl::Status HpackParseResult::BuildMaterialized() const {
switch (status_.get()) {
if (state_ == nullptr) return absl::OkStatus();
switch (state_->status.get()) {
case HpackParseStatus::kOk:
return absl::OkStatus();
case HpackParseStatus::kEof:
@ -75,17 +83,17 @@ absl::Status HpackParseResult::BuildMaterialized() const {
Crash("Materialize() called on moved-from object");
break;
case HpackParseStatus::kInvalidMetadata:
if (key_.empty()) {
if (state_->key.empty()) {
return MakeStreamError(absl::InternalError(
ValidateMetadataResultToString(validate_metadata_result_)));
ValidateMetadataResultToString(state_->validate_metadata_result)));
} else {
return MakeStreamError(absl::InternalError(absl::StrCat(
ValidateMetadataResultToString(validate_metadata_result_), ": ",
key_)));
ValidateMetadataResultToString(state_->validate_metadata_result),
": ", state_->key)));
}
case HpackParseStatus::kSoftMetadataLimitExceeded:
case HpackParseStatus::kHardMetadataLimitExceeded: {
const auto& e = metadata_limit_exceeded_;
const auto& e = state_->metadata_limit_exceeded;
// Collect a summary of sizes so far for debugging
// Do not collect contents, for fear of exposing PII.
std::string summary;
@ -95,35 +103,36 @@ absl::Status HpackParseResult::BuildMaterialized() const {
}
return MakeStreamError(absl::ResourceExhaustedError(absl::StrCat(
"received metadata size exceeds ",
status_.get() == HpackParseStatus::kSoftMetadataLimitExceeded
state_->status.get() == HpackParseStatus::kSoftMetadataLimitExceeded
? "soft"
: "hard",
" limit (", e.frame_length, " vs. ", e.limit, ")",
summary.empty() ? "" : "; ", summary)));
}
case HpackParseStatus::kHardMetadataLimitExceededByKey: {
const auto& e = metadata_limit_exceeded_by_atom_;
const auto& e = state_->metadata_limit_exceeded_by_atom;
return MakeStreamError(absl::ResourceExhaustedError(
absl::StrCat("received metadata size exceeds hard limit (key length ",
e.atom_length, " vs. ", e.limit, ")")));
}
case HpackParseStatus::kHardMetadataLimitExceededByValue: {
const auto& e = metadata_limit_exceeded_by_atom_;
const auto& e = state_->metadata_limit_exceeded_by_atom;
return MakeStreamError(absl::ResourceExhaustedError(absl::StrCat(
"received metadata size exceeds hard limit (value length ",
e.atom_length, " vs. ", e.limit, ")")));
}
case HpackParseStatus::kMetadataParseError:
if (!key_.empty()) {
if (!state_->key.empty()) {
return MakeStreamError(absl::InternalError(
absl::StrCat("Error parsing '", key_, "' metadata")));
absl::StrCat("Error parsing '", state_->key, "' metadata")));
} else {
return MakeStreamError(absl::InternalError("Error parsing metadata"));
}
case HpackParseStatus::kUnbase64Failed:
if (!key_.empty()) {
return MakeStreamError(absl::InternalError(absl::StrCat(
"Error parsing '", key_, "' metadata: illegal base64 encoding")));
if (!state_->key.empty()) {
return MakeStreamError(absl::InternalError(
absl::StrCat("Error parsing '", state_->key,
"' metadata: illegal base64 encoding")));
} else {
return MakeStreamError(absl::InternalError(
absl::StrCat("Failed base64 decoding metadata")));
@ -135,22 +144,23 @@ absl::Status HpackParseResult::BuildMaterialized() const {
return absl::InternalError(absl::StrFormat(
"integer overflow in hpack integer decoding: have 0x%08x, "
"got byte 0x%02x",
varint_out_of_range_.value, varint_out_of_range_.last_byte));
state_->varint_out_of_range.value,
state_->varint_out_of_range.last_byte));
case HpackParseStatus::kIllegalTableSizeChange:
return absl::InternalError(absl::StrCat(
"Attempt to make hpack table ", illegal_table_size_change_.new_size,
" bytes when max is ", illegal_table_size_change_.max_size,
" bytes"));
"Attempt to make hpack table ",
state_->illegal_table_size_change.new_size, " bytes when max is ",
state_->illegal_table_size_change.max_size, " bytes"));
case HpackParseStatus::kAddBeforeTableSizeUpdated:
return absl::InternalError(
absl::StrCat("HPACK max table size reduced to ",
illegal_table_size_change_.new_size,
state_->illegal_table_size_change.new_size,
" but not reflected by hpack stream (still at ",
illegal_table_size_change_.max_size, ")"));
state_->illegal_table_size_change.max_size, ")"));
case HpackParseStatus::kParseHuffFailed:
if (!key_.empty()) {
return absl::InternalError(
absl::StrCat("Failed huffman decoding '", key_, "' metadata"));
if (!state_->key.empty()) {
return absl::InternalError(absl::StrCat("Failed huffman decoding '",
state_->key, "' metadata"));
} else {
return absl::InternalError(
absl::StrCat("Failed huffman decoding metadata"));
@ -164,7 +174,7 @@ absl::Status HpackParseResult::BuildMaterialized() const {
"Malicious varint encoding detected in HPACK stream");
case HpackParseStatus::kInvalidHpackIndex:
return absl::InternalError(absl::StrFormat(
"Invalid HPACK index received (%d)", invalid_hpack_index_));
"Invalid HPACK index received (%d)", state_->invalid_hpack_index));
case HpackParseStatus::kIllegalHpackOpCode:
return absl::InternalError("Illegal hpack op code");
}

@ -31,6 +31,8 @@
#include <grpc/support/log.h>
#include "src/core/lib/gprpp/crash.h"
#include "src/core/lib/gprpp/ref_counted.h"
#include "src/core/lib/gprpp/ref_counted_ptr.h"
#include "src/core/lib/surface/validate_metadata.h"
#include "src/core/lib/transport/metadata_batch.h"
@ -128,10 +130,18 @@ class HpackParseResult {
public:
HpackParseResult() : HpackParseResult{HpackParseStatus::kOk} {}
bool ok() const { return status_.get() == HpackParseStatus::kOk; }
bool stream_error() const { return IsStreamError(status_.get()); }
bool connection_error() const { return IsConnectionError(status_.get()); }
bool ephemeral() const { return IsEphemeralError(status_.get()); }
bool ok() const {
return state_ == nullptr || state_->status.get() == HpackParseStatus::kOk;
}
bool stream_error() const {
return state_ != nullptr && IsStreamError(state_->status.get());
}
bool connection_error() const {
return state_ != nullptr && IsConnectionError(state_->status.get());
}
bool ephemeral() const {
return state_ != nullptr && IsEphemeralError(state_->status.get());
}
std::unique_ptr<HpackParseResult> PersistentStreamErrorOrNullptr() const {
if (ok() || connection_error() || ephemeral()) return nullptr;
@ -154,20 +164,22 @@ class HpackParseResult {
static HpackParseResult FromStatusWithKey(HpackParseStatus status,
absl::string_view key) {
auto r = FromStatus(status);
r.key_ = std::string(key);
if (r.state_ != nullptr) {
r.state_->key = std::string(key);
}
return r;
}
static HpackParseResult MetadataParseError(absl::string_view key) {
HpackParseResult r{HpackParseStatus::kMetadataParseError};
r.key_ = std::string(key);
r.state_->key = std::string(key);
return r;
}
static HpackParseResult AddBeforeTableSizeUpdated(uint32_t current_size,
uint32_t max_size) {
HpackParseResult p{HpackParseStatus::kAddBeforeTableSizeUpdated};
p.illegal_table_size_change_ =
p.state_->illegal_table_size_change =
IllegalTableSizeChange{current_size, max_size};
return p;
}
@ -184,8 +196,8 @@ class HpackParseResult {
absl::string_view key) {
GPR_DEBUG_ASSERT(result != ValidateMetadataResult::kOk);
HpackParseResult p{HpackParseStatus::kInvalidMetadata};
p.key_ = std::string(key);
p.validate_metadata_result_ = result;
p.state_->key = std::string(key);
p.state_->validate_metadata_result = result;
return p;
}
@ -196,20 +208,21 @@ class HpackParseResult {
static HpackParseResult VarintOutOfRangeError(uint32_t value,
uint8_t last_byte) {
HpackParseResult p{HpackParseStatus::kVarintOutOfRange};
p.varint_out_of_range_ = VarintOutOfRange{last_byte, value};
p.state_->varint_out_of_range = VarintOutOfRange{last_byte, value};
return p;
}
static HpackParseResult InvalidHpackIndexError(uint32_t index) {
HpackParseResult p{HpackParseStatus::kInvalidHpackIndex};
p.invalid_hpack_index_ = index;
p.state_->invalid_hpack_index = index;
return p;
}
static HpackParseResult IllegalTableSizeChangeError(uint32_t new_size,
uint32_t max_size) {
HpackParseResult p{HpackParseStatus::kIllegalTableSizeChange};
p.illegal_table_size_change_ = IllegalTableSizeChange{new_size, max_size};
p.state_->illegal_table_size_change =
IllegalTableSizeChange{new_size, max_size};
return p;
}
@ -220,7 +233,7 @@ class HpackParseResult {
static HpackParseResult SoftMetadataLimitExceededError(
grpc_metadata_batch* metadata, uint32_t frame_length, uint32_t limit) {
HpackParseResult p{HpackParseStatus::kSoftMetadataLimitExceeded};
p.metadata_limit_exceeded_ =
p.state_->metadata_limit_exceeded =
MetadataLimitExceeded{frame_length, limit, metadata};
return p;
}
@ -228,7 +241,7 @@ class HpackParseResult {
static HpackParseResult HardMetadataLimitExceededError(
grpc_metadata_batch* metadata, uint32_t frame_length, uint32_t limit) {
HpackParseResult p{HpackParseStatus::kHardMetadataLimitExceeded};
p.metadata_limit_exceeded_ =
p.state_->metadata_limit_exceeded =
MetadataLimitExceeded{frame_length, limit, metadata};
return p;
}
@ -236,7 +249,7 @@ class HpackParseResult {
static HpackParseResult HardMetadataLimitExceededByKeyError(
uint32_t key_length, uint32_t limit) {
HpackParseResult p{HpackParseStatus::kHardMetadataLimitExceededByKey};
p.metadata_limit_exceeded_by_atom_ =
p.state_->metadata_limit_exceeded_by_atom =
MetadataLimitExceededByAtom{key_length, limit};
return p;
}
@ -244,9 +257,9 @@ class HpackParseResult {
static HpackParseResult HardMetadataLimitExceededByValueError(
absl::string_view key, uint32_t value_length, uint32_t limit) {
HpackParseResult p{HpackParseStatus::kHardMetadataLimitExceededByValue};
p.metadata_limit_exceeded_by_atom_ =
p.state_->metadata_limit_exceeded_by_atom =
MetadataLimitExceededByAtom{value_length, limit};
p.key_ = std::string(key);
p.state_->key = std::string(key);
return p;
}
@ -255,7 +268,13 @@ class HpackParseResult {
absl::Status Materialize() const;
private:
explicit HpackParseResult(HpackParseStatus status) : status_(status) {}
explicit HpackParseResult(HpackParseStatus status) {
// Dynamically allocate state if status is not ok.
if (status != HpackParseStatus::kOk) {
state_ = MakeRefCounted<HpackParseResultState>(status);
}
}
absl::Status BuildMaterialized() const;
struct VarintOutOfRange {
@ -300,25 +319,31 @@ class HpackParseResult {
HpackParseStatus status_;
};
StatusWrapper status_;
union {
// Set if status == kInvalidMetadata
ValidateMetadataResult validate_metadata_result_;
// Set if status == kVarintOutOfRange
VarintOutOfRange varint_out_of_range_;
// Set if status == kInvalidHpackIndex
uint32_t invalid_hpack_index_;
// Set if status == kHardMetadataLimitExceeded or
// kSoftMetadataLimitExceeded
MetadataLimitExceeded metadata_limit_exceeded_;
// Set if status == kHardMetadataLimitExceededByKey or
// kHardMetadataLimitExceededByValue
MetadataLimitExceededByAtom metadata_limit_exceeded_by_atom_;
// Set if status == kIllegalTableSizeChange
IllegalTableSizeChange illegal_table_size_change_;
struct HpackParseResultState : public RefCounted<HpackParseResultState> {
explicit HpackParseResultState(HpackParseStatus incoming_status)
: status(incoming_status) {}
StatusWrapper status;
union {
// Set if status == kInvalidMetadata
ValidateMetadataResult validate_metadata_result;
// Set if status == kVarintOutOfRange
VarintOutOfRange varint_out_of_range;
// Set if status == kInvalidHpackIndex
uint32_t invalid_hpack_index;
// Set if status == kHardMetadataLimitExceeded or
// kSoftMetadataLimitExceeded
MetadataLimitExceeded metadata_limit_exceeded;
// Set if status == kHardMetadataLimitExceededByKey or
// kHardMetadataLimitExceededByValue
MetadataLimitExceededByAtom metadata_limit_exceeded_by_atom;
// Set if status == kIllegalTableSizeChange
IllegalTableSizeChange illegal_table_size_change;
};
std::string key;
mutable absl::optional<absl::Status> materialized_status;
};
std::string key_;
mutable absl::optional<absl::Status> materialized_status_;
RefCountedPtr<HpackParseResultState> state_ = nullptr;
};
} // namespace grpc_core

@ -240,10 +240,6 @@ class HPackParser {
uint32_t frame_length = 0;
// Length of the string being parsed
uint32_t string_length;
// How many more dynamic table updates are allowed
uint8_t dynamic_table_updates_allowed;
// Current parse state
ParseState parse_state = ParseState::kTop;
// RED for overly large metadata sets
RandomEarlyDetection metadata_early_detection;
// Should the current header be added to the hpack table?
@ -252,6 +248,10 @@ class HPackParser {
bool is_string_huff_compressed;
// Is the value being parsed binary?
bool is_binary_header;
// How many more dynamic table updates are allowed
uint8_t dynamic_table_updates_allowed;
// Current parse state
ParseState parse_state = ParseState::kTop;
absl::variant<const HPackTable::Memento*, Slice> key;
};

Loading…
Cancel
Save