Incrementally process HTTP/3 METADATA frames instead of buffering the full payload. PiperOrigin-RevId: 615608615
diff --git a/build/source_list.bzl b/build/source_list.bzl index 9d9cf94..19fb0c8 100644 --- a/build/source_list.bzl +++ b/build/source_list.bzl
@@ -224,6 +224,7 @@ "quic/core/http/http_decoder.h", "quic/core/http/http_encoder.h", "quic/core/http/http_frames.h", + "quic/core/http/metadata_decoder.h", "quic/core/http/quic_header_list.h", "quic/core/http/quic_headers_stream.h", "quic/core/http/quic_receive_control_stream.h", @@ -562,6 +563,7 @@ "quic/core/http/http_constants.cc", "quic/core/http/http_decoder.cc", "quic/core/http/http_encoder.cc", + "quic/core/http/metadata_decoder.cc", "quic/core/http/quic_header_list.cc", "quic/core/http/quic_headers_stream.cc", "quic/core/http/quic_receive_control_stream.cc", @@ -1197,6 +1199,7 @@ "quic/core/http/http_decoder_test.cc", "quic/core/http/http_encoder_test.cc", "quic/core/http/http_frames_test.cc", + "quic/core/http/metadata_decoder_test.cc", "quic/core/http/quic_header_list_test.cc", "quic/core/http/quic_headers_stream_test.cc", "quic/core/http/quic_receive_control_stream_test.cc",
diff --git a/build/source_list.gni b/build/source_list.gni index 589f737..b8ab180 100644 --- a/build/source_list.gni +++ b/build/source_list.gni
@@ -224,6 +224,7 @@ "src/quiche/quic/core/http/http_decoder.h", "src/quiche/quic/core/http/http_encoder.h", "src/quiche/quic/core/http/http_frames.h", + "src/quiche/quic/core/http/metadata_decoder.h", "src/quiche/quic/core/http/quic_header_list.h", "src/quiche/quic/core/http/quic_headers_stream.h", "src/quiche/quic/core/http/quic_receive_control_stream.h", @@ -562,6 +563,7 @@ "src/quiche/quic/core/http/http_constants.cc", "src/quiche/quic/core/http/http_decoder.cc", "src/quiche/quic/core/http/http_encoder.cc", + "src/quiche/quic/core/http/metadata_decoder.cc", "src/quiche/quic/core/http/quic_header_list.cc", "src/quiche/quic/core/http/quic_headers_stream.cc", "src/quiche/quic/core/http/quic_receive_control_stream.cc", @@ -1198,6 +1200,7 @@ "src/quiche/quic/core/http/http_decoder_test.cc", "src/quiche/quic/core/http/http_encoder_test.cc", "src/quiche/quic/core/http/http_frames_test.cc", + "src/quiche/quic/core/http/metadata_decoder_test.cc", "src/quiche/quic/core/http/quic_header_list_test.cc", "src/quiche/quic/core/http/quic_headers_stream_test.cc", "src/quiche/quic/core/http/quic_receive_control_stream_test.cc",
diff --git a/build/source_list.json b/build/source_list.json index 274f204..373a3ea 100644 --- a/build/source_list.json +++ b/build/source_list.json
@@ -223,6 +223,7 @@ "quiche/quic/core/http/http_decoder.h", "quiche/quic/core/http/http_encoder.h", "quiche/quic/core/http/http_frames.h", + "quiche/quic/core/http/metadata_decoder.h", "quiche/quic/core/http/quic_header_list.h", "quiche/quic/core/http/quic_headers_stream.h", "quiche/quic/core/http/quic_receive_control_stream.h", @@ -561,6 +562,7 @@ "quiche/quic/core/http/http_constants.cc", "quiche/quic/core/http/http_decoder.cc", "quiche/quic/core/http/http_encoder.cc", + "quiche/quic/core/http/metadata_decoder.cc", "quiche/quic/core/http/quic_header_list.cc", "quiche/quic/core/http/quic_headers_stream.cc", "quiche/quic/core/http/quic_receive_control_stream.cc", @@ -1197,6 +1199,7 @@ "quiche/quic/core/http/http_decoder_test.cc", "quiche/quic/core/http/http_encoder_test.cc", "quiche/quic/core/http/http_frames_test.cc", + "quiche/quic/core/http/metadata_decoder_test.cc", "quiche/quic/core/http/quic_header_list_test.cc", "quiche/quic/core/http/quic_headers_stream_test.cc", "quiche/quic/core/http/quic_receive_control_stream_test.cc",
diff --git a/quiche/quic/core/http/metadata_decoder.cc b/quiche/quic/core/http/metadata_decoder.cc new file mode 100644 index 0000000..ef14513 --- /dev/null +++ b/quiche/quic/core/http/metadata_decoder.cc
@@ -0,0 +1,43 @@ +// Copyright 2024 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "quiche/quic/core/http/metadata_decoder.h" + +namespace quic { + +MetadataDecoder::MetadataDecoder(QuicStreamId id, size_t max_header_list_size, + size_t frame_header_len, size_t payload_length) + : qpack_decoder_(/*maximum_dynamic_table_capacity=*/0, + /*maximum_blocked_streams=*/0, &delegate_), + accumulator_(id, &qpack_decoder_, &decoder_, max_header_list_size), + frame_len_(frame_header_len + payload_length), + bytes_remaining_(payload_length) {} + +bool MetadataDecoder::Decode(absl::string_view payload) { + accumulator_.Decode(payload); + bytes_remaining_ -= payload.length(); + return decoder_.error_code() == QUIC_NO_ERROR; +} + +bool MetadataDecoder::EndHeaderBlock() { + QUIC_BUG_IF(METADATA bytes remaining, bytes_remaining_ != 0) + << "More metadata remaining: " << bytes_remaining_; + + accumulator_.EndHeaderBlock(); + return !decoder_.header_list_size_limit_exceeded(); +} + +void MetadataDecoder::MetadataHeadersDecoder::OnHeadersDecoded( + QuicHeaderList headers, bool header_list_size_limit_exceeded) { + header_list_size_limit_exceeded_ = header_list_size_limit_exceeded; + headers_ = std::move(headers); +} + +void MetadataDecoder::MetadataHeadersDecoder::OnHeaderDecodingError( + QuicErrorCode error_code, absl::string_view error_message) { + error_code_ = error_code; + error_message_ = absl::StrCat("Error decoding metadata: ", error_message); +} + +} // namespace quic
diff --git a/quiche/quic/core/http/metadata_decoder.h b/quiche/quic/core/http/metadata_decoder.h new file mode 100644 index 0000000..ec33c17 --- /dev/null +++ b/quiche/quic/core/http/metadata_decoder.h
@@ -0,0 +1,73 @@ +// Copyright 2024 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef QUICHE_QUIC_CORE_HTTP_METADATA_DECODER_H_ +#define QUICHE_QUIC_CORE_HTTP_METADATA_DECODER_H_ + +#include <sys/types.h> + +#include <cstddef> +#include <memory> +#include <string> + +#include "quiche/quic/core/http/quic_header_list.h" +#include "quiche/quic/core/qpack/qpack_decoded_headers_accumulator.h" +#include "quiche/quic/core/qpack/qpack_decoder.h" +#include "quiche/quic/core/quic_error_codes.h" + +namespace quic { + +// Class for decoding the payload of HTTP/3 METADATA frames. +class QUICHE_EXPORT MetadataDecoder { + public: + MetadataDecoder(QuicStreamId id, size_t max_header_list_size, + size_t frame_header_len, size_t payload_length); + + // Incrementally decodes the next bytes of METADATA frame payload + // and returns true if there were no errors. + bool Decode(absl::string_view payload); + + // Finishes the decoding. Must be called after the full frame payload + // has been decoded. Returns true if there were no errors. + bool EndHeaderBlock(); + + const std::string& error_message() { return decoder_.error_message(); } + size_t frame_len() { return frame_len_; } + const QuicHeaderList& headers() { return decoder_.headers(); } + + private: + class MetadataHeadersDecoder + : public QpackDecodedHeadersAccumulator::Visitor { + public: + // QpackDecodedHeadersAccumulator::Visitor + void OnHeadersDecoded(QuicHeaderList headers, + bool header_list_size_limit_exceeded) override; + void OnHeaderDecodingError(QuicErrorCode error_code, + absl::string_view error_message) override; + + QuicErrorCode error_code() { return error_code_; } + const std::string& error_message() { return error_message_; } + QuicHeaderList& headers() { return headers_; } + bool header_list_size_limit_exceeded() { + return header_list_size_limit_exceeded_; + } + + private: + QuicErrorCode error_code_ = QUIC_NO_ERROR; + QuicHeaderList headers_; + std::string error_message_; + bool header_list_size_limit_exceeded_ = false; + }; + + NoopEncoderStreamErrorDelegate delegate_; + QpackDecoder qpack_decoder_; + MetadataHeadersDecoder decoder_; + QpackDecodedHeadersAccumulator accumulator_; + const size_t frame_len_; + size_t bytes_remaining_ = 0; // Debug only. +}; + +} // namespace quic + +#endif // QUICHE_QUIC_CORE_HTTP_METADATA_DECODER_H_
diff --git a/quiche/quic/core/http/metadata_decoder_test.cc b/quiche/quic/core/http/metadata_decoder_test.cc new file mode 100644 index 0000000..791a84d --- /dev/null +++ b/quiche/quic/core/http/metadata_decoder_test.cc
@@ -0,0 +1,83 @@ +// Copyright 2024 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "quiche/quic/core/http/metadata_decoder.h" + +#include "absl/strings/escaping.h" +#include "quiche/quic/core/qpack/qpack_encoder.h" +#include "quiche/quic/platform/api/quic_test.h" +#include "quiche/quic/test_tools/quic_test_utils.h" + +namespace quic { +namespace test { +namespace { + +class MetadataDecoderTest : public QuicTest { + protected: + std::string EncodeHeaders(spdy::Http2HeaderBlock& headers) { + quic::NoopDecoderStreamErrorDelegate delegate; + quic::QpackEncoder encoder(&delegate, quic::HuffmanEncoding::kDisabled); + return encoder.EncodeHeaderList(id_, headers, + /*encoder_stream_sent_byte_count=*/nullptr); + } + + size_t max_header_list_size = 1 << 20; // 1 MB + const QuicStreamId id_ = 1; +}; + +TEST_F(MetadataDecoderTest, Initialize) { + const size_t frame_header_len = 4; + const size_t payload_len = 123; + MetadataDecoder decoder(id_, max_header_list_size, frame_header_len, + payload_len); + EXPECT_EQ(frame_header_len + payload_len, decoder.frame_len()); + EXPECT_EQ("", decoder.error_message()); + EXPECT_TRUE(decoder.headers().empty()); +} + +TEST_F(MetadataDecoderTest, Decode) { + spdy::Http2HeaderBlock headers; + headers["key1"] = "val1"; + headers["key2"] = "val2"; + headers["key3"] = "val3"; + std::string data = EncodeHeaders(headers); + + const size_t frame_header_len = 4; + MetadataDecoder decoder(id_, max_header_list_size, frame_header_len, + data.length()); + EXPECT_TRUE(decoder.Decode(data)); + EXPECT_TRUE(decoder.EndHeaderBlock()); + EXPECT_EQ(quic::test::AsHeaderList(headers), decoder.headers()); +} + +TEST_F(MetadataDecoderTest, DecodeInvalidHeaders) { + std::string data = "aaaaaaaaaa"; + + const size_t frame_header_len = 4; + MetadataDecoder decoder(id_, max_header_list_size, frame_header_len, + data.length()); + EXPECT_FALSE(decoder.Decode(data)); + EXPECT_EQ("Error decoding metadata: Error decoding Required Insert Count.", + decoder.error_message()); +} + +TEST_F(MetadataDecoderTest, TooLarge) { + spdy::Http2HeaderBlock headers; + for (int i = 0; i < 1024; ++i) { + headers.AppendValueOrAddHeader(absl::StrCat(i), std::string(1024, 'a')); + } + std::string data = EncodeHeaders(headers); + + EXPECT_GT(data.length(), 1 << 20); + const size_t frame_header_len = 4; + MetadataDecoder decoder(id_, max_header_list_size, frame_header_len, + data.length()); + EXPECT_TRUE(decoder.Decode(data)); + EXPECT_FALSE(decoder.EndHeaderBlock()); + EXPECT_TRUE(decoder.error_message().empty()); +} + +} // namespace +} // namespace test +} // namespace quic
diff --git a/quiche/quic/core/http/quic_spdy_stream.cc b/quiche/quic/core/http/quic_spdy_stream.cc index 4a5c2ed..035d21f 100644 --- a/quiche/quic/core/http/quic_spdy_stream.cc +++ b/quiche/quic/core/http/quic_spdy_stream.cc
@@ -1207,11 +1207,11 @@ static_cast<uint64_t>(quic::HttpFrameType::METADATA), header_length, payload_length); } - QUIC_BUG_IF(Invalid METADATA state, received_metadata_payload_ != nullptr) - << "Invalid metadata state"; - received_metadata_payload_ = - std::make_unique<ReceivedMetadataPayload>(payload_length); - received_metadata_payload_->frame_len = header_length + payload_length; + + QUIC_BUG_IF(Invalid METADATA state, metadata_decoder_ != nullptr); + constexpr size_t kMaxMetadataBlockSize = 1 << 20; // 1 MB + metadata_decoder_ = std::make_unique<MetadataDecoder>( + id(), kMaxMetadataBlockSize, header_length, payload_length); // Consume the frame header. QUIC_DVLOG(1) << ENDPOINT << "Consuming " << header_length @@ -1224,8 +1224,12 @@ if (metadata_visitor_ == nullptr) { return OnUnknownFramePayload(payload); } - received_metadata_payload_->buffer.push_back(std::string(payload)); - received_metadata_payload_->bytes_remaining -= payload.size(); + + if (!metadata_decoder_->Decode(payload)) { + OnUnrecoverableError(QUIC_DECOMPRESSION_FAILURE, + metadata_decoder_->error_message()); + return false; + } // Consume the frame payload. QUIC_DVLOG(1) << ENDPOINT << "Consuming " << payload.size() @@ -1233,66 +1237,21 @@ sequencer()->MarkConsumed(body_manager_.OnNonBody(payload.size())); return true; } -namespace { -class MetadataHeadersDecoder : public QpackDecodedHeadersAccumulator::Visitor { - public: - void OnHeadersDecoded(QuicHeaderList headers, - bool header_list_size_limit_exceeded) override { - header_list_size_limit_exceeded_ = header_list_size_limit_exceeded; - headers_ = std::move(headers); - } - - void OnHeaderDecodingError(QuicErrorCode error_code, - absl::string_view error_message) override { - error_code_ = error_code; - error_message_ = absl::StrCat("Error decoding metadata: ", error_message); - } - - QuicErrorCode error_code() { return error_code_; } - std::string error_message() { return error_message_; } - QuicHeaderList& headers() { return headers_; } - bool header_list_size_limit_exceeded() { - return header_list_size_limit_exceeded_; - } - - private: - QuicErrorCode error_code_ = QUIC_NO_ERROR; - QuicHeaderList headers_; - std::string error_message_; - bool header_list_size_limit_exceeded_ = false; -}; -} // namespace bool QuicSpdyStream::OnMetadataFrameEnd() { if (metadata_visitor_ == nullptr) { return OnUnknownFrameEnd(); } - QUIC_BUG_IF(METADATA bytes remaining, - received_metadata_payload_->bytes_remaining != 0) - << "More metadata remaining: " - << received_metadata_payload_->bytes_remaining; - quic::NoopEncoderStreamErrorDelegate delegate; - quic::QpackDecoder qpack_decoder(/*maximum_dynamic_table_capacity=*/0, - /*maximum_blocked_streams=*/0, &delegate); - - constexpr size_t kMaxMetadataBlockSize = 1 << 20; // 1 MB - MetadataHeadersDecoder decoder; - quic::QpackDecodedHeadersAccumulator accumulator( - id(), &qpack_decoder, &decoder, kMaxMetadataBlockSize); - for (const std::string& slice : received_metadata_payload_->buffer) { - accumulator.Decode(slice); - if (decoder.error_code() != QUIC_NO_ERROR) { - OnUnrecoverableError(QUIC_DECOMPRESSION_FAILURE, decoder.error_message()); - return false; - } + if (!metadata_decoder_->EndHeaderBlock()) { + OnUnrecoverableError(QUIC_DECOMPRESSION_FAILURE, + metadata_decoder_->error_message()); + return false; } - accumulator.EndHeaderBlock(); - - metadata_visitor_->OnMetadataComplete(received_metadata_payload_->frame_len, - decoder.headers()); - received_metadata_payload_.reset(); + metadata_visitor_->OnMetadataComplete(metadata_decoder_->frame_len(), + metadata_decoder_->headers()); + metadata_decoder_.reset(); return !sequencer()->IsClosed() && !reading_stopped(); }
diff --git a/quiche/quic/core/http/quic_spdy_stream.h b/quiche/quic/core/http/quic_spdy_stream.h index 10c34b1..d33abed 100644 --- a/quiche/quic/core/http/quic_spdy_stream.h +++ b/quiche/quic/core/http/quic_spdy_stream.h
@@ -21,6 +21,7 @@ #include "absl/types/span.h" #include "quiche/quic/core/http/http_decoder.h" #include "quiche/quic/core/http/http_encoder.h" +#include "quiche/quic/core/http/metadata_decoder.h" #include "quiche/quic/core/http/quic_header_list.h" #include "quiche/quic/core/http/quic_spdy_stream_body_manager.h" #include "quiche/quic/core/http/web_transport_stream_adapter.h" @@ -522,16 +523,9 @@ // Present if HTTP/3 METADATA frames should be parsed. MetadataVisitor* metadata_visitor_ = nullptr; - struct ReceivedMetadataPayload { - explicit ReceivedMetadataPayload(size_t remaining) - : bytes_remaining(remaining) {} - std::list<std::string> buffer; - size_t frame_len = 0; - size_t bytes_remaining = 0; - }; // Present if an HTTP/3 METADATA is currently being parsed. - std::unique_ptr<ReceivedMetadataPayload> received_metadata_payload_; + std::unique_ptr<MetadataDecoder> metadata_decoder_; // Empty if the headers are valid. std::string invalid_request_details_;