diff --git a/src/core/ext/transport/chttp2/transport/bin_encoder.cc b/src/core/ext/transport/chttp2/transport/bin_encoder.cc index 272907bf5aefc..033b4cb2252c9 100644 --- a/src/core/ext/transport/chttp2/transport/bin_encoder.cc +++ b/src/core/ext/transport/chttp2/transport/bin_encoder.cc @@ -149,7 +149,8 @@ static void enc_flush_some(huff_out* out) { } } -static void enc_add2(huff_out* out, uint8_t a, uint8_t b) { +static void enc_add2(huff_out* out, uint8_t a, uint8_t b, uint32_t* wire_size) { + *wire_size += 2; b64_huff_sym sa = huff_alphabet[a]; b64_huff_sym sb = huff_alphabet[b]; out->temp = (out->temp << (sa.length + sb.length)) | @@ -159,7 +160,8 @@ static void enc_add2(huff_out* out, uint8_t a, uint8_t b) { enc_flush_some(out); } -static void enc_add1(huff_out* out, uint8_t a) { +static void enc_add1(huff_out* out, uint8_t a, uint32_t* wire_size) { + *wire_size += 1; b64_huff_sym sa = huff_alphabet[a]; out->temp = (out->temp << sa.length) | sa.bits; out->temp_length += sa.length; @@ -167,7 +169,7 @@ static void enc_add1(huff_out* out, uint8_t a) { } grpc_slice grpc_chttp2_base64_encode_and_huffman_compress( - const grpc_slice& input) { + const grpc_slice& input, uint32_t* wire_size) { size_t input_length = GRPC_SLICE_LENGTH(input); size_t input_triplets = input_length / 3; size_t tail_case = input_length % 3; @@ -183,16 +185,17 @@ grpc_slice grpc_chttp2_base64_encode_and_huffman_compress( out.temp = 0; out.temp_length = 0; out.out = start_out; + *wire_size = 0; // encode full triplets for (i = 0; i < input_triplets; i++) { const uint8_t low_to_high = static_cast((in[0] & 0x3) << 4); const uint8_t high_to_low = in[1] >> 4; - enc_add2(&out, in[0] >> 2, low_to_high | high_to_low); + enc_add2(&out, in[0] >> 2, low_to_high | high_to_low, wire_size); const uint8_t a = static_cast((in[1] & 0xf) << 2); const uint8_t b = (in[2] >> 6); - enc_add2(&out, a | b, in[2] & 0x3f); + enc_add2(&out, a | b, in[2] & 0x3f, wire_size); in += 3; } @@ -201,14 +204,15 @@ grpc_slice grpc_chttp2_base64_encode_and_huffman_compress( case 0: break; case 1: - enc_add2(&out, in[0] >> 2, static_cast((in[0] & 0x3) << 4)); + enc_add2(&out, in[0] >> 2, static_cast((in[0] & 0x3) << 4), + wire_size); in += 1; break; case 2: { const uint8_t low_to_high = static_cast((in[0] & 0x3) << 4); const uint8_t high_to_low = in[1] >> 4; - enc_add2(&out, in[0] >> 2, low_to_high | high_to_low); - enc_add1(&out, static_cast((in[1] & 0xf) << 2)); + enc_add2(&out, in[0] >> 2, low_to_high | high_to_low, wire_size); + enc_add1(&out, static_cast((in[1] & 0xf) << 2), wire_size); in += 2; break; } diff --git a/src/core/ext/transport/chttp2/transport/bin_encoder.h b/src/core/ext/transport/chttp2/transport/bin_encoder.h index ec0ef0bb80f1b..1b43d01705550 100644 --- a/src/core/ext/transport/chttp2/transport/bin_encoder.h +++ b/src/core/ext/transport/chttp2/transport/bin_encoder.h @@ -21,6 +21,8 @@ #include +#include + #include // base64 encode a slice. Returns a new slice, does not take ownership of the @@ -36,7 +38,9 @@ grpc_slice grpc_chttp2_huffman_compress(const grpc_slice& input); // grpc_slice y = grpc_chttp2_huffman_compress(x); // grpc_core::CSliceUnref( x); // return y; +// *wire_size is the length of the base64 encoded string prior to huffman +// compression (as is needed for hpack table math) grpc_slice grpc_chttp2_base64_encode_and_huffman_compress( - const grpc_slice& input); + const grpc_slice& input, uint32_t* wire_size); #endif // GRPC_SRC_CORE_EXT_TRANSPORT_CHTTP2_TRANSPORT_BIN_ENCODER_H diff --git a/src/core/ext/transport/chttp2/transport/hpack_encoder.cc b/src/core/ext/transport/chttp2/transport/hpack_encoder.cc index 2fc4f8f3b31ea..dc99bdc2bfe78 100644 --- a/src/core/ext/transport/chttp2/transport/hpack_encoder.cc +++ b/src/core/ext/transport/chttp2/transport/hpack_encoder.cc @@ -131,21 +131,34 @@ struct WireValue { : data(std::move(slice)), huffman_prefix(huffman_prefix), insert_null_before_wire_value(insert_null_before_wire_value), - length(data.length() + (insert_null_before_wire_value ? 1 : 0)) {} + length(data.length() + (insert_null_before_wire_value ? 1 : 0)), + hpack_length(length) {} + WireValue(uint8_t huffman_prefix, bool insert_null_before_wire_value, + Slice slice, size_t hpack_length) + : data(std::move(slice)), + huffman_prefix(huffman_prefix), + insert_null_before_wire_value(insert_null_before_wire_value), + length(data.length() + (insert_null_before_wire_value ? 1 : 0)), + hpack_length(hpack_length + (insert_null_before_wire_value ? 1 : 0)) {} Slice data; const uint8_t huffman_prefix; const bool insert_null_before_wire_value; const size_t length; + const size_t hpack_length; }; +// Construct a wire value from a slice. +// true_binary_enabled => use the true binary system +// is_bin_hdr => the header is -bin suffixed WireValue GetWireValue(Slice value, bool true_binary_enabled, bool is_bin_hdr) { if (is_bin_hdr) { if (true_binary_enabled) { return WireValue(0x00, true, std::move(value)); } else { - return WireValue(0x80, false, - Slice(grpc_chttp2_base64_encode_and_huffman_compress( - value.c_slice()))); + uint32_t hpack_length; + Slice output(grpc_chttp2_base64_encode_and_huffman_compress( + value.c_slice(), &hpack_length)); + return WireValue(0x80, false, std::move(output), hpack_length); } } else { // TODO(ctiller): opportunistically compress non-binary headers @@ -185,6 +198,8 @@ class BinaryStringValue { Slice data() { return std::move(wire_value_.data); } + uint32_t hpack_length() { return wire_value_.hpack_length; } + private: WireValue wire_value_; VarintWriter<1> len_val_; @@ -232,14 +247,21 @@ void Encoder::EmitIndexed(uint32_t elem_index) { w.Write(0x80, output_.AddTiny(w.length())); } -void Encoder::EmitLitHdrWithNonBinaryStringKeyIncIdx(Slice key_slice, - Slice value_slice) { +uint32_t Encoder::EmitLitHdrWithNonBinaryStringKeyIncIdx(Slice key_slice, + Slice value_slice) { + auto key_len = key_slice.length(); + auto value_len = value_slice.length(); StringKey key(std::move(key_slice)); key.WritePrefix(0x40, output_.AddTiny(key.prefix_length())); output_.Append(key.key()); NonBinaryStringValue emit(std::move(value_slice)); emit.WritePrefix(output_.AddTiny(emit.prefix_length())); + // Allocate an index in the hpack table for this newly emitted entry. + // (we do so here because we know the length of the key and value) + uint32_t index = compressor_->table_.AllocateIndex( + key_len + value_len + hpack_constants::kEntryOverhead); output_.Append(emit.data()); + return index; } void Encoder::EmitLitHdrWithBinaryStringKeyNotIdx(Slice key_slice, @@ -252,14 +274,20 @@ void Encoder::EmitLitHdrWithBinaryStringKeyNotIdx(Slice key_slice, output_.Append(emit.data()); } -void Encoder::EmitLitHdrWithBinaryStringKeyIncIdx(Slice key_slice, - Slice value_slice) { +uint32_t Encoder::EmitLitHdrWithBinaryStringKeyIncIdx(Slice key_slice, + Slice value_slice) { + auto key_len = key_slice.length(); StringKey key(std::move(key_slice)); key.WritePrefix(0x40, output_.AddTiny(key.prefix_length())); output_.Append(key.key()); BinaryStringValue emit(std::move(value_slice), use_true_binary_metadata_); emit.WritePrefix(output_.AddTiny(emit.prefix_length())); + // Allocate an index in the hpack table for this newly emitted entry. + // (we do so here because we know the length of the key and value) + uint32_t index = compressor_->table_.AllocateIndex( + key_len + emit.hpack_length() + hpack_constants::kEntryOverhead); output_.Append(emit.data()); + return index; } void Encoder::EmitLitHdrWithBinaryStringKeyNotIdx(uint32_t key_index, @@ -308,8 +336,7 @@ void SliceIndex::EmitTo(absl::string_view key, const Slice& value, encoder->EmitIndexed(table.DynamicIndex(it->index)); } else { // Not current, emit a new literal and update the index. - it->index = table.AllocateIndex(transport_length); - encoder->EmitLitHdrWithNonBinaryStringKeyIncIdx( + it->index = encoder->EmitLitHdrWithNonBinaryStringKeyIncIdx( Slice::FromStaticString(key), value.Ref()); } // Bubble this entry up if we can - ensures that the most used values end @@ -327,9 +354,8 @@ void SliceIndex::EmitTo(absl::string_view key, const Slice& value, prev = it; } // No hit, emit a new literal and add it to the index. - uint32_t index = table.AllocateIndex(transport_length); - encoder->EmitLitHdrWithNonBinaryStringKeyIncIdx(Slice::FromStaticString(key), - value.Ref()); + uint32_t index = encoder->EmitLitHdrWithNonBinaryStringKeyIncIdx( + Slice::FromStaticString(key), value.Ref()); values_.emplace_back(value.Ref(), index); } @@ -386,7 +412,7 @@ void Compressor::EncodeWith( if (GPR_LIKELY(index != 0)) { encoder->EmitIndexed(index); } else { - encoder->EmitLitHdrWithNonBinaryStringKeyIncIdx( + encoder->EmitLitHdrWithNonBinaryStringKeyNotIdx( Slice::FromStaticString(":status"), Slice::FromInt64(status)); } } @@ -414,13 +440,12 @@ void Compressor::EncodeWith( } void Encoder::EncodeAlwaysIndexed(uint32_t* index, absl::string_view key, - Slice value, size_t transport_length) { + Slice value, size_t) { if (compressor_->table_.ConvertableToDynamicIndex(*index)) { EmitIndexed(compressor_->table_.DynamicIndex(*index)); } else { - *index = compressor_->table_.AllocateIndex(transport_length); - EmitLitHdrWithNonBinaryStringKeyIncIdx(Slice::FromStaticString(key), - std::move(value)); + *index = EmitLitHdrWithNonBinaryStringKeyIncIdx( + Slice::FromStaticString(key), std::move(value)); } } @@ -431,10 +456,8 @@ void Encoder::EncodeIndexedKeyWithBinaryValue(uint32_t* index, EmitLitHdrWithBinaryStringKeyNotIdx( compressor_->table_.DynamicIndex(*index), std::move(value)); } else { - *index = compressor_->table_.AllocateIndex(key.length() + value.length() + - hpack_constants::kEntryOverhead); - EmitLitHdrWithBinaryStringKeyIncIdx(Slice::FromStaticString(key), - std::move(value)); + *index = EmitLitHdrWithBinaryStringKeyIncIdx(Slice::FromStaticString(key), + std::move(value)); } } @@ -474,11 +497,9 @@ void TimeoutCompressorImpl::EncodeWith(absl::string_view key, previous_timeouts_.pop_back(); } Slice encoded = timeout.Encode(); - uint32_t index = table.AllocateIndex(key.length() + encoded.length() + - hpack_constants::kEntryOverhead); + uint32_t index = encoder->EmitLitHdrWithNonBinaryStringKeyIncIdx( + Slice::FromStaticString(key), std::move(encoded)); previous_timeouts_.push_back(PreviousTimeout{timeout, index}); - encoder->EmitLitHdrWithNonBinaryStringKeyIncIdx(Slice::FromStaticString(key), - std::move(encoded)); } Encoder::Encoder(HPackCompressor* compressor, bool use_true_binary_metadata, diff --git a/src/core/ext/transport/chttp2/transport/hpack_encoder.h b/src/core/ext/transport/chttp2/transport/hpack_encoder.h index 4e149f1ca7d60..c22c8d7c7b3e1 100644 --- a/src/core/ext/transport/chttp2/transport/hpack_encoder.h +++ b/src/core/ext/transport/chttp2/transport/hpack_encoder.h @@ -61,9 +61,12 @@ class Encoder { void AdvertiseTableSizeChange(); void EmitIndexed(uint32_t index); - void EmitLitHdrWithNonBinaryStringKeyIncIdx(Slice key_slice, - Slice value_slice); - void EmitLitHdrWithBinaryStringKeyIncIdx(Slice key_slice, Slice value_slice); + GRPC_MUST_USE_RESULT + uint32_t EmitLitHdrWithNonBinaryStringKeyIncIdx(Slice key_slice, + Slice value_slice); + GRPC_MUST_USE_RESULT + uint32_t EmitLitHdrWithBinaryStringKeyIncIdx(Slice key_slice, + Slice value_slice); void EmitLitHdrWithBinaryStringKeyNotIdx(Slice key_slice, Slice value_slice); void EmitLitHdrWithBinaryStringKeyNotIdx(uint32_t key_index, Slice value_slice); @@ -233,12 +236,9 @@ class Compressor> { } auto key = Slice::FromStaticString(MetadataTrait::key()); auto encoded_value = MetadataTrait::Encode(value); - size_t transport_length = - key.length() + encoded_value.length() + hpack_constants::kEntryOverhead; if (index != nullptr) { - *index = table.AllocateIndex(transport_length); - encoder->EmitLitHdrWithNonBinaryStringKeyIncIdx(std::move(key), - std::move(encoded_value)); + *index = encoder->EmitLitHdrWithNonBinaryStringKeyIncIdx( + std::move(key), std::move(encoded_value)); } else { encoder->EmitLitHdrWithNonBinaryStringKeyNotIdx(std::move(key), std::move(encoded_value)); diff --git a/src/core/ext/transport/chttp2/transport/hpack_encoder_table.cc b/src/core/ext/transport/chttp2/transport/hpack_encoder_table.cc index 209a4eea3a2dc..55d2e444ddac9 100644 --- a/src/core/ext/transport/chttp2/transport/hpack_encoder_table.cc +++ b/src/core/ext/transport/chttp2/transport/hpack_encoder_table.cc @@ -23,6 +23,8 @@ namespace grpc_core { uint32_t HPackEncoderTable::AllocateIndex(size_t element_size) { + GPR_DEBUG_ASSERT(element_size >= 32); + uint32_t new_index = tail_remote_index_ + table_elems_ + 1; GPR_DEBUG_ASSERT(element_size <= MaxEntrySize()); diff --git a/src/core/ext/transport/chttp2/transport/hpack_encoder_table.h b/src/core/ext/transport/chttp2/transport/hpack_encoder_table.h index 6fc44824974eb..623e50bf62078 100644 --- a/src/core/ext/transport/chttp2/transport/hpack_encoder_table.h +++ b/src/core/ext/transport/chttp2/transport/hpack_encoder_table.h @@ -50,6 +50,8 @@ class HPackEncoderTable { uint32_t max_size() const { return max_table_size_; } // Get the current table size uint32_t test_only_table_size() const { return table_size_; } + // Get the number of entries in the table + uint32_t test_only_table_elems() const { return table_elems_; } // Convert an element index into a dynamic index uint32_t DynamicIndex(uint32_t index) const { diff --git a/src/core/ext/transport/chttp2/transport/hpack_parser.cc b/src/core/ext/transport/chttp2/transport/hpack_parser.cc index 09d154e5c5340..2e20c773b1153 100644 --- a/src/core/ext/transport/chttp2/transport/hpack_parser.cc +++ b/src/core/ext/transport/chttp2/transport/hpack_parser.cc @@ -38,7 +38,6 @@ #include "absl/types/span.h" #include "absl/types/variant.h" -#include #include #include "src/core/ext/transport/chttp2/transport/decode_huff.h" @@ -46,9 +45,11 @@ #include "src/core/lib/debug/stats.h" #include "src/core/lib/debug/stats_data.h" #include "src/core/lib/debug/trace.h" +#include "src/core/lib/gprpp/crash.h" #include "src/core/lib/gprpp/status_helper.h" #include "src/core/lib/slice/slice.h" #include "src/core/lib/slice/slice_refcount.h" +#include "src/core/lib/surface/validate_metadata.h" #include "src/core/lib/transport/parsed_metadata.h" // IWYU pragma: no_include @@ -80,6 +81,40 @@ struct Base64InverseTable { }; constexpr Base64InverseTable kBase64InverseTable; + +absl::Status EnsureStreamError(absl::Status error) { + if (error.ok()) return error; + return grpc_error_set_int(std::move(error), StatusIntProperty::kStreamId, 0); +} + +bool IsStreamError(const absl::Status& status) { + intptr_t stream_id; + return grpc_error_get_int(status, StatusIntProperty::kStreamId, &stream_id); +} + +class MetadataSizeLimitExceededEncoder { + public: + explicit MetadataSizeLimitExceededEncoder(std::string& summary) + : summary_(summary) {} + + void Encode(const Slice& key, const Slice& value) { + AddToSummary(key.as_string_view(), value.size()); + } + + template + void Encode(Key, const Value& value) { + AddToSummary(Key::key(), EncodedSizeOfKey(Key(), value)); + } + + private: + void AddToSummary(absl::string_view key, + size_t value_length) GPR_ATTRIBUTE_NOINLINE { + absl::StrAppend(&summary_, " ", key, ":", + hpack_constants::SizeForEntry(key.size(), value_length), + "B"); + } + std::string& summary_; +}; } // namespace // Input tracks the current byte through the input data and provides it @@ -121,7 +156,8 @@ class HPackParser::Input { // of stream absl::optional Next() { if (end_of_stream()) { - return UnexpectedEOF(absl::optional()); + UnexpectedEOF(); + return absl::optional(); } return *begin_++; } @@ -187,7 +223,10 @@ class HPackParser::Input { // Parse a string prefix absl::optional ParseStringPrefix() { auto cur = Next(); - if (!cur.has_value()) return {}; + if (!cur.has_value()) { + GPR_DEBUG_ASSERT(eof_error()); + return {}; + } // Huffman if the top bit is 1 const bool huff = (*cur & 0x80) != 0; // String length @@ -195,14 +234,19 @@ class HPackParser::Input { if (strlen == 0x7f) { // all ones ==> varint string length auto v = ParseVarint(0x7f); - if (!v.has_value()) return {}; + if (!v.has_value()) { + GPR_DEBUG_ASSERT(eof_error()); + return {}; + } strlen = *v; } return StringPrefix{strlen, huff}; } // Check if we saw an EOF.. must be verified before looking at TakeError - bool eof_error() const { return eof_error_; } + bool eof_error() const { + return eof_error_ || (!error_.ok() && !IsStreamError(error_)); + } // Extract the parse error, leaving the current error as NONE. grpc_error_handle TakeError() { @@ -211,34 +255,33 @@ class HPackParser::Input { return out; } - // Set the current error - allows the rest of the code not to need to pass - // around StatusOr<> which would be prohibitive here. - GPR_ATTRIBUTE_NOINLINE void SetError(grpc_error_handle error) { - if (!error_.ok() || eof_error_) { - return; - } - error_ = error; - begin_ = end_; + bool has_error() const { return !error_.ok(); } + + // Set the current error - tweaks the error to include a stream id so that + // chttp2 does not close the connection. + // Intended for errors that are specific to a stream and recoverable. + // Callers should ensure that any hpack table updates happen. + GPR_ATTRIBUTE_NOINLINE void SetErrorAndContinueParsing( + grpc_error_handle error) { + GPR_ASSERT(!error.ok()); + // StreamId is used as a signal to skip this stream but keep the connection + // alive + SetError(EnsureStreamError(std::move(error))); } - // If no error is set, set it to the value produced by error_factory. - // Return return_value unchanged. - template - GPR_ATTRIBUTE_NOINLINE T MaybeSetErrorAndReturn(F error_factory, - T return_value) { - if (!error_.ok() || eof_error_) return return_value; - error_ = error_factory(); + // Set the current error, and skip past remaining bytes. + // Intended for unrecoverable errors, with the expectation that they will + // close the connection on return to chttp2. + GPR_ATTRIBUTE_NOINLINE void SetErrorAndStopParsing(grpc_error_handle error) { + GPR_ASSERT(!error.ok()); + SetError(std::move(error)); begin_ = end_; - return return_value; } - // Set the error to an unexpected eof, and return result (code golfed as this - // is a common case) - template - T UnexpectedEOF(T return_value) { - if (!error_.ok()) return return_value; + // Set the error to an unexpected eof + void UnexpectedEOF() { + if (!error_.ok() && !IsStreamError(error_)) return; eof_error_ = true; - return return_value; } // Update the frontier - signifies we've successfully parsed another element @@ -251,14 +294,24 @@ class HPackParser::Input { // Helper to set the error to out of range for ParseVarint absl::optional ParseVarintOutOfRange(uint32_t value, uint8_t last_byte) { - return MaybeSetErrorAndReturn( - [value, last_byte] { - return GRPC_ERROR_CREATE(absl::StrFormat( - "integer overflow in hpack integer decoding: have 0x%08x, " - "got byte 0x%02x on byte 5", - value, last_byte)); - }, - absl::optional()); + SetErrorAndStopParsing(absl::InternalError(absl::StrFormat( + "integer overflow in hpack integer decoding: have 0x%08x, " + "got byte 0x%02x on byte 5", + value, last_byte))); + return absl::optional(); + } + + // If no error is set, set it to the given error (i.e. first error wins) + // Do not use this directly, instead use SetErrorAndContinueParsing or + // SetErrorAndStopParsing. + void SetError(grpc_error_handle error) { + if (!error_.ok() || eof_error_) { + if (!IsStreamError(error) && IsStreamError(error_)) { + error_ = std::move(error); // connection errors dominate + } + return; + } + error_ = std::move(error); } // Refcount if we are backed by a slice @@ -279,6 +332,21 @@ class HPackParser::Input { // management characteristics class HPackParser::String { public: + // ParseResult carries both a ParseStatus and the parsed string + struct ParseResult; + // Result of parsing a string + enum class ParseStatus { + // Parsed OK + kOk, + // Parse reached end of the current frame + kEof, + // Parse failed due to a huffman decode error + kParseHuffFailed, + // Parse failed due to a base64 decode error + kUnbase64Failed, + }; + + String() : value_(absl::Span()) {} String(const String&) = delete; String& operator=(const String&) = delete; String(String&& other) noexcept : value_(std::move(other.value_)) { @@ -308,72 +376,10 @@ class HPackParser::String { } // Parse a non-binary string - static absl::optional Parse(Input* input) { - auto pfx = input->ParseStringPrefix(); - if (!pfx.has_value()) return {}; - if (pfx->huff) { - // Huffman coded - std::vector output; - auto v = ParseHuff(input, pfx->length, - [&output](uint8_t c) { output.push_back(c); }); - if (!v) return {}; - return String(std::move(output)); - } - return ParseUncompressed(input, pfx->length); - } + static ParseResult Parse(Input* input); // Parse a binary string - static absl::optional ParseBinary(Input* input) { - auto pfx = input->ParseStringPrefix(); - if (!pfx.has_value()) return {}; - if (!pfx->huff) { - if (pfx->length > 0 && input->peek() == 0) { - // 'true-binary' - input->Advance(1); - return ParseUncompressed(input, pfx->length - 1); - } - // Base64 encoded... pull out the string, then unbase64 it - auto base64 = ParseUncompressed(input, pfx->length); - if (!base64.has_value()) return {}; - return Unbase64(input, std::move(*base64)); - } else { - // Huffman encoded... - std::vector decompressed; - // State here says either we don't know if it's base64 or binary, or we do - // and what is it. - enum class State { kUnsure, kBinary, kBase64 }; - State state = State::kUnsure; - auto decompressed_ok = - ParseHuff(input, pfx->length, [&state, &decompressed](uint8_t c) { - if (state == State::kUnsure) { - // First byte... if it's zero it's binary - if (c == 0) { - // Save the type, and skip the zero - state = State::kBinary; - return; - } else { - // Flag base64, store this value - state = State::kBase64; - } - } - // Non-first byte, or base64 first byte - decompressed.push_back(c); - }); - if (!decompressed_ok) return {}; - switch (state) { - case State::kUnsure: - // No bytes, empty span - return String(absl::Span()); - case State::kBinary: - // Binary, we're done - return String(std::move(decompressed)); - case State::kBase64: - // Base64 - unpack it - return Unbase64(input, String(std::move(decompressed))); - } - GPR_UNREACHABLE_CODE(abort();); - } - } + static ParseResult ParseBinary(Input* input); private: void AppendBytes(const uint8_t* data, size_t length); @@ -385,54 +391,27 @@ class HPackParser::String { // Parse some huffman encoded bytes, using output(uint8_t b) to emit each // decoded byte. template - static bool ParseHuff(Input* input, uint32_t length, Out output) { + static ParseStatus ParseHuff(Input* input, uint32_t length, Out output) { // If there's insufficient bytes remaining, return now. if (input->remaining() < length) { - return input->UnexpectedEOF(false); + input->UnexpectedEOF(); + GPR_DEBUG_ASSERT(input->eof_error()); + return ParseStatus::kEof; } // Grab the byte range, and iterate through it. const uint8_t* p = input->cur_ptr(); input->Advance(length); - return HuffDecoder(output, p, p + length).Run(); + return HuffDecoder(output, p, p + length).Run() + ? ParseStatus::kOk + : ParseStatus::kParseHuffFailed; } // Parse some uncompressed string bytes. - static absl::optional ParseUncompressed(Input* input, - uint32_t length) { - // Check there's enough bytes - if (input->remaining() < length) { - return input->UnexpectedEOF(absl::optional()); - } - auto* refcount = input->slice_refcount(); - auto* p = input->cur_ptr(); - input->Advance(length); - if (refcount != nullptr) { - return String(refcount, p, p + length); - } else { - return String(absl::Span(p, length)); - } - } + static ParseResult ParseUncompressed(Input* input, uint32_t length, + uint32_t wire_size); // Turn base64 encoded bytes into not base64 encoded bytes. - // Only takes input to set an error on failure. - static absl::optional Unbase64(Input* input, String s) { - absl::optional> result; - if (auto* p = absl::get_if(&s.value_)) { - result = Unbase64Loop(p->begin(), p->end()); - } - if (auto* p = absl::get_if>(&s.value_)) { - result = Unbase64Loop(p->begin(), p->end()); - } - if (auto* p = absl::get_if>(&s.value_)) { - result = Unbase64Loop(p->data(), p->data() + p->size()); - } - if (!result.has_value()) { - return input->MaybeSetErrorAndReturn( - [] { return GRPC_ERROR_CREATE("illegal base64 encoding"); }, - absl::optional()); - } - return String(std::move(*result)); - } + static ParseResult Unbase64(String s); // Main loop for Unbase64 static absl::optional> Unbase64Loop(const uint8_t* cur, @@ -519,25 +498,154 @@ class HPackParser::String { absl::variant, std::vector> value_; }; +struct HPackParser::String::ParseResult { + ParseResult() = delete; + ParseResult(ParseStatus status, size_t wire_size, String value) + : status(status), wire_size(wire_size), value(std::move(value)) {} + ParseStatus status; + size_t wire_size; + String value; +}; + +HPackParser::String::ParseResult HPackParser::String::ParseUncompressed( + Input* input, uint32_t length, uint32_t wire_size) { + // Check there's enough bytes + if (input->remaining() < length) { + input->UnexpectedEOF(); + GPR_DEBUG_ASSERT(input->eof_error()); + return ParseResult{ParseStatus::kEof, wire_size, String{}}; + } + auto* refcount = input->slice_refcount(); + auto* p = input->cur_ptr(); + input->Advance(length); + if (refcount != nullptr) { + return ParseResult{ParseStatus::kOk, wire_size, + String(refcount, p, p + length)}; + } else { + return ParseResult{ParseStatus::kOk, wire_size, + String(absl::Span(p, length))}; + } +} + +HPackParser::String::ParseResult HPackParser::String::Unbase64(String s) { + absl::optional> result; + if (auto* p = absl::get_if(&s.value_)) { + result = Unbase64Loop(p->begin(), p->end()); + } + if (auto* p = absl::get_if>(&s.value_)) { + result = Unbase64Loop(p->begin(), p->end()); + } + if (auto* p = absl::get_if>(&s.value_)) { + result = Unbase64Loop(p->data(), p->data() + p->size()); + } + if (!result.has_value()) { + return ParseResult{ParseStatus::kUnbase64Failed, s.string_view().length(), + String{}}; + } + return ParseResult{ParseStatus::kOk, s.string_view().length(), + String(std::move(*result))}; +} + +HPackParser::String::ParseResult HPackParser::String::Parse(Input* input) { + auto pfx = input->ParseStringPrefix(); + if (!pfx.has_value()) { + GPR_DEBUG_ASSERT(input->eof_error()); + return ParseResult{ParseStatus::kEof, 0, String{}}; + } + if (pfx->huff) { + // Huffman coded + std::vector output; + ParseStatus sts = ParseHuff(input, pfx->length, + [&output](uint8_t c) { output.push_back(c); }); + size_t wire_len = output.size(); + return ParseResult{sts, wire_len, String(std::move(output))}; + } + return ParseUncompressed(input, pfx->length, pfx->length); +} + +HPackParser::String::ParseResult HPackParser::String::ParseBinary( + Input* input) { + auto pfx = input->ParseStringPrefix(); + if (!pfx.has_value()) { + GPR_DEBUG_ASSERT(input->eof_error()); + return ParseResult{ParseStatus::kEof, 0, String{}}; + } + if (!pfx->huff) { + if (pfx->length > 0 && input->peek() == 0) { + // 'true-binary' + input->Advance(1); + return ParseUncompressed(input, pfx->length - 1, pfx->length); + } + // Base64 encoded... pull out the string, then unbase64 it + auto base64 = ParseUncompressed(input, pfx->length, pfx->length); + if (base64.status != ParseStatus::kOk) return base64; + return Unbase64(std::move(base64.value)); + } else { + // Huffman encoded... + std::vector decompressed; + // State here says either we don't know if it's base64 or binary, or we do + // and what is it. + enum class State { kUnsure, kBinary, kBase64 }; + State state = State::kUnsure; + auto sts = + ParseHuff(input, pfx->length, [&state, &decompressed](uint8_t c) { + if (state == State::kUnsure) { + // First byte... if it's zero it's binary + if (c == 0) { + // Save the type, and skip the zero + state = State::kBinary; + return; + } else { + // Flag base64, store this value + state = State::kBase64; + } + } + // Non-first byte, or base64 first byte + decompressed.push_back(c); + }); + if (sts != ParseStatus::kOk) { + return ParseResult{sts, 0, String{}}; + } + switch (state) { + case State::kUnsure: + // No bytes, empty span + return ParseResult{ParseStatus::kOk, 0, + String(absl::Span())}; + case State::kBinary: + // Binary, we're done + { + size_t wire_len = decompressed.size(); + return ParseResult{ParseStatus::kOk, wire_len, + String(std::move(decompressed))}; + } + case State::kBase64: + // Base64 - unpack it + return Unbase64(String(std::move(decompressed))); + } + GPR_UNREACHABLE_CODE(abort();); + } +} + // Parser parses one key/value pair from a byte stream. class HPackParser::Parser { public: Parser(Input* input, grpc_metadata_batch* metadata_buffer, HPackTable* table, uint8_t* dynamic_table_updates_allowed, uint32_t* frame_length, - RandomEarlyDetection* metadata_early_detection, bool is_last, - LogInfo log_info) + RandomEarlyDetection* metadata_early_detection, LogInfo log_info) : input_(input), metadata_buffer_(metadata_buffer), table_(table), dynamic_table_updates_allowed_(dynamic_table_updates_allowed), frame_length_(frame_length), metadata_early_detection_(metadata_early_detection), - is_last_(is_last), log_info_(log_info) {} // Skip any priority bits, or return false on failure bool SkipPriority() { - if (input_->remaining() < 5) return input_->UnexpectedEOF(false); + if (input_->remaining() < 5) { + input_->UnexpectedEOF(); + return false; + } input_->Advance(5); return true; } @@ -610,8 +718,9 @@ class HPackParser::Parser { case 8: if (cur == 0x80) { // illegal value. - return input_->MaybeSetErrorAndReturn( - [] { return GRPC_ERROR_CREATE("Illegal hpack op code"); }, false); + input_->SetErrorAndStopParsing( + absl::InternalError("Illegal hpack op code")); + return false; } ABSL_FALLTHROUGH_INTENDED; case 9: @@ -648,24 +757,31 @@ class HPackParser::Parser { type = "???"; break; } - gpr_log(GPR_DEBUG, "HTTP:%d:%s:%s: %s", log_info_.stream_id, type, - log_info_.is_client ? "CLI" : "SVR", memento.DebugString().c_str()); + gpr_log(GPR_DEBUG, "HTTP:%d:%s:%s: %s%s", log_info_.stream_id, type, + log_info_.is_client ? "CLI" : "SVR", + memento.md.DebugString().c_str(), + memento.parse_status.ok() + ? "" + : absl::StrCat( + " (parse error: ", memento.parse_status.ToString(), ")") + .c_str()); } - bool EmitHeader(const HPackTable::Memento& md) { + void EmitHeader(const HPackTable::Memento& md) { // Pass up to the transport - if (GPR_UNLIKELY(metadata_buffer_ == nullptr)) return true; - *frame_length_ += md.transport_size(); - if (metadata_early_detection_->MustReject(*frame_length_)) { + *frame_length_ += md.md.transport_size(); + if (!input_->has_error() && + metadata_early_detection_->MustReject(*frame_length_)) { // Reject any requests above hard metadata limit. - return HandleMetadataSizeLimitExceeded(md, /*exceeded_hard_limit=*/true); - } else if (is_last_ && metadata_early_detection_->Reject(*frame_length_)) { - // Reject some random sample of requests above soft metadata limit. - return HandleMetadataSizeLimitExceeded(md, /*exceeded_hard_limit=*/false); + HandleMetadataHardSizeLimitExceeded(md); + } + if (!md.parse_status.ok()) { + // Reject any requests with invalid metadata. + HandleMetadataParseError(md.parse_status); + } + if (GPR_LIKELY(metadata_buffer_ != nullptr)) { + metadata_buffer_->Set(md.md); } - - metadata_buffer_->Set(md); - return true; } bool FinishHeaderAndAddToTable(absl::optional md) { @@ -676,73 +792,149 @@ class HPackParser::Parser { LogHeader(*md); } // Emit whilst we own the metadata. - auto r = EmitHeader(*md); + EmitHeader(*md); // Add to the hpack table grpc_error_handle err = table_->Add(std::move(*md)); if (GPR_UNLIKELY(!err.ok())) { - input_->SetError(err); + input_->SetErrorAndStopParsing(std::move(err)); return false; }; - return r; + return true; } bool FinishHeaderOmitFromTable(absl::optional md) { // Allow higher code to just pass in failures ... simplifies things a bit. if (!md.has_value()) return false; - return FinishHeaderOmitFromTable(*md); + FinishHeaderOmitFromTable(*md); + return true; } - bool FinishHeaderOmitFromTable(const HPackTable::Memento& md) { + void FinishHeaderOmitFromTable(const HPackTable::Memento& md) { // Log if desired if (GRPC_TRACE_FLAG_ENABLED(grpc_trace_chttp2_hpack_parser)) { LogHeader(md); } - return EmitHeader(md); + EmitHeader(md); } + // Helper type to build a memento from a key & value, and to consolidate some + // tricky error path code. + class MementoBuilder { + public: + explicit MementoBuilder(Input* input, absl::string_view key_string, + absl::Status status = absl::OkStatus()) + : input_(input), key_string_(key_string), status_(std::move(status)) {} + + auto ErrorHandler() { + return [this](absl::string_view error, const Slice&) { + auto message = + absl::StrCat("Error parsing '", key_string_, + "' metadata: error=", error, " key=", key_string_); + gpr_log(GPR_ERROR, "%s", message.c_str()); + if (status_.ok()) { + status_ = absl::InternalError(message); + } + }; + } + + HPackTable::Memento Build(ParsedMetadata memento) { + return HPackTable::Memento{std::move(memento), std::move(status_)}; + } + + // Handle the result of parsing a value. + // Returns true if parsing should continue, false if it should stop. + // Stores an error on the input if necessary. + bool HandleParseResult(String::ParseStatus status) { + auto continuable = [this](absl::string_view error) { + auto this_error = absl::InternalError(absl::StrCat( + "Error parsing '", key_string_, "' metadata: error=", error)); + if (status_.ok()) status_ = this_error; + input_->SetErrorAndContinueParsing(std::move(this_error)); + }; + switch (status) { + case String::ParseStatus::kOk: + return true; + case String::ParseStatus::kParseHuffFailed: + input_->SetErrorAndStopParsing( + absl::InternalError("Huffman decoding failed")); + return false; + case String::ParseStatus::kUnbase64Failed: + continuable("illegal base64 encoding"); + return true; + case String::ParseStatus::kEof: + GPR_DEBUG_ASSERT(input_->eof_error()); + return false; + } + GPR_UNREACHABLE_CODE(return false); + } + + private: + Input* input_; + absl::string_view key_string_; + absl::Status status_; + }; + // Parse a string encoded key and a string encoded value absl::optional ParseLiteralKey() { auto key = String::Parse(input_); - if (!key.has_value()) return {}; - auto value = ParseValueString(absl::EndsWith(key->string_view(), "-bin")); - if (GPR_UNLIKELY(!value.has_value())) { - return {}; + switch (key.status) { + case String::ParseStatus::kOk: + break; + case String::ParseStatus::kParseHuffFailed: + input_->SetErrorAndStopParsing( + absl::InternalError("Huffman decoding failed")); + return absl::nullopt; + case String::ParseStatus::kUnbase64Failed: + Crash("unreachable"); + case String::ParseStatus::kEof: + GPR_DEBUG_ASSERT(input_->eof_error()); + return absl::nullopt; } - auto key_string = key->string_view(); - auto value_slice = value->Take(); - const auto transport_size = key_string.size() + value_slice.size() + - hpack_constants::kEntryOverhead; - return grpc_metadata_batch::Parse( - key->string_view(), std::move(value_slice), transport_size, - [key_string](absl::string_view error, const Slice& value) { - ReportMetadataParseError(key_string, error, value.as_string_view()); - }); + auto key_string = key.value.string_view(); + auto value = ParseValueString(absl::EndsWith(key_string, "-bin")); + MementoBuilder builder(input_, key_string, + EnsureStreamError(ValidateKey(key_string))); + if (!builder.HandleParseResult(value.status)) return absl::nullopt; + auto value_slice = value.value.Take(); + const auto transport_size = + key_string.size() + value.wire_size + hpack_constants::kEntryOverhead; + return builder.Build( + grpc_metadata_batch::Parse(key_string, std::move(value_slice), + transport_size, builder.ErrorHandler())); + } + + absl::Status ValidateKey(absl::string_view key) { + if (key == HttpSchemeMetadata::key() || key == HttpMethodMetadata::key() || + key == HttpAuthorityMetadata::key() || key == HttpPathMetadata::key() || + key == HttpStatusMetadata::key()) { + return absl::OkStatus(); + } + return ValidateHeaderKeyIsLegal(key); } // Parse an index encoded key and a string encoded value absl::optional ParseIdxKey(uint32_t index) { const auto* elem = table_->Lookup(index); if (GPR_UNLIKELY(elem == nullptr)) { - return InvalidHPackIndexError(index, - absl::optional()); - } - auto value = ParseValueString(elem->is_binary_header()); - if (GPR_UNLIKELY(!value.has_value())) return {}; - return elem->WithNewValue( - value->Take(), [=](absl::string_view error, const Slice& value) { - ReportMetadataParseError(elem->key(), error, value.as_string_view()); - }); - } + InvalidHPackIndexError(index); + return absl::optional(); + } + MementoBuilder builder(input_, elem->md.key(), elem->parse_status); + auto value = ParseValueString(elem->md.is_binary_header()); + if (!builder.HandleParseResult(value.status)) return absl::nullopt; + return builder.Build(elem->md.WithNewValue( + value.value.Take(), value.wire_size, builder.ErrorHandler())); + }; // Parse a varint index encoded key and a string encoded value absl::optional ParseVarIdxKey(uint32_t offset) { auto index = input_->ParseVarint(offset); - if (GPR_UNLIKELY(!index.has_value())) return {}; + if (GPR_UNLIKELY(!index.has_value())) return absl::nullopt; return ParseIdxKey(*index); } // Parse a string, figuring out if it's binary or not by the key name. - absl::optional ParseValueString(bool is_binary) { + String::ParseResult ParseValueString(bool is_binary) { if (is_binary) { return String::ParseBinary(input_); } else { @@ -756,26 +948,25 @@ class HPackParser::Parser { if (!index.has_value()) return false; const auto* elem = table_->Lookup(*index); if (GPR_UNLIKELY(elem == nullptr)) { - return InvalidHPackIndexError(*index, false); + InvalidHPackIndexError(*index); + return false; } - return FinishHeaderOmitFromTable(*elem); + FinishHeaderOmitFromTable(*elem); + return true; } // finish parsing a max table size change bool FinishMaxTableSize(absl::optional size) { if (!size.has_value()) return false; if (*dynamic_table_updates_allowed_ == 0) { - return input_->MaybeSetErrorAndReturn( - [] { - return GRPC_ERROR_CREATE( - "More than two max table size changes in a single frame"); - }, - false); + input_->SetErrorAndStopParsing(absl::InternalError( + "More than two max table size changes in a single frame")); + return false; } (*dynamic_table_updates_allowed_)--; grpc_error_handle err = table_->SetCurrentTableSize(*size); if (!err.ok()) { - input_->SetError(err); + input_->SetErrorAndStopParsing(std::move(err)); return false; } return true; @@ -783,47 +974,28 @@ class HPackParser::Parser { // Set an invalid hpack index error if no error has been set. Returns result // unmodified. - template - R InvalidHPackIndexError(uint32_t index, R result) { - return input_->MaybeSetErrorAndReturn( - [this, index] { - return grpc_error_set_int( - grpc_error_set_int( - GRPC_ERROR_CREATE("Invalid HPACK index received"), - StatusIntProperty::kIndex, static_cast(index)), - StatusIntProperty::kSize, - static_cast(this->table_->num_entries())); - }, - std::move(result)); - } - - class MetadataSizeLimitExceededEncoder { - public: - explicit MetadataSizeLimitExceededEncoder(std::string& summary) - : summary_(summary) {} - - void Encode(const Slice& key, const Slice& value) { - AddToSummary(key.as_string_view(), value.size()); - } - - template - void Encode(Key, const Value& value) { - AddToSummary(Key::key(), EncodedSizeOfKey(Key(), value)); - } + void InvalidHPackIndexError(uint32_t index) { + input_->SetErrorAndStopParsing(grpc_error_set_int( + grpc_error_set_int(absl::InternalError("Invalid HPACK index received"), + StatusIntProperty::kIndex, + static_cast(index)), + StatusIntProperty::kSize, + static_cast(this->table_->num_entries()))); + } - private: - void AddToSummary(absl::string_view key, - size_t value_length) GPR_ATTRIBUTE_NOINLINE { - absl::StrAppend(&summary_, " ", key, ":", - hpack_constants::SizeForEntry(key.size(), value_length), - "B"); + GPR_ATTRIBUTE_NOINLINE + void HandleMetadataParseError(const absl::Status& status) { + if (metadata_buffer_ != nullptr) { + metadata_buffer_->Clear(); + metadata_buffer_ = nullptr; } - std::string& summary_; - }; + // StreamId is used as a signal to skip this stream but keep the connection + // alive + input_->SetErrorAndContinueParsing(status); + } GPR_ATTRIBUTE_NOINLINE - bool HandleMetadataSizeLimitExceeded(const HPackTable::Memento& md, - bool exceeded_hard_limit) { + void HandleMetadataHardSizeLimitExceeded(const HPackTable::Memento& md) { // Collect a summary of sizes so far for debugging // Do not collect contents, for fear of exposing PII. std::string summary; @@ -832,49 +1004,22 @@ class HPackParser::Parser { MetadataSizeLimitExceededEncoder encoder(summary); metadata_buffer_->Encode(&encoder); } - summary = - absl::StrCat("; adding ", md.key(), " (length ", md.transport_size(), - "B)", summary.empty() ? "" : " to ", summary); - if (exceeded_hard_limit) { - error_message = absl::StrCat( - "received metadata size exceeds hard limit (", *frame_length_, - " vs. ", metadata_early_detection_->hard_limit(), ")", summary); - } else { - error_message = absl::StrCat( - "received metadata size exceeds soft limit (", *frame_length_, - " vs. ", metadata_early_detection_->soft_limit(), - "), rejecting requests with some random probability", summary); - } - if (metadata_buffer_ != nullptr) metadata_buffer_->Clear(); - // StreamId is used as a signal to skip this stream but keep the connection - // alive - return input_->MaybeSetErrorAndReturn( - [error_message = std::move(error_message)] { - return grpc_error_set_int( - grpc_error_set_int(GRPC_ERROR_CREATE(error_message), - StatusIntProperty::kRpcStatus, - GRPC_STATUS_RESOURCE_EXHAUSTED), - StatusIntProperty::kStreamId, 0); - }, - false); - } - - static void ReportMetadataParseError(absl::string_view key, - absl::string_view error, - absl::string_view value) { - gpr_log( - GPR_ERROR, "Error parsing metadata: %s", - absl::StrCat("error=", error, " key=", key, " value=", value).c_str()); + summary = absl::StrCat("; adding ", md.md.key(), " (length ", + md.md.transport_size(), "B)", + summary.empty() ? "" : " to ", summary); + error_message = absl::StrCat( + "received metadata size exceeds hard limit (", *frame_length_, " vs. ", + metadata_early_detection_->hard_limit(), ")", summary); + HandleMetadataParseError(absl::ResourceExhaustedError(error_message)); } Input* const input_; - grpc_metadata_batch* const metadata_buffer_; + grpc_metadata_batch* metadata_buffer_; HPackTable* const table_; uint8_t* const dynamic_table_updates_allowed_; uint32_t* const frame_length_; // Random early detection of metadata size limits. RandomEarlyDetection* metadata_early_detection_; - bool is_last_; // Whether this is the last frame. const LogInfo log_info_; }; @@ -928,26 +1073,35 @@ grpc_error_handle HPackParser::Parse(const grpc_slice& slice, bool is_last) { } grpc_error_handle HPackParser::ParseInput(Input input, bool is_last) { - bool parsed_ok = ParseInputInner(&input, is_last); - if (is_last) global_stats().IncrementHttp2MetadataSize(frame_length_); - if (parsed_ok) return absl::OkStatus(); + ParseInputInner(&input); + if (is_last) { + if (metadata_early_detection_.Reject(frame_length_)) { + HandleMetadataSoftSizeLimitExceeded(&input); + } + global_stats().IncrementHttp2MetadataSize(frame_length_); + } if (input.eof_error()) { if (GPR_UNLIKELY(is_last && is_boundary())) { - return GRPC_ERROR_CREATE( + auto err = input.TakeError(); + if (!err.ok() && !IsStreamError(err)) return err; + return absl::InternalError( "Incomplete header at the end of a header/continuation sequence"); } unparsed_bytes_ = std::vector(input.frontier(), input.end_ptr()); - return absl::OkStatus(); + return input.TakeError(); } return input.TakeError(); } -bool HPackParser::ParseInputInner(Input* input, bool is_last) { +void HPackParser::ParseInputInner(Input* input) { switch (priority_) { case Priority::None: break; case Priority::Included: { - if (input->remaining() < 5) return input->UnexpectedEOF(false); + if (input->remaining() < 5) { + input->UnexpectedEOF(); + return; + } input->Advance(5); input->UpdateFrontier(); priority_ = Priority::None; @@ -956,15 +1110,35 @@ bool HPackParser::ParseInputInner(Input* input, bool is_last) { while (!input->end_of_stream()) { if (GPR_UNLIKELY(!Parser(input, metadata_buffer_, &table_, &dynamic_table_updates_allowed_, &frame_length_, - &metadata_early_detection_, is_last, log_info_) + &metadata_early_detection_, log_info_) .Parse())) { - return false; + return; } input->UpdateFrontier(); } - return true; } void HPackParser::FinishFrame() { metadata_buffer_ = nullptr; } +void HPackParser::HandleMetadataSoftSizeLimitExceeded(Input* input) { + // Collect a summary of sizes so far for debugging + // Do not collect contents, for fear of exposing PII. + std::string summary; + std::string error_message; + if (metadata_buffer_ != nullptr) { + MetadataSizeLimitExceededEncoder encoder(summary); + metadata_buffer_->Encode(&encoder); + } + error_message = absl::StrCat( + "received metadata size exceeds soft limit (", frame_length_, " vs. ", + metadata_early_detection_.soft_limit(), + "), rejecting requests with some random probability", summary); + if (metadata_buffer_ != nullptr) { + metadata_buffer_->Clear(); + metadata_buffer_ = nullptr; + } + input->SetErrorAndContinueParsing( + absl::ResourceExhaustedError(error_message)); +} + } // namespace grpc_core diff --git a/src/core/ext/transport/chttp2/transport/hpack_parser.h b/src/core/ext/transport/chttp2/transport/hpack_parser.h index e7ad604ab7b16..d2863076e0f88 100644 --- a/src/core/ext/transport/chttp2/transport/hpack_parser.h +++ b/src/core/ext/transport/chttp2/transport/hpack_parser.h @@ -105,7 +105,9 @@ class HPackParser { class String; grpc_error_handle ParseInput(Input input, bool is_last); - bool ParseInputInner(Input* input, bool is_last); + void ParseInputInner(Input* input); + GPR_ATTRIBUTE_NOINLINE + void HandleMetadataSoftSizeLimitExceeded(Input* input); // Target metadata buffer grpc_metadata_batch* metadata_buffer_ = nullptr; diff --git a/src/core/ext/transport/chttp2/transport/hpack_parser_table.cc b/src/core/ext/transport/chttp2/transport/hpack_parser_table.cc index 1e582e28bba2c..3079093c51e6b 100644 --- a/src/core/ext/transport/chttp2/transport/hpack_parser_table.cc +++ b/src/core/ext/transport/chttp2/transport/hpack_parser_table.cc @@ -82,8 +82,8 @@ void HPackTable::MementoRingBuffer::Rebuild(uint32_t max_entries) { // Evict one element from the table void HPackTable::EvictOne() { auto first_entry = entries_.PopOne(); - GPR_ASSERT(first_entry.transport_size() <= mem_used_); - mem_used_ -= first_entry.transport_size(); + GPR_ASSERT(first_entry.md.transport_size() <= mem_used_); + mem_used_ -= first_entry.md.transport_size(); } void HPackTable::SetMaxBytes(uint32_t max_bytes) { @@ -104,7 +104,7 @@ grpc_error_handle HPackTable::SetCurrentTableSize(uint32_t bytes) { return absl::OkStatus(); } if (bytes > max_bytes_) { - return GRPC_ERROR_CREATE(absl::StrFormat( + return absl::InternalError(absl::StrFormat( "Attempt to make hpack table %d bytes when max is %d bytes", bytes, max_bytes_)); } @@ -130,7 +130,7 @@ grpc_error_handle HPackTable::Add(Memento md) { } // we can't add elements bigger than the max table size - if (md.transport_size() > current_table_bytes_) { + if (md.md.transport_size() > current_table_bytes_) { // HPACK draft 10 section 4.4 states: // If the size of the new entry is less than or equal to the maximum // size, that entry is added to the table. It is not an error to @@ -145,13 +145,13 @@ grpc_error_handle HPackTable::Add(Memento md) { } // evict entries to ensure no overflow - while (md.transport_size() > + while (md.md.transport_size() > static_cast(current_table_bytes_) - mem_used_) { EvictOne(); } // copy the finalized entry in - mem_used_ += md.transport_size(); + mem_used_ += md.md.transport_size(); entries_.Put(std::move(md)); return absl::OkStatus(); } @@ -228,12 +228,14 @@ const StaticTableEntry kStaticTable[hpack_constants::kLastStaticEntry] = { HPackTable::Memento MakeMemento(size_t i) { auto sm = kStaticTable[i]; - return grpc_metadata_batch::Parse( - sm.key, Slice::FromStaticString(sm.value), - strlen(sm.key) + strlen(sm.value) + hpack_constants::kEntryOverhead, - [](absl::string_view, const Slice&) { - abort(); // not expecting to see this - }); + return HPackTable::Memento{ + grpc_metadata_batch::Parse( + sm.key, Slice::FromStaticString(sm.value), + strlen(sm.key) + strlen(sm.value) + hpack_constants::kEntryOverhead, + [](absl::string_view, const Slice&) { + abort(); // not expecting to see this + }), + absl::OkStatus()}; } } // namespace diff --git a/src/core/ext/transport/chttp2/transport/hpack_parser_table.h b/src/core/ext/transport/chttp2/transport/hpack_parser_table.h index 086647f296c78..7e29cb5fa84b4 100644 --- a/src/core/ext/transport/chttp2/transport/hpack_parser_table.h +++ b/src/core/ext/transport/chttp2/transport/hpack_parser_table.h @@ -25,6 +25,8 @@ #include +#include "absl/status/status.h" + #include "src/core/ext/transport/chttp2/transport/hpack_constants.h" #include "src/core/lib/gprpp/no_destruct.h" #include "src/core/lib/iomgr/error.h" @@ -45,7 +47,10 @@ class HPackTable { void SetMaxBytes(uint32_t max_bytes); grpc_error_handle SetCurrentTableSize(uint32_t bytes); - using Memento = ParsedMetadata; + struct Memento { + ParsedMetadata md; + absl::Status parse_status; + }; // Lookup, but don't ref. const Memento* Lookup(uint32_t index) const { @@ -68,6 +73,9 @@ class HPackTable { // Current entry count in the table. uint32_t num_entries() const { return entries_.num_entries(); } + // Current size of the table. + uint32_t test_only_table_size() const { return mem_used_; } + private: struct StaticMementos { StaticMementos(); diff --git a/src/core/lib/surface/validate_metadata.cc b/src/core/lib/surface/validate_metadata.cc index 3fef47819061d..4e38d6b91719c 100644 --- a/src/core/lib/surface/validate_metadata.cc +++ b/src/core/lib/surface/validate_metadata.cc @@ -21,46 +21,20 @@ #include "src/core/lib/surface/validate_metadata.h" #include "absl/status/status.h" +#include "absl/strings/escaping.h" +#include "absl/strings/str_cat.h" #include "absl/strings/string_view.h" #include -#include "src/core/lib/gpr/string.h" #include "src/core/lib/gprpp/bitset.h" -#include "src/core/lib/gprpp/memory.h" -#include "src/core/lib/gprpp/status_helper.h" #include "src/core/lib/iomgr/error.h" +#include "src/core/lib/slice/slice_internal.h" -static grpc_error_handle conforms_to(const grpc_slice& slice, - const grpc_core::BitSet<256>& legal_bits, - const char* err_desc) { - const uint8_t* p = GRPC_SLICE_START_PTR(slice); - const uint8_t* e = GRPC_SLICE_END_PTR(slice); - for (; p != e; p++) { - if (!legal_bits.is_set(*p)) { - size_t len; - grpc_core::UniquePtr ptr(gpr_dump_return_len( - reinterpret_cast GRPC_SLICE_START_PTR(slice), - GRPC_SLICE_LENGTH(slice), GPR_DUMP_HEX | GPR_DUMP_ASCII, &len)); - grpc_error_handle error = grpc_error_set_str( - grpc_error_set_int(GRPC_ERROR_CREATE(err_desc), - grpc_core::StatusIntProperty::kOffset, - p - GRPC_SLICE_START_PTR(slice)), - grpc_core::StatusStrProperty::kRawBytes, - absl::string_view(ptr.get(), len)); - return error; - } - } - return absl::OkStatus(); -} - -static int error2int(grpc_error_handle error) { - int r = (error.ok()); - return r; -} +namespace grpc_core { namespace { -class LegalHeaderKeyBits : public grpc_core::BitSet<256> { +class LegalHeaderKeyBits : public BitSet<256> { public: constexpr LegalHeaderKeyBits() { for (int i = 'a'; i <= 'z'; i++) set(i); @@ -71,19 +45,45 @@ class LegalHeaderKeyBits : public grpc_core::BitSet<256> { } }; constexpr LegalHeaderKeyBits g_legal_header_key_bits; -} // namespace -grpc_error_handle grpc_validate_header_key_is_legal(const grpc_slice& slice) { - if (GRPC_SLICE_LENGTH(slice) == 0) { - return GRPC_ERROR_CREATE("Metadata keys cannot be zero length"); +GPR_ATTRIBUTE_NOINLINE +absl::Status DoesNotConformTo(absl::string_view x, const char* err_desc) { + return absl::InternalError(absl::StrCat(err_desc, ": ", x, " (hex ", + absl::BytesToHexString(x), ")")); +} + +absl::Status ConformsTo(absl::string_view x, const BitSet<256>& legal_bits, + const char* err_desc) { + for (uint8_t c : x) { + if (!legal_bits.is_set(c)) { + return DoesNotConformTo(x, err_desc); + } } - if (GRPC_SLICE_LENGTH(slice) > UINT32_MAX) { - return GRPC_ERROR_CREATE("Metadata keys cannot be larger than UINT32_MAX"); + return absl::OkStatus(); +} +} // namespace + +absl::Status ValidateHeaderKeyIsLegal(absl::string_view key) { + if (key.empty()) { + return absl::InternalError("Metadata keys cannot be zero length"); } - if (GRPC_SLICE_START_PTR(slice)[0] == ':') { - return GRPC_ERROR_CREATE("Metadata keys cannot start with :"); + if (key.size() > UINT32_MAX) { + return absl::InternalError( + "Metadata keys cannot be larger than UINT32_MAX"); } - return conforms_to(slice, g_legal_header_key_bits, "Illegal header key"); + return ConformsTo(key, g_legal_header_key_bits, "Illegal header key"); +} + +} // namespace grpc_core + +static int error2int(grpc_error_handle error) { + int r = (error.ok()); + return r; +} + +grpc_error_handle grpc_validate_header_key_is_legal(const grpc_slice& slice) { + return grpc_core::ValidateHeaderKeyIsLegal( + grpc_core::StringViewFromSlice(slice)); } int grpc_header_key_is_legal(grpc_slice slice) { @@ -104,8 +104,9 @@ constexpr LegalHeaderNonBinValueBits g_legal_header_non_bin_value_bits; grpc_error_handle grpc_validate_header_nonbin_value_is_legal( const grpc_slice& slice) { - return conforms_to(slice, g_legal_header_non_bin_value_bits, - "Illegal header value"); + return grpc_core::ConformsTo(grpc_core::StringViewFromSlice(slice), + g_legal_header_non_bin_value_bits, + "Illegal header value"); } int grpc_header_nonbin_value_is_legal(grpc_slice slice) { diff --git a/src/core/lib/surface/validate_metadata.h b/src/core/lib/surface/validate_metadata.h index 3e6fdf482acf1..a6e99cabba9c4 100644 --- a/src/core/lib/surface/validate_metadata.h +++ b/src/core/lib/surface/validate_metadata.h @@ -25,11 +25,20 @@ #include +#include "absl/status/status.h" +#include "absl/strings/string_view.h" + #include #include #include "src/core/lib/iomgr/error.h" +namespace grpc_core { + +absl::Status ValidateHeaderKeyIsLegal(absl::string_view key); + +} + grpc_error_handle grpc_validate_header_key_is_legal(const grpc_slice& slice); grpc_error_handle grpc_validate_header_nonbin_value_is_legal( const grpc_slice& slice); diff --git a/src/core/lib/transport/metadata_batch.h b/src/core/lib/transport/metadata_batch.h index 7e95665b4ac89..50f148add2334 100644 --- a/src/core/lib/transport/metadata_batch.h +++ b/src/core/lib/transport/metadata_batch.h @@ -611,8 +611,9 @@ class ParseHelper { GPR_ATTRIBUTE_NOINLINE ParsedMetadata NotFound( absl::string_view key) { - return ParsedMetadata(Slice::FromCopiedString(key), - std::move(value_)); + return ParsedMetadata( + typename ParsedMetadata::FromSlicePair{}, + Slice::FromCopiedString(key), std::move(value_), transport_size_); } private: diff --git a/src/core/lib/transport/parsed_metadata.h b/src/core/lib/transport/parsed_metadata.h index 5ae9250c7d7d8..4d99c0ca22668 100644 --- a/src/core/lib/transport/parsed_metadata.h +++ b/src/core/lib/transport/parsed_metadata.h @@ -152,9 +152,14 @@ class ParsedMetadata { value_.slice = value.TakeCSlice(); } // Construct metadata from a string key, slice value pair. - ParsedMetadata(Slice key, Slice value) + // FromSlicePair() is used to adjust the overload set so that we don't + // inadvertently match against any of the previous overloads. + // TODO(ctiller): re-evaluate the overload functions here so and maybe + // introduce some factory functions? + struct FromSlicePair {}; + ParsedMetadata(FromSlicePair, Slice key, Slice value, uint32_t transport_size) : vtable_(ParsedMetadata::KeyValueVTable(key.as_string_view())), - transport_size_(static_cast(key.size() + value.size() + 32)) { + transport_size_(transport_size) { value_.pointer = new std::pair(std::move(key), std::move(value)); } @@ -188,14 +193,13 @@ class ParsedMetadata { // HTTP2 defined storage size of this metadatum. uint32_t transport_size() const { return transport_size_; } // Create a new parsed metadata with the same key but a different value. - ParsedMetadata WithNewValue(Slice value, + ParsedMetadata WithNewValue(Slice value, uint32_t value_wire_size, MetadataParseErrorFn on_error) const { ParsedMetadata result; result.vtable_ = vtable_; result.value_ = value_; result.transport_size_ = - TransportSize(static_cast(key().length()), - static_cast(value.length())); + TransportSize(static_cast(key().length()), value_wire_size); vtable_->with_new_value(&value, on_error, &result); return result; } diff --git a/test/core/transport/chttp2/BUILD b/test/core/transport/chttp2/BUILD index fb5d49a3482c2..88804c949035c 100644 --- a/test/core/transport/chttp2/BUILD +++ b/test/core/transport/chttp2/BUILD @@ -32,6 +32,18 @@ grpc_proto_fuzzer( ], ) +grpc_proto_fuzzer( + name = "hpack_sync_fuzzer", + srcs = ["hpack_sync_fuzzer.cc"], + corpus = "hpack_sync_corpus", + proto = "hpack_sync_fuzzer.proto", + tags = ["no_windows"], + deps = [ + "//:grpc", + "//test/core/util:grpc_test_util", + ], +) + grpc_proto_fuzzer( name = "flow_control_fuzzer", srcs = ["flow_control_fuzzer.cc"], @@ -47,6 +59,23 @@ grpc_proto_fuzzer( ], ) +grpc_fuzzer( + name = "hpack_parser_input_size_fuzzer", + srcs = ["hpack_parser_input_size_fuzzer.cc"], + corpus = "hpack_parser_input_size_corpus", + external_deps = [ + "absl/cleanup", + "absl/status:statusor", + "absl/status", + ], + tags = ["no_windows"], + deps = [ + "//:grpc", + "//test/core/util:grpc_test_util", + "//test/core/util:grpc_test_util_base", + ], +) + grpc_fuzzer( name = "decode_huff_fuzzer", srcs = ["decode_huff_fuzzer.cc"], diff --git a/test/core/transport/chttp2/MiXeD-CaSe.headers b/test/core/transport/chttp2/MiXeD-CaSe.headers new file mode 100644 index 0000000000000..8fb852588d9bc --- /dev/null +++ b/test/core/transport/chttp2/MiXeD-CaSe.headers @@ -0,0 +1 @@ +MiXeD-CaSe: should not parse diff --git a/test/core/transport/chttp2/bad-base64.headers b/test/core/transport/chttp2/bad-base64.headers new file mode 100644 index 0000000000000..bff6de9c268ca --- /dev/null +++ b/test/core/transport/chttp2/bad-base64.headers @@ -0,0 +1 @@ +a.b.c-bin: luckily for us, it's tuesday diff --git a/test/core/transport/chttp2/bad-te.headers b/test/core/transport/chttp2/bad-te.headers new file mode 100644 index 0000000000000..a66f0aa6c72f3 --- /dev/null +++ b/test/core/transport/chttp2/bad-te.headers @@ -0,0 +1 @@ +te: garbage diff --git a/test/core/transport/chttp2/bin_encoder_test.cc b/test/core/transport/chttp2/bin_encoder_test.cc index 83fe34a6d2dbf..22643e38fb142 100644 --- a/test/core/transport/chttp2/bin_encoder_test.cc +++ b/test/core/transport/chttp2/bin_encoder_test.cc @@ -70,7 +70,9 @@ static void expect_combined_equiv(const char* s, size_t len, int line) { grpc_slice input = grpc_slice_from_copied_buffer(s, len); grpc_slice base64 = grpc_chttp2_base64_encode(input); grpc_slice expect = grpc_chttp2_huffman_compress(base64); - grpc_slice got = grpc_chttp2_base64_encode_and_huffman_compress(input); + uint32_t wire_size; + grpc_slice got = + grpc_chttp2_base64_encode_and_huffman_compress(input, &wire_size); if (!grpc_slice_eq(expect, got)) { char* t = grpc_dump_slice(input, GPR_DUMP_HEX | GPR_DUMP_ASCII); char* e = grpc_dump_slice(expect, GPR_DUMP_HEX | GPR_DUMP_ASCII); diff --git a/test/core/transport/chttp2/hpack_encoder_test.cc b/test/core/transport/chttp2/hpack_encoder_test.cc index 671ca25c0307b..9153236f9d829 100644 --- a/test/core/transport/chttp2/hpack_encoder_test.cc +++ b/test/core/transport/chttp2/hpack_encoder_test.cc @@ -193,7 +193,7 @@ static void verify( bool is_eof, const char* expected, const std::vector>& header_fields) { const grpc_core::Slice merged(EncodeHeaderIntoBytes(is_eof, header_fields)); - const grpc_core::Slice expect(parse_hexstring(expected)); + const grpc_core::Slice expect(grpc_core::ParseHexstring(expected)); EXPECT_EQ(merged, expect); } @@ -343,6 +343,66 @@ TEST(HpackEncoderTest, TestContinuationHeaders) { delete g_compressor; } +TEST(HpackEncoderTest, EncodeBinaryAsBase64) { + grpc_core::MemoryAllocator memory_allocator = + grpc_core::MemoryAllocator(grpc_core::ResourceQuota::Default() + ->memory_quota() + ->CreateMemoryAllocator("test")); + auto arena = grpc_core::MakeScopedArena(1024, &memory_allocator); + grpc_metadata_batch b(arena.get()); + // Haiku by Bard + b.Append("grpc-trace-bin", + grpc_core::Slice::FromStaticString( + "Base64, a tool\nTo encode binary data into " + "text\nSo it can be shared."), + CrashOnAppendError); + grpc_transport_one_way_stats stats; + stats = {}; + grpc_slice_buffer output; + grpc_slice_buffer_init(&output); + grpc_core::HPackCompressor::EncodeHeaderOptions hopt = { + 0xdeadbeef, // stream_id + true, // is_eof + false, // use_true_binary_metadata + 150, // max_frame_size + &stats}; + grpc_core::HPackCompressor compressor; + compressor.EncodeHeaders(hopt, b, &output); + grpc_slice_buffer_destroy(&output); + + EXPECT_EQ(compressor.test_only_table_size(), 136); +} + +TEST(HpackEncoderTest, EncodeBinaryAsTrueBinary) { + grpc_core::MemoryAllocator memory_allocator = + grpc_core::MemoryAllocator(grpc_core::ResourceQuota::Default() + ->memory_quota() + ->CreateMemoryAllocator("test")); + auto arena = grpc_core::MakeScopedArena(1024, &memory_allocator); + grpc_metadata_batch b(arena.get()); + // Haiku by Bard + b.Append("grpc-trace-bin", + grpc_core::Slice::FromStaticString( + "Base64, a tool\nTo encode binary data into " + "text\nSo it can be shared."), + CrashOnAppendError); + grpc_transport_one_way_stats stats; + stats = {}; + grpc_slice_buffer output; + grpc_slice_buffer_init(&output); + grpc_core::HPackCompressor::EncodeHeaderOptions hopt = { + 0xdeadbeef, // stream_id + true, // is_eof + true, // use_true_binary_metadata + 150, // max_frame_size + &stats}; + grpc_core::HPackCompressor compressor; + compressor.EncodeHeaders(hopt, b, &output); + grpc_slice_buffer_destroy(&output); + + EXPECT_EQ(compressor.test_only_table_size(), 114); +} + int main(int argc, char** argv) { grpc::testing::TestEnvironment env(&argc, argv); ::testing::InitGoogleTest(&argc, argv); diff --git a/test/core/transport/chttp2/hpack_parser_corpus/crash-39214285ae59b7193aad114858056cca7c21a8e5 b/test/core/transport/chttp2/hpack_parser_corpus/crash-39214285ae59b7193aad114858056cca7c21a8e5 new file mode 100644 index 0000000000000..3490435504199 --- /dev/null +++ b/test/core/transport/chttp2/hpack_parser_corpus/crash-39214285ae59b7193aad114858056cca7c21a8e5 @@ -0,0 +1,12 @@ +frames { + max_metadata_length: 4096 + parse: "\244\244\020\007\360\244\017-bin\213c[)(-\'bin;\t!!?\244\037\333\360!\020\007\360{(-bin\360\t!\\\t!\345\037\351\033;?G\355[((!!\\\360" +} +frames { + max_metadata_length: 4096 + parse: "*\244\020\007\360\244\017-bin\203c\035\037\000\'[\360i(bn-!?\244\037\333\360!(!\\\360\tc" + parse: "\244\244\020\007\360\244\017-bin\213c[)(-\'bin\t;!!?\244\037\333\360!\020\007\360{(-bin\360\t!\\\t!\345\037\351\033;?G\355[((!!\\\360" + parse: "\244\244\020\007\360\244\017-bin\213c[)(-\'bin\t;!!?\244\037\333\360!\020\007\360{(-bin\360\t!\\\t!\345\037\351\033;?G\355[((!!\\\360" + parse: "\244\244\020\007\360\244\017-bin\213c[)(-\'bin\t;!!?\244\037\333\360!\020\007\360{(-bin\360\t!\\\t!\345\037\351\033;?G\355[((!!\\\360" + absolute_max_metadata_length: 4096 +} diff --git a/test/core/transport/chttp2/hpack_parser_corpus/crash-7f5f186fa8ac3950346da51bce3d76d0437d3b20 b/test/core/transport/chttp2/hpack_parser_corpus/crash-7f5f186fa8ac3950346da51bce3d76d0437d3b20 new file mode 100644 index 0000000000000..e9a303b57c34c --- /dev/null +++ b/test/core/transport/chttp2/hpack_parser_corpus/crash-7f5f186fa8ac3950346da51bce3d76d0437d3b20 @@ -0,0 +1,38 @@ +frames { + end_of_headers: true +} +frames { + end_of_headers: true + end_of_stream: true + stop_buffering_after_segments: 4096 + max_metadata_length: 16252928 + parse: "iiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiii" + parse: "\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272" + parse: "\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272" + parse: "\244\244\020\007\360\244\017-bin\213c*[)\244(\244-\017\333\360\'\360b\203cin\t!!" + absolute_max_metadata_length: 16252928 +} +frames { + end_of_headers: true + end_of_stream: true + max_metadata_length: -4093 + parse: "\261\261\261\261\261\261\261" + parse: "D\005:path" + parse: "\261\261\261\261\261\261\261" + parse: "\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272" + parse: "\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272" + absolute_max_metadata_length: 4096 +} +frames { + parse: "\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272" +} +frames { + end_of_headers: true + end_of_stream: true + stop_buffering_after_segments: 4096 + max_metadata_length: 4096 + parse: "iiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiii" + parse: "\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272" + parse: "\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272\272" + parse: "\244\244\020\007\360\244\017-bin\213\377\377\377\377\377\377\377\377\377\377\377\377\377\377\377\377\377\377\377\377c*[)\244(\244-\017\333\360\'\360b\203cin\t!!" +} diff --git a/test/core/transport/chttp2/hpack_parser_input_size_corpus/empty b/test/core/transport/chttp2/hpack_parser_input_size_corpus/empty new file mode 100644 index 0000000000000..8b137891791fe --- /dev/null +++ b/test/core/transport/chttp2/hpack_parser_input_size_corpus/empty @@ -0,0 +1 @@ + diff --git a/test/core/transport/chttp2/hpack_parser_input_size_fuzzer.cc b/test/core/transport/chttp2/hpack_parser_input_size_fuzzer.cc new file mode 100644 index 0000000000000..a67529126b8ed --- /dev/null +++ b/test/core/transport/chttp2/hpack_parser_input_size_fuzzer.cc @@ -0,0 +1,151 @@ +// Copyright 2023 gRPC authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// For all inputs, ensure parsing one byte at a time produces the same result as +// parsing the entire input at once. + +#include +#include +#include + +#include +#include + +#include "absl/cleanup/cleanup.h" +#include "absl/status/status.h" +#include "absl/status/statusor.h" +#include "absl/strings/str_cat.h" + +#include +#include +#include +#include + +#include "src/core/ext/transport/chttp2/transport/hpack_parser.h" +#include "src/core/lib/gprpp/ref_counted_ptr.h" +#include "src/core/lib/gprpp/status_helper.h" +#include "src/core/lib/iomgr/error.h" +#include "src/core/lib/iomgr/exec_ctx.h" +#include "src/core/lib/resource_quota/arena.h" +#include "src/core/lib/resource_quota/memory_quota.h" +#include "src/core/lib/resource_quota/resource_quota.h" +#include "src/core/lib/slice/slice.h" +#include "src/core/lib/transport/metadata_batch.h" +#include "test/core/util/slice_splitter.h" + +bool squelch = true; +bool leak_check = true; + +namespace grpc_core { +namespace { + +class TestEncoder { + public: + std::string result() { return out_; } + + void Encode(const Slice& key, const Slice& value) { + out_.append( + absl::StrCat(key.as_string_view(), ": ", value.as_string_view(), "\n")); + } + + template + void Encode(T, const V& v) { + out_.append(absl::StrCat(T::key(), ": ", T::DisplayValue(v), "\n")); + } + + private: + std::string out_; +}; + +bool IsStreamError(const absl::Status& status) { + intptr_t stream_id; + return grpc_error_get_int(status, StatusIntProperty::kStreamId, &stream_id); +} + +absl::StatusOr TestVector(grpc_slice_split_mode mode, + Slice input) { + MemoryAllocator memory_allocator = MemoryAllocator( + ResourceQuota::Default()->memory_quota()->CreateMemoryAllocator("test")); + auto arena = MakeScopedArena(1024, &memory_allocator); + ExecCtx exec_ctx; + grpc_slice* slices; + size_t nslices; + size_t i; + + grpc_metadata_batch b(arena.get()); + + HPackParser parser; + parser.BeginFrame( + &b, 1024, 1024, HPackParser::Boundary::None, HPackParser::Priority::None, + HPackParser::LogInfo{1, HPackParser::LogInfo::kHeaders, false}); + + grpc_split_slices(mode, const_cast(&input.c_slice()), 1, &slices, + &nslices); + auto cleanup_slices = absl::MakeCleanup([slices, nslices] { + for (size_t i = 0; i < nslices; i++) { + grpc_slice_unref(slices[i]); + } + gpr_free(slices); + }); + + absl::Status found_err; + for (i = 0; i < nslices; i++) { + ExecCtx exec_ctx; + auto err = parser.Parse(slices[i], i == nslices - 1); + if (!err.ok()) { + if (!IsStreamError(err)) return err; + if (found_err.ok()) found_err = err; + } + } + if (!found_err.ok()) return found_err; + + TestEncoder encoder; + b.Encode(&encoder); + return encoder.result(); +} + +std::string Stringify(absl::StatusOr result) { + if (result.ok()) { + return absl::StrCat("OK\n", result.value()); + } else { + intptr_t stream_id; + bool has_stream = grpc_error_get_int( + result.status(), StatusIntProperty::kStreamId, &stream_id); + return absl::StrCat( + has_stream ? "STREAM" : "CONNECTION", " ERROR: ", + result.status().ToString(absl::StatusToStringMode::kWithNoExtraData)); + } +} + +} // namespace +} // namespace grpc_core + +extern gpr_timespec (*gpr_now_impl)(gpr_clock_type clock_type); + +extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) { + gpr_now_impl = [](gpr_clock_type clock_type) { + return gpr_timespec{10, 0, clock_type}; + }; + auto slice = grpc_core::Slice::FromCopiedBuffer(data, size); + auto full = grpc_core::Stringify( + grpc_core::TestVector(GRPC_SLICE_SPLIT_IDENTITY, slice.Ref())); + auto one_byte = grpc_core::Stringify( + grpc_core::TestVector(GRPC_SLICE_SPLIT_ONE_BYTE, slice.Ref())); + if (full != one_byte) { + fprintf(stderr, "MISMATCHED RESULTS\nFULL SLICE: %s\nONE BYTE: %s\n", + full.c_str(), one_byte.c_str()); + abort(); + } + return 0; +} diff --git a/test/core/transport/chttp2/hpack_parser_table_test.cc b/test/core/transport/chttp2/hpack_parser_table_test.cc index cf1b24cf40acb..7b1dbd02b228e 100644 --- a/test/core/transport/chttp2/hpack_parser_table_test.cc +++ b/test/core/transport/chttp2/hpack_parser_table_test.cc @@ -37,7 +37,7 @@ void AssertIndex(const HPackTable* tbl, uint32_t idx, const char* key, const char* value) { const auto* md = tbl->Lookup(idx); ASSERT_NE(md, nullptr); - EXPECT_EQ(md->DebugString(), absl::StrCat(key, ": ", value)); + EXPECT_EQ(md->md.DebugString(), absl::StrCat(key, ": ", value)); } } // namespace @@ -119,8 +119,12 @@ TEST(HpackParserTableTest, ManyAdditions) { std::string value = absl::StrCat("VALUE.", i); auto key_slice = Slice::FromCopiedString(key); auto value_slice = Slice::FromCopiedString(value); - auto memento = - HPackTable::Memento(std::move(key_slice), std::move(value_slice)); + auto memento = HPackTable::Memento{ + ParsedMetadata( + ParsedMetadata::FromSlicePair{}, + std::move(key_slice), std::move(value_slice), + key.length() + value.length() + 32), + absl::OkStatus()}; auto add_err = tbl.Add(std::move(memento)); ASSERT_EQ(add_err, absl::OkStatus()); AssertIndex(&tbl, 1 + hpack_constants::kLastStaticEntry, key.c_str(), diff --git a/test/core/transport/chttp2/hpack_parser_test.cc b/test/core/transport/chttp2/hpack_parser_test.cc index 453036d12e9c4..f8ebc4d3f609c 100644 --- a/test/core/transport/chttp2/hpack_parser_test.cc +++ b/test/core/transport/chttp2/hpack_parser_test.cc @@ -20,53 +20,64 @@ #include -#include #include #include +#include "absl/cleanup/cleanup.h" #include "absl/status/status.h" +#include "absl/status/statusor.h" #include "absl/strings/str_cat.h" -#include "absl/strings/str_format.h" +#include "absl/strings/string_view.h" #include "absl/types/optional.h" +#include "gmock/gmock.h" #include "gtest/gtest.h" #include #include #include +#include #include -#include "src/core/lib/gprpp/crash.h" #include "src/core/lib/gprpp/ref_counted_ptr.h" #include "src/core/lib/gprpp/status_helper.h" +#include "src/core/lib/gprpp/time.h" #include "src/core/lib/iomgr/exec_ctx.h" #include "src/core/lib/resource_quota/arena.h" #include "src/core/lib/resource_quota/memory_quota.h" #include "src/core/lib/resource_quota/resource_quota.h" #include "src/core/lib/slice/slice.h" +#include "src/core/lib/transport/error_utils.h" #include "test/core/util/parse_hexstring.h" #include "test/core/util/slice_splitter.h" #include "test/core/util/test_config.h" +namespace grpc_core { +namespace { + +const uint32_t kFailureIsConnectionError = 1; +const uint32_t kWithPriority = 2; +const uint32_t kEndOfStream = 4; +const uint32_t kEndOfHeaders = 8; + struct TestInput { - const char* input; - const char* expected_parse; + absl::string_view input; + absl::StatusOr expected_parse; + uint32_t flags; }; struct Test { absl::optional table_size; + absl::optional max_metadata_size; std::vector inputs; }; class ParseTest : public ::testing::TestWithParam { public: - ParseTest() { - grpc_init(); - parser_ = std::make_unique(); - } + ParseTest() { grpc_init(); } ~ParseTest() override { { - grpc_core::ExecCtx exec_ctx; + ExecCtx exec_ctx; parser_.reset(); } @@ -74,6 +85,7 @@ class ParseTest : public ::testing::TestWithParam { } void SetUp() override { + parser_ = std::make_unique(); if (GetParam().table_size.has_value()) { parser_->hpack_table()->SetMaxBytes(GetParam().table_size.value()); EXPECT_EQ(parser_->hpack_table()->SetCurrentTableSize( @@ -82,15 +94,21 @@ class ParseTest : public ::testing::TestWithParam { } } - void TestVector(grpc_slice_split_mode mode, const char* hexstring, - std::string expect) { - grpc_core::MemoryAllocator memory_allocator = - grpc_core::MemoryAllocator(grpc_core::ResourceQuota::Default() - ->memory_quota() - ->CreateMemoryAllocator("test")); - auto arena = grpc_core::MakeScopedArena(1024, &memory_allocator); - grpc_core::ExecCtx exec_ctx; - grpc_slice input = parse_hexstring(hexstring); + static bool IsStreamError(const absl::Status& status) { + intptr_t stream_id; + return grpc_error_get_int(status, StatusIntProperty::kStreamId, &stream_id); + } + + void TestVector(grpc_slice_split_mode mode, + absl::optional max_metadata_size, + absl::string_view hexstring, + absl::StatusOr expect, uint32_t flags) { + MemoryAllocator memory_allocator = MemoryAllocator( + ResourceQuota::Default()->memory_quota()->CreateMemoryAllocator( + "test")); + auto arena = MakeScopedArena(1024, &memory_allocator); + ExecCtx exec_ctx; + auto input = ParseHexstring(hexstring); grpc_slice* slices; size_t nslices; size_t i; @@ -98,32 +116,59 @@ class ParseTest : public ::testing::TestWithParam { grpc_metadata_batch b(arena.get()); parser_->BeginFrame( - &b, 4096, 4096, grpc_core::HPackParser::Boundary::None, - grpc_core::HPackParser::Priority::None, - grpc_core::HPackParser::LogInfo{ - 1, grpc_core::HPackParser::LogInfo::kHeaders, false}); + &b, max_metadata_size.value_or(4096), max_metadata_size.value_or(4096), + (flags & kEndOfStream) + ? HPackParser::Boundary::EndOfStream + : ((flags & kEndOfHeaders) ? HPackParser::Boundary::EndOfHeaders + : HPackParser::Boundary::None), + flags & kWithPriority ? HPackParser::Priority::Included + : HPackParser::Priority::None, + HPackParser::LogInfo{1, HPackParser::LogInfo::kHeaders, false}); - grpc_split_slices(mode, &input, 1, &slices, &nslices); - grpc_slice_unref(input); + grpc_split_slices(mode, const_cast(&input.c_slice()), 1, + &slices, &nslices); + auto cleanup_slices = absl::MakeCleanup([slices, nslices] { + for (size_t i = 0; i < nslices; i++) { + grpc_slice_unref(slices[i]); + } + gpr_free(slices); + }); + bool saw_error = false; for (i = 0; i < nslices; i++) { - grpc_core::ExecCtx exec_ctx; + ExecCtx exec_ctx; auto err = parser_->Parse(slices[i], i == nslices - 1); - if (!err.ok()) { - grpc_core::Crash( - absl::StrFormat("Unexpected parse error: %s", - grpc_core::StatusToString(err).c_str())); + if (!err.ok() && (flags & kFailureIsConnectionError) == 0) { + EXPECT_TRUE(IsStreamError(err)) << err; + } + if (!saw_error && !err.ok()) { + // one byte at a time mode might fail with a stream error early + if (mode == GRPC_SLICE_SPLIT_ONE_BYTE && + (flags & kFailureIsConnectionError) && IsStreamError(err)) { + continue; + } + grpc_status_code code; + std::string message; + grpc_error_get_status(err, Timestamp::InfFuture(), &code, &message, + nullptr, nullptr); + EXPECT_EQ(code, static_cast(expect.status().code())) + << err; + EXPECT_THAT(message, ::testing::HasSubstr(expect.status().message())) + << err; + saw_error = true; + if (flags & kFailureIsConnectionError) return; } } - for (i = 0; i < nslices; i++) { - grpc_slice_unref(slices[i]); + if (!saw_error) { + EXPECT_TRUE(expect.ok()) << expect.status(); } - gpr_free(slices); - TestEncoder encoder; - b.Encode(&encoder); - EXPECT_EQ(encoder.result(), expect); + if (expect.ok()) { + TestEncoder encoder; + b.Encode(&encoder); + EXPECT_EQ(encoder.result(), *expect) << "Input: " << hexstring; + } } private: @@ -131,7 +176,7 @@ class ParseTest : public ::testing::TestWithParam { public: std::string result() { return out_; } - void Encode(const grpc_core::Slice& key, const grpc_core::Slice& value) { + void Encode(const Slice& key, const Slice& value) { out_.append(absl::StrCat(key.as_string_view(), ": ", value.as_string_view(), "\n")); } @@ -146,41 +191,45 @@ class ParseTest : public ::testing::TestWithParam { std::string out_; }; - std::unique_ptr parser_; + std::unique_ptr parser_; }; TEST_P(ParseTest, WholeSlices) { for (const auto& input : GetParam().inputs) { - TestVector(GRPC_SLICE_SPLIT_MERGE_ALL, input.input, input.expected_parse); + TestVector(GRPC_SLICE_SPLIT_MERGE_ALL, GetParam().max_metadata_size, + input.input, input.expected_parse, input.flags); } } TEST_P(ParseTest, OneByteAtATime) { for (const auto& input : GetParam().inputs) { - TestVector(GRPC_SLICE_SPLIT_ONE_BYTE, input.input, input.expected_parse); + TestVector(GRPC_SLICE_SPLIT_ONE_BYTE, GetParam().max_metadata_size, + input.input, input.expected_parse, input.flags); } } INSTANTIATE_TEST_SUITE_P( ParseTest, ParseTest, ::testing::Values( - Test{ - {}, - { - // D.2.1 - {"400a 6375 7374 6f6d 2d6b 6579 0d63 7573" - "746f 6d2d 6865 6164 6572", - "custom-key: custom-header\n"}, - // D.2.2 - {"040c 2f73 616d 706c 652f 7061 7468", ":path: /sample/path\n"}, - // D.2.3 - {"1008 7061 7373 776f 7264 0673 6563 7265" - "74", - "password: secret\n"}, - // D.2.4 - {"82", ":method: GET\n"}, - }}, Test{{}, + {}, + { + // D.2.1 + {"400a 6375 7374 6f6d 2d6b 6579 0d63 7573" + "746f 6d2d 6865 6164 6572", + "custom-key: custom-header\n", 0}, + // D.2.2 + {"040c 2f73 616d 706c 652f 7061 7468", ":path: /sample/path\n", + 0}, + // D.2.3 + {"1008 7061 7373 776f 7264 0673 6563 7265" + "74", + "password: secret\n", 0}, + // D.2.4 + {"82", ":method: GET\n", 0}, + }}, + Test{{}, + {}, { // D.3.1 {"8286 8441 0f77 7777 2e65 7861 6d70 6c65" @@ -188,14 +237,16 @@ INSTANTIATE_TEST_SUITE_P( ":path: /\n" ":authority: www.example.com\n" ":method: GET\n" - ":scheme: http\n"}, + ":scheme: http\n", + 0}, // D.3.2 {"8286 84be 5808 6e6f 2d63 6163 6865", ":path: /\n" ":authority: www.example.com\n" ":method: GET\n" ":scheme: http\n" - "cache-control: no-cache\n"}, + "cache-control: no-cache\n", + 0}, // D.3.3 {"8287 85bf 400a 6375 7374 6f6d 2d6b 6579" "0c63 7573 746f 6d2d 7661 6c75 65", @@ -203,9 +254,11 @@ INSTANTIATE_TEST_SUITE_P( ":authority: www.example.com\n" ":method: GET\n" ":scheme: https\n" - "custom-key: custom-value\n"}, + "custom-key: custom-value\n", + 0}, }}, Test{{}, + {}, { // D.4.1 {"8286 8441 8cf1 e3c2 e5f2 3a6b a0ab 90f4" @@ -213,14 +266,16 @@ INSTANTIATE_TEST_SUITE_P( ":path: /\n" ":authority: www.example.com\n" ":method: GET\n" - ":scheme: http\n"}, + ":scheme: http\n", + 0}, // D.4.2 {"8286 84be 5886 a8eb 1064 9cbf", ":path: /\n" ":authority: www.example.com\n" ":method: GET\n" ":scheme: http\n" - "cache-control: no-cache\n"}, + "cache-control: no-cache\n", + 0}, // D.4.3 {"8287 85bf 4088 25a8 49e9 5ba9 7d7f 8925" "a849 e95b b8e8 b4bf", @@ -228,9 +283,11 @@ INSTANTIATE_TEST_SUITE_P( ":authority: www.example.com\n" ":method: GET\n" ":scheme: https\n" - "custom-key: custom-value\n"}, + "custom-key: custom-value\n", + 0}, }}, Test{{256}, + {}, { // D.5.1 {"4803 3330 3258 0770 7269 7661 7465 611d" @@ -241,13 +298,15 @@ INSTANTIATE_TEST_SUITE_P( ":status: 302\n" "cache-control: private\n" "date: Mon, 21 Oct 2013 20:13:21 GMT\n" - "location: https://www.example.com\n"}, + "location: https://www.example.com\n", + 0}, // D.5.2 {"4803 3330 37c1 c0bf", ":status: 307\n" "cache-control: private\n" "date: Mon, 21 Oct 2013 20:13:21 GMT\n" - "location: https://www.example.com\n"}, + "location: https://www.example.com\n", + 0}, // D.5.3 {"88c1 611d 4d6f 6e2c 2032 3120 4f63 7420" "3230 3133 2032 303a 3133 3a32 3220 474d" @@ -262,9 +321,11 @@ INSTANTIATE_TEST_SUITE_P( "location: https://www.example.com\n" "content-encoding: gzip\n" "set-cookie: foo=ASDJKHQKBZXOQWEOPIUAXQWEOIU; max-age=3600; " - "version=1\n"}, + "version=1\n", + 0}, }}, Test{{256}, + {}, { // D.6.1 {"4882 6402 5885 aec3 771a 4b61 96d0 7abe" @@ -274,13 +335,15 @@ INSTANTIATE_TEST_SUITE_P( ":status: 302\n" "cache-control: private\n" "date: Mon, 21 Oct 2013 20:13:21 GMT\n" - "location: https://www.example.com\n"}, + "location: https://www.example.com\n", + 0}, // D.6.2 {"4883 640e ffc1 c0bf", ":status: 307\n" "cache-control: private\n" "date: Mon, 21 Oct 2013 20:13:21 GMT\n" - "location: https://www.example.com\n"}, + "location: https://www.example.com\n", + 0}, // D.6.3 {"88c1 6196 d07a be94 1054 d444 a820 0595" "040b 8166 e084 a62d 1bff c05a 839b d9ab" @@ -293,18 +356,356 @@ INSTANTIATE_TEST_SUITE_P( "location: https://www.example.com\n" "content-encoding: gzip\n" "set-cookie: foo=ASDJKHQKBZXOQWEOPIUAXQWEOIU; max-age=3600; " - "version=1\n"}, + "version=1\n", + 0}, }}, Test{{}, + {1024}, + {{"3fc43fc4", absl::InternalError("Attempt to make hpack table"), + kFailureIsConnectionError}}}, + Test{{}, + {}, + {{"3ba4a41007f0a40f2d62696e8b632a5b29a40fa4a4281007f0", + absl::InternalError("Invalid HPACK index received"), + kFailureIsConnectionError}}}, + Test{{}, + {}, + {{"2aa41007f0a40f2d62696e8163a41f1f00275bf0692862a4dbf0f00963", + absl::InternalError( + "More than two max table size changes in a single frame"), + kFailureIsConnectionError}}}, + Test{{}, + {}, + {{"2aa41007f0a40f2d62696e8363271f00275bf06928626e2d213fa40fdbf0212" + "8215cf00963", + absl::InternalError("illegal base64 encoding"), 0}}}, + Test{{}, + {}, + {{"a4a41007f0a40f2d62696e8b635b29282d2762696e3b0921213fa41fdbf0211" + "007f07b282d62696ef009215c0921e51fe91b3b3f47ed5b282821215cf0", + absl::InternalError( + "More than two max table size changes in a single frame"), + kFailureIsConnectionError}}}, + Test{ + {}, + {}, + {{"696969696969696969696969696969696969696969696969696969696969696" + "969696969696969696969696969696969696969696969696969696969696969" + "6969696969696969696969696969bababababababababababababababababab" + "abababababababababababababababababababababababababababababababa" + "bababababababababababababababababababababababababababababababab" + "abababababaa4a41007f0a40f2d62696e8bffffffffffffffffffffffffffff" + "ffffffffffff632a5b29a428a42d0fdbf027f0628363696e092121", + absl::InternalError("integer overflow in hpack integer decoding"), + kEndOfHeaders | kFailureIsConnectionError}}}, + Test{{}, + {}, + {{"0e 00 00 df", + absl::InternalError( + "Error parsing ':status' metadata: error=not an integer"), + 0}}}, + Test{{}, + {}, { // Binary metadata: created using: // tools/codegen/core/gen_header_frame.py - // --compression inc --no_framing --hex + // --compression inc --no_framing --output hexstr // < test/core/transport/chttp2/binary-metadata.headers {"40 09 61 2e 62 2e 63 2d 62 69 6e 0c 62 32 31 6e 4d 6a 41 79 " "4d 51 3d 3d", - "a.b.c-bin: omg2021\n"}, - }})); + "a.b.c-bin: omg2021\n", 0}, + }}, + Test{{}, + {}, + {// Binary metadata: created using: + // tools/codegen/core/gen_header_frame.py + // --compression inc --no_framing --output hexstr + // < test/core/transport/chttp2/bad-base64.headers + {"4009612e622e632d62696e1c6c75636b696c7920666f722075732c206974" + "27732074756573646179", + absl::InternalError("Error parsing 'a.b.c-bin' metadata: " + "error=illegal base64 encoding"), + 0}, + {"be", + absl::InternalError("Error parsing 'a.b.c-bin' metadata: " + "error=illegal base64 encoding"), + 0}}}, + Test{{}, + {}, + {// created using: + // tools/codegen/core/gen_header_frame.py + // --compression inc --no_framing --output hexstr + // < test/core/transport/chttp2/bad-te.headers + {"400274650767617262616765", + absl::InternalError("Error parsing 'te' metadata"), 0}, + {"be", absl::InternalError("Error parsing 'te' metadata"), 0}}}, + Test{{}, + 128, + { + {// Generated with: tools/codegen/core/gen_header_frame.py + // --compression inc --output hexstr --no_framing < + // test/core/transport/chttp2/large-metadata.headers + "40096164616c64726964610a6272616e64796275636b40086164616c6772" + "696d04746f6f6b4008616d6172616e74680a6272616e64796275636b4008" + "616e67656c6963610762616767696e73", + absl::ResourceExhaustedError( + "received metadata size exceeds hard limit"), + 0}, + // Should be able to look up the added elements individually + // (do not corrupt the hpack table test!) + {"be", "angelica: baggins\n", 0}, + {"bf", "amaranth: brandybuck\n", 0}, + {"c0", "adalgrim: took\n", 0}, + {"c1", "adaldrida: brandybuck\n", 0}, + // But not as a whole - that exceeds metadata limits for one + // request again + {"bebfc0c1", + absl::ResourceExhaustedError( + "received metadata size exceeds hard limit"), + 0}, + }}, + Test{ + {}, + {}, + {{"be", absl::InternalError("Invalid HPACK index received"), + kFailureIsConnectionError}}, + }, + Test{ + {}, + {}, + {{"80", absl::InternalError("Illegal hpack op code"), + kFailureIsConnectionError}}, + }, + Test{ + {}, + {}, + {{"29", "", kFailureIsConnectionError}}, + }, + Test{ + {}, + {}, + {{"", "", kWithPriority}}, + }, + Test{ + {}, + {}, + {{"f5", absl::InternalError("Invalid HPACK index received"), + kFailureIsConnectionError}}, + }, + Test{ + {}, + {}, + {{"0f", "", 0}}, + }, + Test{ + {}, + {}, + {{"7f", "", 0}}, + }, + Test{ + {}, + {}, + {{"1bffffff7c1b", "", 0}}, + }, + Test{ + {}, + {}, + {{"ffffffffff00ff", + absl::InternalError("Invalid HPACK index received"), + kFailureIsConnectionError}}, + }, + Test{ + {}, + {}, + {{"ff8d8d8d8d8d8d8d8d8d8d8d8d8d8d8d8d8d8d8d8d8d8d8d8d8d8d8d8d8d8d8" + "d8d8d8d8d8d8d8d8d8d8d8d8d8d8d8d8d8d8d8d8d8d8d8d8d8d8d8d8d8d8d8d" + "8d8d8d8d8d8d8d8d8d8d8d8d8d8d8d8d8d8d8d8d8d8d8d8d8d8d8d8d8d8d8d8" + "d8d8d8d8d8d8d8d", + absl::InternalError("integer overflow in hpack integer decoding"), + kFailureIsConnectionError}}}, + Test{ + {}, + {9}, + {{"3f6672616d6573207ba2020656e645f6f665f686561646572733a2074727565a" + "2020656e645f6f665f73747265616d3a2074727565a202073746f705f6275666" + "66572696e675f61667465725f7365676d656e74733a2039a202070617273653a" + "20225c3030305c3030305c3030305c3030305c3030305c3030305c3030305c30" + "30305c3030305c3030305c3030305c3030305c3030305c3030305c3030305c30" + "30305c3030305c3030305c3030305c3030305c3030305c3030305c3030305c", + absl::ResourceExhaustedError( + "received metadata size exceeds hard limit"), + kWithPriority}}}, + Test{{}, + {}, + {{"52046772706300073a737461747573033230300e7f", + ":status: 200\naccept-ranges: grpc\n", 0}}}, + Test{{}, + {}, + {{"a4a41007f0a40f2d62696e8beda42d5b63272129a410626907", + absl::InternalError("illegal base64 encoding"), 0}}}, + Test{ + // haiku segment: 149bytes*2, a:a segment: 34 bytes + // So we arrange for one less than the total so we force a hpack + // table overflow + {149 * 2 + 34 - 1}, + {}, + { + {// Generated with: tools/codegen/core/gen_header_frame.py + // --compression inc --output hexstr --no_framing < + // test/core/transport/chttp2/long-base64.headers + "4005782d62696e70516d467a5a545930494756755932396b6157356e4f67" + "704a644342305957746c6379426961573568636e6b675a47463059534268" + "626d5167625746725a584d6761585167644756346443344b56584e6c5a6e5" + "67349475a766369427a644739796157356e49475a706247567a4c673d3d", + // Haiku by Bard. + "x-bin: Base64 encoding:\nIt takes binary data and makes it " + "text.\nUseful for storing files.\n", + 0}, + // Should go into the hpack table (x-bin: ... is 149 bytes long + // by hpack rules) + {"be", + "x-bin: Base64 encoding:\nIt takes binary data and " + "makes it text.\nUseful for storing files.\n", + 0}, + // Add another copy + {"4005782d62696e70516d467a5a545930494756755932396b6157356e4f67" + "704a644342305957746c6379426961573568636e6b675a47463059534268" + "626d5167625746725a584d6761585167644756346443344b56584e6c5a6e5" + "67349475a766369427a644739796157356e49475a706247567a4c673d3d", + "x-bin: Base64 encoding:\nIt takes binary data and makes it " + "text.\nUseful for storing files.\n", + 0}, + // 149*2 == 298, so we should have two copies in the hpack table + {"bebf", + "x-bin: Base64 encoding:\nIt takes binary data and makes it " + "text.\nUseful for storing files.\n" + "x-bin: Base64 encoding:\nIt takes binary data and makes it " + "text.\nUseful for storing files.\n", + 0}, + // Add some very short headers (should push the first long thing + // out) + // Generated with: tools/codegen/core/gen_header_frame.py + // --compression inc --output hexstr --no_framing < + // test/core/transport/chttp2/short.headers + {"4001610161", "a: a\n", 0}, + // First two entries should be what was just pushed and then one + // long entry + {"bebf", + "a: a\nx-bin: Base64 encoding:\nIt takes binary data and " + "makes " + "it text.\nUseful for storing files.\n", + 0}, + // Third entry should be unprobable (it's no longer in the + // table!) + {"c0", absl::InternalError("Invalid HPACK index received"), + kFailureIsConnectionError}, + }}, + Test{ + // haiku segment: 149bytes*2, a:a segment: 34 bytes + // So we arrange for one less than the total so we force a hpack + // table overflow + {149 * 2 + 34 - 1}, + {}, + { + {// Generated with: tools/codegen/core/gen_header_frame.py + // --compression inc --output hexstr --no_framing --huff < + // test/core/transport/chttp2/long-base64.headers + "4005782d62696edbd94e1f7fbbf983262e36f313fd47c9bab54d5e592f5d0" + "73e49a09eae987c9b9c95759bf7161073dd7678e9d9347cb0d9fbf9a261fe" + "6c9a4c5c5a92f359b8fe69a3f6ae28c98bf7b90d77dc989ff43e4dd59317e" + "d71e2e3ef3cd041", + // Haiku by Bard. + "x-bin: Base64 encoding:\nIt takes binary data and makes it " + "text.\nUseful for storing files.\n", + 0}, + // Should go into the hpack table (x-bin: ... is 149 bytes long + // by hpack rules) + {"be", + "x-bin: Base64 encoding:\nIt takes binary data and " + "makes it text.\nUseful for storing files.\n", + 0}, + // Add another copy + {"4005782d62696edbd94e1f7fbbf983262e36f313fd47c9bab54d5e592f5d0" + "73e49a09eae987c9b9c95759bf7161073dd7678e9d9347cb0d9fbf9a261fe" + "6c9a4c5c5a92f359b8fe69a3f6ae28c98bf7b90d77dc989ff43e4dd59317e" + "d71e2e3ef3cd041", + "x-bin: Base64 encoding:\nIt takes binary data and makes it " + "text.\nUseful for storing files.\n", + 0}, + // 149*2 == 298, so we should have two copies in the hpack table + {"bebf", + "x-bin: Base64 encoding:\nIt takes binary data and makes it " + "text.\nUseful for storing files.\n" + "x-bin: Base64 encoding:\nIt takes binary data and makes it " + "text.\nUseful for storing files.\n", + 0}, + // Add some very short headers (should push the first long thing + // out) + // Generated with: tools/codegen/core/gen_header_frame.py + // --compression inc --output hexstr --no_framing < + // test/core/transport/chttp2/short.headers + {"4001610161", "a: a\n", 0}, + // First two entries should be what was just pushed and then one + // long entry + {"bebf", + "a: a\nx-bin: Base64 encoding:\nIt takes binary data and " + "makes " + "it text.\nUseful for storing files.\n", + 0}, + // Third entry should be unprobable (it's no longer in the + // table!) + {"c0", absl::InternalError("Invalid HPACK index received"), + kFailureIsConnectionError}, + }}, + Test{{}, {}, {{"7a", "", 0}}}, + Test{{}, + {}, + {{"60", + absl::InternalError("Incomplete header at the end of a " + "header/continuation sequence"), + kEndOfStream | kFailureIsConnectionError}}}, + Test{{}, + {}, + {{"89", ":status: 204\n", 0}, + {"89", ":status: 204\n", 0}, + {"393939393939393939393939393939393939393939", + absl::InternalError( + "More than two max table size changes in a single frame"), + kFailureIsConnectionError}}}, + Test{{}, + {}, + {{"4005782d62696edbd94e1f7fbbf983267e36a313fd47c9bab54d5e592f5d", + "", 0}}}, + Test{{}, {}, {{"72656672657368", "", 0}}}, + Test{{}, {}, {{"66e6645f74", "", 0}, {"66645f74", "", 0}}}, + Test{ + {}, + {}, + {{// Generated with: tools/codegen/core/gen_header_frame.py + // --compression inc --output hexstr --no_framing < + // test/core/transport/chttp2/MiXeD-CaSe.headers + "400a4d695865442d436153651073686f756c64206e6f74207061727365", + absl::InternalError("Illegal header key: MiXeD-CaSe"), 0}, + {// Looking up with hpack indices should work, but also return + // error + "be", absl::InternalError("Illegal header key: MiXeD-CaSe"), 0}}}, + Test{ + {}, + {}, + {{"ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", + absl::InternalError("integer overflow in hpack integer decoding"), + kFailureIsConnectionError}}}, + Test{{}, + {}, + {{"dadadadadadadadadadadadadadadadadadadadadadadadadadadadadadadad" + "adadadadadadadadadadadadadadadadadadadadadadadadadadadadadadada" + "dadadadadadadadadadadadadadadadadadadadadadadadadadadadadadadad" + "adadadadadadadadadadadadadadadadadadada", + absl::InternalError("Invalid HPACK index received"), + kWithPriority | kFailureIsConnectionError}}})); + +} // namespace +} // namespace grpc_core int main(int argc, char** argv) { grpc::testing::TestEnvironment env(&argc, argv); diff --git a/test/core/transport/chttp2/hpack_sync_corpus/clusterfuzz-testcase-minimized-hpack_sync_fuzzer-5224520566571008.fuzz b/test/core/transport/chttp2/hpack_sync_corpus/clusterfuzz-testcase-minimized-hpack_sync_fuzzer-5224520566571008.fuzz new file mode 100644 index 0000000000000..9057bb66f12b9 --- /dev/null +++ b/test/core/transport/chttp2/hpack_sync_corpus/clusterfuzz-testcase-minimized-hpack_sync_fuzzer-5224520566571008.fuzz @@ -0,0 +1,44 @@ +headers { +} +headers { +} +headers { + literal_inc_idx { + key: ":scheme" + value: "grpc-taOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOO\"\n value: \"`````````````````````````````````````````````````````````````````````\"\n }\n}\nheaders {\n}\nheaders {\n indexed: 2\n}\nheaders {\n indexed: 2\n}\nheaders {\n literal_inc_idx {\n key: \"OOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOO                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                              z                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                       OOOOOOOOOOOOOOOOOOO\"\n value: \"`````````````gs-bin" + } +} +headers { + literal_inc_idx { + key: "user-agent" + value: "LLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLL" + } +} +headers { + literal_not_idx { + value: "LLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLL" + } +} +headers { + indexed: 62 +} +headers { + indexed: 62 +} +headers { + indexed: 62 +} +headers { + indexed: 62 +} +headers { + literal_not_idx { + value: "LLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLL" + } +} +headers { + indexed: 8 +} +headers { + indexed: 62 +} diff --git a/test/core/transport/chttp2/hpack_sync_corpus/crash-0c85d3a3dad81ec97be1a3079ff93f17c25d9723 b/test/core/transport/chttp2/hpack_sync_corpus/crash-0c85d3a3dad81ec97be1a3079ff93f17c25d9723 new file mode 100644 index 0000000000000..de93bb7f52f3b --- /dev/null +++ b/test/core/transport/chttp2/hpack_sync_corpus/crash-0c85d3a3dad81ec97be1a3079ff93f17c25d9723 @@ -0,0 +1,10 @@ +headers { + literal_not_idx { + key: "\000\000\000\026" + } +} +headers { + literal_inc_idx { + value: "\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000" + } +} diff --git a/test/core/transport/chttp2/hpack_sync_corpus/crash-212b1a7ccb2034b7f21be3b413c2de51fcacef79 b/test/core/transport/chttp2/hpack_sync_corpus/crash-212b1a7ccb2034b7f21be3b413c2de51fcacef79 new file mode 100644 index 0000000000000..2e5ed89305226 --- /dev/null +++ b/test/core/transport/chttp2/hpack_sync_corpus/crash-212b1a7ccb2034b7f21be3b413c2de51fcacef79 @@ -0,0 +1,14 @@ +headers { + literal_not_idx_from_idx { + } +} +headers { + literal_not_idx_from_idx { + index: 18688 + } +} +headers { + literal_inc_idx { + key: "\001\000\000\000\000\000\000\003" + } +} diff --git a/test/core/transport/chttp2/hpack_sync_corpus/crash-298b34c2c15ac7b7fe8174017265d7e3b3313804 b/test/core/transport/chttp2/hpack_sync_corpus/crash-298b34c2c15ac7b7fe8174017265d7e3b3313804 new file mode 100644 index 0000000000000..b62c811708e83 --- /dev/null +++ b/test/core/transport/chttp2/hpack_sync_corpus/crash-298b34c2c15ac7b7fe8174017265d7e3b3313804 @@ -0,0 +1,12 @@ +headers { + literal_inc_idx { + value: "\013" + } +} +headers { +} +headers { + literal_inc_idx { + value: "\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000" + } +} diff --git a/test/core/transport/chttp2/hpack_sync_corpus/crash-5d27241617bb39dc456910aa4fa18431f2458dc4 b/test/core/transport/chttp2/hpack_sync_corpus/crash-5d27241617bb39dc456910aa4fa18431f2458dc4 new file mode 100644 index 0000000000000..80fb5f1df0df2 --- /dev/null +++ b/test/core/transport/chttp2/hpack_sync_corpus/crash-5d27241617bb39dc456910aa4fa18431f2458dc4 @@ -0,0 +1,9 @@ +headers { +} +headers { +} +headers { + literal_inc_idx { + key: "E" + } +} diff --git a/test/core/transport/chttp2/hpack_sync_corpus/crash-85f9f9c7c971ec3fa839df8b14b4bad15d13f4ea b/test/core/transport/chttp2/hpack_sync_corpus/crash-85f9f9c7c971ec3fa839df8b14b4bad15d13f4ea new file mode 100644 index 0000000000000..89b8cb868c34a --- /dev/null +++ b/test/core/transport/chttp2/hpack_sync_corpus/crash-85f9f9c7c971ec3fa839df8b14b4bad15d13f4ea @@ -0,0 +1,24 @@ +use_true_binary_metadata: true +headers { + literal_not_idx { + key: "OOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOO" + } +} +headers { + literal_inc_idx { + key: "grpc-tags-bin" + value: "grpc-tags-bin" + } +} +headers { + literal_inc_idx { + key: "grpc-tags-bin" + } +} +headers { + literal_inc_idx { + key: "grpc-tags-bin" + } +} +headers { +} diff --git a/test/core/transport/chttp2/hpack_sync_fuzzer.cc b/test/core/transport/chttp2/hpack_sync_fuzzer.cc new file mode 100644 index 0000000000000..3046d501bf8a0 --- /dev/null +++ b/test/core/transport/chttp2/hpack_sync_fuzzer.cc @@ -0,0 +1,173 @@ +// Copyright 2023 gRPC authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include +#include +#include + +#include +#include +#include +#include +#include +#include + +#include "absl/status/status.h" +#include "absl/strings/escaping.h" +#include "absl/strings/match.h" + +#include + +#include "src/core/ext/transport/chttp2/transport/hpack_encoder.h" +#include "src/core/ext/transport/chttp2/transport/hpack_encoder_table.h" +#include "src/core/ext/transport/chttp2/transport/hpack_parser.h" +#include "src/core/ext/transport/chttp2/transport/hpack_parser_table.h" +#include "src/core/lib/gprpp/ref_counted_ptr.h" +#include "src/core/lib/gprpp/status_helper.h" +#include "src/core/lib/iomgr/error.h" +#include "src/core/lib/iomgr/exec_ctx.h" +#include "src/core/lib/resource_quota/arena.h" +#include "src/core/lib/resource_quota/memory_quota.h" +#include "src/core/lib/resource_quota/resource_quota.h" +#include "src/core/lib/slice/slice.h" +#include "src/core/lib/slice/slice_buffer.h" +#include "src/core/lib/transport/metadata_batch.h" +#include "src/libfuzzer/libfuzzer_macro.h" +#include "test/core/transport/chttp2/hpack_sync_fuzzer.pb.h" + +bool squelch = true; +bool leak_check = true; + +static void dont_log(gpr_log_func_args* /*args*/) {} + +namespace grpc_core { +namespace { + +bool IsStreamError(const absl::Status& status) { + intptr_t stream_id; + return grpc_error_get_int(status, StatusIntProperty::kStreamId, &stream_id); +} + +void FuzzOneInput(const hpack_sync_fuzzer::Msg& msg) { + // STAGE 1: Encode the fuzzing input into a buffer (encode_output) + HPackCompressor compressor; + SliceBuffer encode_output; + hpack_encoder_detail::Encoder encoder( + &compressor, msg.use_true_binary_metadata(), encode_output); + for (const auto& header : msg.headers()) { + switch (header.ty_case()) { + case hpack_sync_fuzzer::Header::TY_NOT_SET: + break; + case hpack_sync_fuzzer::Header::kIndexed: + if (header.indexed() == 0) continue; // invalid encoding + encoder.EmitIndexed(header.indexed()); + break; + case hpack_sync_fuzzer::Header::kLiteralIncIdx: + if (header.literal_inc_idx().key().length() + + header.literal_inc_idx().value().length() > + HPackEncoderTable::MaxEntrySize() / 2) { + // Not an interesting case to fuzz + continue; + } + if (absl::EndsWith(header.literal_inc_idx().value(), "-bin")) { + std::ignore = encoder.EmitLitHdrWithBinaryStringKeyIncIdx( + Slice::FromCopiedString(header.literal_inc_idx().key()), + Slice::FromCopiedString(header.literal_inc_idx().value())); + } else { + std::ignore = encoder.EmitLitHdrWithNonBinaryStringKeyIncIdx( + Slice::FromCopiedString(header.literal_inc_idx().key()), + Slice::FromCopiedString(header.literal_inc_idx().value())); + } + break; + case hpack_sync_fuzzer::Header::kLiteralNotIdx: + if (absl::EndsWith(header.literal_not_idx().value(), "-bin")) { + encoder.EmitLitHdrWithBinaryStringKeyNotIdx( + Slice::FromCopiedString(header.literal_not_idx().key()), + Slice::FromCopiedString(header.literal_not_idx().value())); + } else { + encoder.EmitLitHdrWithNonBinaryStringKeyNotIdx( + Slice::FromCopiedString(header.literal_not_idx().key()), + Slice::FromCopiedString(header.literal_not_idx().value())); + } + break; + case hpack_sync_fuzzer::Header::kLiteralNotIdxFromIdx: + if (header.literal_not_idx_from_idx().index() == 0) continue; + encoder.EmitLitHdrWithBinaryStringKeyNotIdx( + header.literal_not_idx_from_idx().index(), + Slice::FromCopiedString(header.literal_not_idx_from_idx().value())); + break; + } + } + + // STAGE 2: Decode the buffer (encode_output) into a list of headers + HPackParser parser; + auto memory_allocator = + ResourceQuota::Default()->memory_quota()->CreateMemoryAllocator( + "test-allocator"); + auto arena = MakeScopedArena(1024, &memory_allocator); + ExecCtx exec_ctx; + grpc_metadata_batch read_metadata(arena.get()); + parser.BeginFrame( + &read_metadata, 1024, 1024, HPackParser::Boundary::EndOfHeaders, + HPackParser::Priority::None, + HPackParser::LogInfo{1, HPackParser::LogInfo::kHeaders, false}); + std::vector> seen_errors; + for (size_t i = 0; i < encode_output.Count(); i++) { + auto err = parser.Parse(encode_output.c_slice_at(i), + i == (encode_output.Count() - 1)); + if (!err.ok()) { + seen_errors.push_back(std::make_pair(i, err)); + // If we get a connection error (i.e. not a stream error), stop parsing, + // return. + if (!IsStreamError(err)) return; + } + } + + // STAGE 3: If we reached here we either had a stream error or no error + // parsing. + // Either way, the hpack tables should be of the same size between client and + // server. + const auto encoder_size = encoder.hpack_table().test_only_table_size(); + const auto parser_size = parser.hpack_table()->test_only_table_size(); + const auto encoder_elems = encoder.hpack_table().test_only_table_elems(); + const auto parser_elems = parser.hpack_table()->num_entries(); + if (encoder_size != parser_size || encoder_elems != parser_elems) { + fprintf(stderr, "Encoder size: %d Parser size: %d\n", encoder_size, + parser_size); + fprintf(stderr, "Encoder elems: %d Parser elems: %d\n", encoder_elems, + parser_elems); + if (!seen_errors.empty()) { + fprintf(stderr, "Seen errors during parse:\n"); + for (const auto& error : seen_errors) { + fprintf(stderr, " [slice %" PRIdPTR "] %s\n", error.first, + error.second.ToString().c_str()); + } + } + fprintf(stderr, "Encoded data:\n"); + for (size_t i = 0; i < encode_output.Count(); i++) { + fprintf( + stderr, " [slice %" PRIdPTR "]: %s\n", i, + absl::BytesToHexString(encode_output[i].as_string_view()).c_str()); + } + abort(); + } +} + +} // namespace +} // namespace grpc_core + +DEFINE_PROTO_FUZZER(const hpack_sync_fuzzer::Msg& msg) { + if (squelch) gpr_set_log_function(dont_log); + grpc_core::FuzzOneInput(msg); +} diff --git a/test/core/transport/chttp2/hpack_sync_fuzzer.proto b/test/core/transport/chttp2/hpack_sync_fuzzer.proto new file mode 100644 index 0000000000000..7cdacfeb1e740 --- /dev/null +++ b/test/core/transport/chttp2/hpack_sync_fuzzer.proto @@ -0,0 +1,43 @@ +// Copyright 2023 gRPC authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +syntax = "proto3"; + +package hpack_sync_fuzzer; + +message Empty {} + +message StringKeyValue { + string key = 1; + string value = 2; +} + +message IndexedKeyValue { + uint32 index = 1; + string value = 2; +} + +message Header { + oneof ty { + uint32 indexed = 1; + StringKeyValue literal_inc_idx = 2; + StringKeyValue literal_not_idx = 3; + IndexedKeyValue literal_not_idx_from_idx = 4; + } +} + +message Msg { + bool use_true_binary_metadata = 1; + repeated Header headers = 2; +} diff --git a/test/core/transport/chttp2/large-metadata.headers b/test/core/transport/chttp2/large-metadata.headers new file mode 100644 index 0000000000000..c3eb2bc7cab7c --- /dev/null +++ b/test/core/transport/chttp2/large-metadata.headers @@ -0,0 +1,8 @@ +# 19 -> 51 +adaldrida: brandybuck +# 12 -> 95 +adalgrim: took +# 18 -> 145 +amaranth: brandybuck +# 15 -> 192 +angelica: baggins diff --git a/test/core/transport/chttp2/long-base64.headers b/test/core/transport/chttp2/long-base64.headers new file mode 100644 index 0000000000000..37a62b127ec17 --- /dev/null +++ b/test/core/transport/chttp2/long-base64.headers @@ -0,0 +1 @@ +x-bin: QmFzZTY0IGVuY29kaW5nOgpJdCB0YWtlcyBiaW5hcnkgZGF0YSBhbmQgbWFrZXMgaXQgdGV4dC4KVXNlZnVsIGZvciBzdG9yaW5nIGZpbGVzLg== diff --git a/test/core/transport/chttp2/short.headers b/test/core/transport/chttp2/short.headers new file mode 100644 index 0000000000000..75dc0fcdd95ec --- /dev/null +++ b/test/core/transport/chttp2/short.headers @@ -0,0 +1 @@ +a: a diff --git a/test/core/transport/parsed_metadata_test.cc b/test/core/transport/parsed_metadata_test.cc index 387226b0c30a0..7d57ae9caffaf 100644 --- a/test/core/transport/parsed_metadata_test.cc +++ b/test/core/transport/parsed_metadata_test.cc @@ -208,16 +208,17 @@ INSTANTIATE_TYPED_TEST_SUITE_P(My, TraitSpecializedTest, InterestingTraits); TEST(KeyValueTest, Simple) { using PM = ParsedMetadata; using PMPtr = std::unique_ptr; - PMPtr p = std::make_unique(Slice::FromCopiedString("key"), - Slice::FromCopiedString("value")); + PMPtr p = + std::make_unique(PM::FromSlicePair{}, Slice::FromCopiedString("key"), + Slice::FromCopiedString("value"), 40); EXPECT_EQ(p->DebugString(), "key: value"); EXPECT_EQ(p->transport_size(), 40); - PM p2 = p->WithNewValue(Slice::FromCopiedString("some_other_value"), - [](absl::string_view msg, const Slice& value) { - ASSERT_TRUE(false) - << "Should not be called: msg=" << msg - << ", value=" << value.as_string_view(); - }); + PM p2 = p->WithNewValue( + Slice::FromCopiedString("some_other_value"), strlen("some_other_value"), + [](absl::string_view msg, const Slice& value) { + ASSERT_TRUE(false) << "Should not be called: msg=" << msg + << ", value=" << value.as_string_view(); + }); EXPECT_EQ(p->DebugString(), "key: value"); EXPECT_EQ(p2.DebugString(), "key: some_other_value"); EXPECT_EQ(p2.transport_size(), 51); @@ -232,18 +233,19 @@ TEST(KeyValueTest, Simple) { TEST(KeyValueTest, LongKey) { using PM = ParsedMetadata; using PMPtr = std::unique_ptr; - PMPtr p = std::make_unique(Slice::FromCopiedString(std::string(60, 'a')), - Slice::FromCopiedString("value")); + PMPtr p = std::make_unique(PM::FromSlicePair{}, + Slice::FromCopiedString(std::string(60, 'a')), + Slice::FromCopiedString("value"), 60 + 5 + 32); EXPECT_EQ( p->DebugString(), "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa: value"); EXPECT_EQ(p->transport_size(), 97); - PM p2 = p->WithNewValue(Slice::FromCopiedString("some_other_value"), - [](absl::string_view msg, const Slice& value) { - ASSERT_TRUE(false) - << "Should not be called: msg=" << msg - << ", value=" << value.as_string_view(); - }); + PM p2 = p->WithNewValue( + Slice::FromCopiedString("some_other_value"), strlen("some_other_value"), + [](absl::string_view msg, const Slice& value) { + ASSERT_TRUE(false) << "Should not be called: msg=" << msg + << ", value=" << value.as_string_view(); + }); EXPECT_EQ( p->DebugString(), "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa: value"); diff --git a/test/core/util/parse_hexstring.cc b/test/core/util/parse_hexstring.cc index 66b135acd26ad..5fb5876f35c65 100644 --- a/test/core/util/parse_hexstring.cc +++ b/test/core/util/parse_hexstring.cc @@ -21,17 +21,18 @@ #include #include +#include #include -grpc_slice parse_hexstring(const char* hexstring) { +namespace grpc_core { +Slice ParseHexstring(absl::string_view hexstring) { size_t nibbles = 0; - const char* p = nullptr; uint8_t* out; uint8_t temp; grpc_slice slice; - for (p = hexstring; *p; p++) { - nibbles += (*p >= '0' && *p <= '9') || (*p >= 'a' && *p <= 'f'); + for (auto c : hexstring) { + nibbles += (c >= '0' && c <= '9') || (c >= 'a' && c <= 'f'); } GPR_ASSERT((nibbles & 1) == 0); @@ -41,13 +42,13 @@ grpc_slice parse_hexstring(const char* hexstring) { nibbles = 0; temp = 0; - for (p = hexstring; *p; p++) { - if (*p >= '0' && *p <= '9') { - temp = static_cast(temp << 4) | static_cast(*p - '0'); + for (auto c : hexstring) { + if (c >= '0' && c <= '9') { + temp = static_cast(temp << 4) | static_cast(c - '0'); nibbles++; - } else if (*p >= 'a' && *p <= 'f') { + } else if (c >= 'a' && c <= 'f') { temp = - static_cast(temp << 4) | static_cast(*p - 'a' + 10); + static_cast(temp << 4) | static_cast(c - 'a' + 10); nibbles++; } if (nibbles == 2) { @@ -56,5 +57,6 @@ grpc_slice parse_hexstring(const char* hexstring) { } } - return slice; + return Slice(slice); } +} // namespace grpc_core diff --git a/test/core/util/parse_hexstring.h b/test/core/util/parse_hexstring.h index b34257868ae18..6870bfcd1e143 100644 --- a/test/core/util/parse_hexstring.h +++ b/test/core/util/parse_hexstring.h @@ -19,8 +19,12 @@ #ifndef GRPC_TEST_CORE_UTIL_PARSE_HEXSTRING_H #define GRPC_TEST_CORE_UTIL_PARSE_HEXSTRING_H -#include +#include "absl/strings/string_view.h" -grpc_slice parse_hexstring(const char* hexstring); +#include "src/core/lib/slice/slice.h" + +namespace grpc_core { +Slice ParseHexstring(absl::string_view hexstring); +} #endif // GRPC_TEST_CORE_UTIL_PARSE_HEXSTRING_H diff --git a/test/cpp/end2end/grpclb_end2end_test.cc b/test/cpp/end2end/grpclb_end2end_test.cc index 9a0b86d833e0f..9861e409da09c 100644 --- a/test/cpp/end2end/grpclb_end2end_test.cc +++ b/test/cpp/end2end/grpclb_end2end_test.cc @@ -101,8 +101,8 @@ constexpr char kDefaultServiceConfig[] = using BackendService = CountedService; using BalancerService = CountedService; -const char g_kCallCredsMdKey[] = "Balancer should not ..."; -const char g_kCallCredsMdValue[] = "... receive me"; +const char g_kCallCredsMdKey[] = "call-creds"; +const char g_kCallCredsMdValue[] = "should not be received by balancer"; // A test user agent string sent by the client only to the grpclb loadbalancer. // The backend should not see this user-agent string. diff --git a/tools/codegen/core/gen_header_frame.py b/tools/codegen/core/gen_header_frame.py index 4545f3e56d1bb..a501330ec9e05 100755 --- a/tools/codegen/core/gen_header_frame.py +++ b/tools/codegen/core/gen_header_frame.py @@ -24,36 +24,80 @@ import sys -def append_never_indexed(payload_line, n, count, key, value): +def append_never_indexed(payload_line, n, count, key, value, value_is_huff): payload_line.append(0x10) assert (len(key) <= 126) payload_line.append(len(key)) payload_line.extend(ord(c) for c in key) assert (len(value) <= 126) - payload_line.append(len(value)) - payload_line.extend(ord(c) for c in value) + payload_line.append(len(value) + (0x80 if value_is_huff else 0)) + payload_line.extend(value) -def append_inc_indexed(payload_line, n, count, key, value): +def append_inc_indexed(payload_line, n, count, key, value, value_is_huff): payload_line.append(0x40) assert (len(key) <= 126) payload_line.append(len(key)) payload_line.extend(ord(c) for c in key) assert (len(value) <= 126) - payload_line.append(len(value)) - payload_line.extend(ord(c) for c in value) + payload_line.append(len(value) + (0x80 if value_is_huff else 0)) + payload_line.extend(value) -def append_pre_indexed(payload_line, n, count, key, value): +def append_pre_indexed(payload_line, n, count, key, value, value_is_huff): + assert not value_is_huff payload_line.append(0x80 + 61 + count - n) +def esc_c(line): + out = "\"" + last_was_hex = False + for c in line: + if 32 <= c < 127: + if c in hex_bytes and last_was_hex: + out += "\"\"" + if c != ord('"'): + out += chr(c) + else: + out += "\\\"" + last_was_hex = False + else: + out += "\\x%02x" % c + last_was_hex = True + return out + "\"" + + +def output_c(payload_bytes): + for line in payload_bytes: + print((esc_c(line))) + + +def output_hex(payload_bytes): + all_bytes = [] + for line in payload_bytes: + all_bytes.extend(line) + print(('{%s}' % ', '.join('0x%02x' % c for c in all_bytes))) + + +def output_hexstr(payload_bytes): + all_bytes = [] + for line in payload_bytes: + all_bytes.extend(line) + print(('%s' % ''.join('%02x' % c for c in all_bytes))) + + _COMPRESSORS = { 'never': append_never_indexed, 'inc': append_inc_indexed, 'pre': append_pre_indexed, } +_OUTPUTS = { + 'c': output_c, + 'hex': output_hex, + 'hexstr': output_hexstr, +} + argp = argparse.ArgumentParser('Generate header frames') argp.add_argument('--set_end_stream', default=False, @@ -66,7 +110,8 @@ def append_pre_indexed(payload_line, n, count, key, value): argp.add_argument('--compression', choices=sorted(_COMPRESSORS.keys()), default='never') -argp.add_argument('--hex', default=False, action='store_const', const=True) +argp.add_argument('--huff', default=False, action='store_const', const=True) +argp.add_argument('--output', default='c', choices=sorted(_OUTPUTS.keys())) args = argp.parse_args() # parse input, fill in vals @@ -79,7 +124,13 @@ def append_pre_indexed(payload_line, n, count, key, value): continue key_tail, value = line[1:].split(':') key = (line[0] + key_tail).strip() - value = value.strip() + value = value.strip().encode('ascii') + if args.huff: + from hpack.huffman import HuffmanEncoder + from hpack.huffman_constants import REQUEST_CODES + from hpack.huffman_constants import REQUEST_CODES_LENGTH + value = HuffmanEncoder(REQUEST_CODES, + REQUEST_CODES_LENGTH).encode(value) vals.append((key, value)) # generate frame payload binary data @@ -90,7 +141,8 @@ def append_pre_indexed(payload_line, n, count, key, value): n = 0 for key, value in vals: payload_line = [] - _COMPRESSORS[args.compression](payload_line, n, len(vals), key, value) + _COMPRESSORS[args.compression](payload_line, n, len(vals), key, value, + args.huff) n += 1 payload_len += len(payload_line) payload_bytes.append(payload_line) @@ -117,31 +169,5 @@ def append_pre_indexed(payload_line, n, count, key, value): hex_bytes = [ord(c) for c in "abcdefABCDEF0123456789"] - -def esc_c(line): - out = "\"" - last_was_hex = False - for c in line: - if 32 <= c < 127: - if c in hex_bytes and last_was_hex: - out += "\"\"" - if c != ord('"'): - out += chr(c) - else: - out += "\\\"" - last_was_hex = False - else: - out += "\\x%02x" % c - last_was_hex = True - return out + "\"" - - # dump bytes -if args.hex: - all_bytes = [] - for line in payload_bytes: - all_bytes.extend(line) - print(('{%s}' % ', '.join('0x%02x' % c for c in all_bytes))) -else: - for line in payload_bytes: - print((esc_c(line))) +_OUTPUTS[args.output](payload_bytes)