No public description PiperOrigin-RevId: 853349771
diff --git a/quiche/common/http/http_header_block.cc b/quiche/common/http/http_header_block.cc index b0aa4cb..f6c6169 100644 --- a/quiche/common/http/http_header_block.cc +++ b/quiche/common/http/http_header_block.cc
@@ -51,6 +51,7 @@ HttpHeaderBlock::HeaderValue::HeaderValue(HeaderValue&& other) : storage_(other.storage_), fragments_(std::move(other.fragments_)), + consolidated_(std::move(other.consolidated_)), pair_(std::move(other.pair_)), size_(other.size_), separator_size_(other.separator_size_) {} @@ -59,6 +60,7 @@ HeaderValue&& other) { storage_ = other.storage_; fragments_ = std::move(other.fragments_); + consolidated_ = std::move(other.consolidated_); pair_ = std::move(other.pair_); size_ = other.size_; separator_size_ = other.separator_size_; @@ -75,16 +77,20 @@ if (fragments_.empty()) { return absl::string_view(); } - if (fragments_.size() > 1) { - fragments_ = { + if (fragments_.size() == 1 && consolidated_.empty()) { + consolidated_ = fragments_[0]; + } + if (fragments_.size() > 1 && consolidated_.empty()) { + consolidated_ = { storage_->WriteFragments(fragments_, SeparatorForKey(pair_.first))}; } - return fragments_[0]; + return consolidated_; } void HttpHeaderBlock::HeaderValue::Append(absl::string_view fragment) { size_ += (fragment.size() + separator_size_); fragments_.push_back(fragment); + consolidated_ = {}; } const std::pair<absl::string_view, absl::string_view>& @@ -93,6 +99,11 @@ return pair_; } +absl::Span<const absl::string_view> HttpHeaderBlock::HeaderValue::fragments() + const { + return fragments_; +} + HttpHeaderBlock::iterator::iterator(MapType::const_iterator it) : it_(it) {} HttpHeaderBlock::iterator::iterator(const iterator& other) = default; @@ -284,6 +295,16 @@ return ValueProxy(this, iter, out_key, &value_size_); } +void HttpHeaderBlock::ForEach( + quiche::UnretainedCallback<void(absl::string_view, absl::string_view)> fn) + const { + for (const auto& [name, header_value] : map_) { + for (absl::string_view value : header_value.fragments()) { + fn(name, value); + } + } +} + void HttpHeaderBlock::AppendValueOrAddHeader(const absl::string_view key, const absl::string_view value) { value_size_ += value.size();
diff --git a/quiche/common/http/http_header_block.h b/quiche/common/http/http_header_block.h index a11fd89..d5162c2 100644 --- a/quiche/common/http/http_header_block.h +++ b/quiche/common/http/http_header_block.h
@@ -18,6 +18,7 @@ #include "quiche/common/http/http_header_storage.h" #include "quiche/common/platform/api/quiche_export.h" #include "quiche/common/platform/api/quiche_logging.h" +#include "quiche/common/quiche_callbacks.h" #include "quiche/common/quiche_linked_hash_map.h" #include "quiche/common/quiche_text_utils.h" @@ -73,6 +74,7 @@ absl::string_view value() const { return as_pair().second; } const std::pair<absl::string_view, absl::string_view>& as_pair() const; + absl::Span<const absl::string_view> fragments() const; // Size estimate including separators. Used when keys are erased from // HttpHeaderBlock. @@ -85,6 +87,7 @@ mutable HttpHeaderStorage* storage_; mutable Fragments fragments_; + mutable absl::string_view consolidated_; // The first element is the key; the second is the consolidated value. mutable std::pair<absl::string_view, absl::string_view> pair_; size_t size_ = 0; @@ -250,6 +253,12 @@ // Allows either lookup or mutation of the value associated with a key. ABSL_MUST_USE_RESULT ValueProxy operator[](const absl::string_view key); + // The function is invoked on each (name, value) pair, where individual value + // fragments receive their own invocation. + void ForEach( + quiche::UnretainedCallback<void(absl::string_view, absl::string_view)> fn) + const; + size_t TotalBytesUsed() const { return key_size_ + value_size_; } private:
diff --git a/quiche/common/http/http_header_block_test.cc b/quiche/common/http/http_header_block_test.cc index 78deb71..81f299b 100644 --- a/quiche/common/http/http_header_block_test.cc +++ b/quiche/common/http/http_header_block_test.cc
@@ -12,6 +12,7 @@ #include "quiche/common/platform/api/quiche_test.h" using ::testing::ElementsAre; +using ::testing::IsEmpty; namespace quiche { namespace test { @@ -23,11 +24,20 @@ } }; -std::pair<absl::string_view, absl::string_view> Pair(absl::string_view k, - absl::string_view v) { +using HeaderField = std::pair<absl::string_view, absl::string_view>; + +HeaderField Pair(absl::string_view k, absl::string_view v) { return std::make_pair(k, v); } +struct Gatherer { + void operator()(absl::string_view name, absl::string_view value) { + fields.push_back({name, value}); + } + + std::vector<HeaderField> fields; +}; + // This test verifies that HttpHeaderBlock behaves correctly when empty. TEST(HttpHeaderBlockTest, EmptyBlock) { HttpHeaderBlock block; @@ -39,6 +49,12 @@ // Should have no effect. block.erase("bar"); + + Gatherer g; + block.ForEach(g); + EXPECT_THAT(g.fields, IsEmpty()); + + EXPECT_THAT(block, IsEmpty()); } TEST(HttpHeaderBlockTest, KeyMemoryReclaimedOnLookup) { @@ -74,13 +90,21 @@ // This test verifies that headers can be set in a variety of ways. TEST(HttpHeaderBlockTest, AddHeaders) { HttpHeaderBlock block; - block["foo"] = std::string(300, 'x'); + const std::string foo_value = std::string(300, 'x'); + block["foo"] = foo_value; block["bar"] = "baz"; block["qux"] = "qux1"; block["qux"] = "qux2"; block.insert(std::make_pair("key", "value")); - EXPECT_EQ(Pair("foo", std::string(300, 'x')), *block.find("foo")); + Gatherer g; + block.ForEach(g); + EXPECT_THAT( + g.fields, + ElementsAre(HeaderField{"foo", foo_value}, HeaderField{"bar", "baz"}, + HeaderField{"qux", "qux2"}, HeaderField{"key", "value"})); + + EXPECT_EQ(Pair("foo", foo_value), *block.find("foo")); EXPECT_EQ("baz", block["bar"]); std::string qux("qux"); EXPECT_EQ("qux2", block[qux]); @@ -90,6 +114,13 @@ block.erase("key"); EXPECT_EQ(block.end(), block.find("key")); + + g = {}; + block.ForEach(g); + ASSERT_EQ(g.fields.size(), 3); + EXPECT_THAT(g.fields, ElementsAre(HeaderField{"foo", foo_value}, + HeaderField{"bar", "baz"}, + HeaderField{"qux", "qux2"})); } // This test verifies that HttpHeaderBlock can be copied using Clone(). @@ -198,6 +229,34 @@ EXPECT_EQ(std::string("h3v2\0h3v3", 9), block["h3"]); EXPECT_EQ("singleton", block["h4"]); EXPECT_EQ(std::string("yummy\0scrumptious", 17), block["set-cookie"]); + + // Iterating over the block yields field names and consolidated values. + EXPECT_THAT( + block, + ElementsAre( + HeaderField{"foo", "baz"}, + HeaderField{"cookie", "key1=value1; key2=value2; key3=value3"}, + HeaderField{"h1", std::string("h1v1\0h1v2\0h1v3", 14)}, + HeaderField{"h2", std::string("h2v1\0h2v2\0h2v3", 14)}, + HeaderField{"h3", std::string("h3v2\0h3v3", 9)}, + HeaderField{"h4", "singleton"}, + HeaderField{"set-cookie", std::string("yummy\0scrumptious", 17)})); + + Gatherer g; + block.ForEach(g); + // Iterating with ForEach yields each name/value pair individually. + EXPECT_THAT(g.fields, + ElementsAre(HeaderField{"foo", "baz"}, + HeaderField{"cookie", "key1=value1"}, + HeaderField{"cookie", "key2=value2"}, + HeaderField{"cookie", "key3=value3"}, + HeaderField{"h1", "h1v1"}, HeaderField{"h1", "h1v2"}, + HeaderField{"h1", "h1v3"}, HeaderField{"h2", "h2v1"}, + HeaderField{"h2", "h2v2"}, HeaderField{"h2", "h2v3"}, + HeaderField{"h3", "h3v2"}, HeaderField{"h3", "h3v3"}, + HeaderField{"h4", "singleton"}, + HeaderField{"set-cookie", "yummy"}, + HeaderField{"set-cookie", "scrumptious"})); } TEST(HttpHeaderBlockTest, CompareValueToStringPiece) {
diff --git a/quiche/common/quiche_feature_flags_list.h b/quiche/common/quiche_feature_flags_list.h index 57bc31e..43de29a 100755 --- a/quiche/common/quiche_feature_flags_list.h +++ b/quiche/common/quiche_feature_flags_list.h
@@ -9,6 +9,7 @@ #if defined(QUICHE_FLAG) QUICHE_FLAG(bool, quiche_reloadable_flag_enable_h3_origin_frame, false, true, "If true, enables support for parsing HTTP/3 ORIGIN frames.") +QUICHE_FLAG(bool, quiche_reloadable_flag_http2_avoid_decompose_representation, false, false, "If true, HPACK encoding will use a different iteration method that avoids unnecessary copies and string splitting.") QUICHE_FLAG(bool, quiche_reloadable_flag_quic_act_upon_invalid_header, true, true, "If true, reject or send error response code upon receiving invalid request or response headers.") QUICHE_FLAG(bool, quiche_reloadable_flag_quic_add_stream_info_to_idle_close_detail, false, true, "If true, include stream information in idle timeout connection close detail.") QUICHE_FLAG(bool, quiche_reloadable_flag_quic_allow_client_enabled_bbr_v2, true, true, "If true, allow client to enable BBRv2 on server via connection option 'B2ON'.")
diff --git a/quiche/http2/adapter/oghttp2_adapter_test.cc b/quiche/http2/adapter/oghttp2_adapter_test.cc index b8189fe..97a9f80 100644 --- a/quiche/http2/adapter/oghttp2_adapter_test.cc +++ b/quiche/http2/adapter/oghttp2_adapter_test.cc
@@ -7292,8 +7292,13 @@ OnHeaderForStream(1, ":authority", "example.com")); EXPECT_CALL(server_visitor, OnHeaderForStream(1, ":path", "/this/is/request/one")); - EXPECT_CALL(server_visitor, - OnHeaderForStream(1, "cookie", "a; b=2; c; d=e, f, g; h")); + if (GetQuicheReloadableFlag(http2_avoid_decompose_representation)) { + EXPECT_CALL(server_visitor, OnHeaderForStream(1, "cookie", "a; b=2; c")); + EXPECT_CALL(server_visitor, OnHeaderForStream(1, "cookie", "d=e, f, g; h")); + } else { + EXPECT_CALL(server_visitor, + OnHeaderForStream(1, "cookie", "a; b=2; c; d=e, f, g; h")); + } EXPECT_CALL(server_visitor, OnEndHeadersForStream(1)); EXPECT_CALL(server_visitor, OnEndStream(1));
diff --git a/quiche/http2/hpack/hpack_encoder.cc b/quiche/http2/hpack/hpack_encoder.cc index e9707a7..e5ddb5f 100644 --- a/quiche/http2/hpack/hpack_encoder.cc +++ b/quiche/http2/hpack/hpack_encoder.cc
@@ -20,7 +20,10 @@ #include "quiche/http2/hpack/huffman/hpack_huffman_encoder.h" #include "quiche/common/http/http_header_block.h" #include "quiche/common/platform/api/quiche_bug_tracker.h" +#include "quiche/common/platform/api/quiche_flag_utils.h" +#include "quiche/common/platform/api/quiche_flags.h" #include "quiche/common/platform/api/quiche_logging.h" +#include "quiche/common/quiche_feature_flags_list.h" namespace spdy { @@ -98,22 +101,43 @@ // Separate header set into pseudo-headers and regular headers. Representations pseudo_headers; Representations regular_headers; - bool found_cookie = false; - for (const auto& header : header_set) { - if (!found_cookie && header.first == "cookie") { - // Note that there can only be one "cookie" header, because header_set is - // a map. - found_cookie = true; - if (crumble_cookies_) { - CookieToCrumbs(header, ®ular_headers); + if (GetQuicheReloadableFlag(http2_avoid_decompose_representation)) { + QUICHE_RELOADABLE_FLAG_COUNT_N(http2_avoid_decompose_representation, 1, 4); + header_set.ForEach( + [&pseudo_headers, ®ular_headers, crumble_cookies = crumble_cookies_]( + absl::string_view name, absl::string_view value) { + QUICHE_RELOADABLE_FLAG_COUNT_N(http2_avoid_decompose_representation, + 2, 4); + if (name == "cookie") { + if (crumble_cookies) { + CookieToCrumbs({name, value}, ®ular_headers); + } else { + regular_headers.push_back({name, value}); + } + } else if (!name.empty() && name[0] == kPseudoHeaderPrefix) { + pseudo_headers.push_back({name, value}); + } else { + regular_headers.push_back({name, value}); + } + }); + } else { + bool found_cookie = false; + for (const auto& header : header_set) { + if (!found_cookie && header.first == "cookie") { + // Note that there can only be one "cookie" header, because header_set + // is a map. + found_cookie = true; + if (crumble_cookies_) { + CookieToCrumbs(header, ®ular_headers); + } else { + DecomposeRepresentation(header, ®ular_headers); + } + } else if (!header.first.empty() && + header.first[0] == kPseudoHeaderPrefix) { + DecomposeRepresentation(header, &pseudo_headers); } else { DecomposeRepresentation(header, ®ular_headers); } - } else if (!header.first.empty() && - header.first[0] == kPseudoHeaderPrefix) { - DecomposeRepresentation(header, &pseudo_headers); - } else { - DecomposeRepresentation(header, ®ular_headers); } } @@ -320,23 +344,43 @@ HpackEncoder::Encoderator::Encoderator( const quiche::HttpHeaderBlock& header_set, HpackEncoder* encoder) : encoder_(encoder), has_next_(true) { - // Separate header set into pseudo-headers and regular headers. - bool found_cookie = false; - for (const auto& header : header_set) { - if (!found_cookie && header.first == "cookie") { - // Note that there can only be one "cookie" header, because header_set - // is a map. - found_cookie = true; - if (encoder_->crumble_cookies_) { - CookieToCrumbs(header, ®ular_headers_); + if (GetQuicheReloadableFlag(http2_avoid_decompose_representation)) { + QUICHE_RELOADABLE_FLAG_COUNT_N(http2_avoid_decompose_representation, 3, 4); + header_set.ForEach([this](absl::string_view name, absl::string_view value) { + QUICHE_RELOADABLE_FLAG_COUNT_N(http2_avoid_decompose_representation, 4, + 4); + if (name == "cookie") { + if (encoder_->crumble_cookies_) { + CookieToCrumbs({name, value}, ®ular_headers_); + } else { + regular_headers_.push_back({name, value}); + } + } else if (!name.empty() && name[0] == kPseudoHeaderPrefix) { + pseudo_headers_.push_back({name, value}); + } else { + regular_headers_.push_back({name, value}); + } + }); + + } else { + // Separate header set into pseudo-headers and regular headers. + bool found_cookie = false; + for (const auto& header : header_set) { + if (!found_cookie && header.first == "cookie") { + // Note that there can only be one "cookie" header, because header_set + // is a map. + found_cookie = true; + if (encoder_->crumble_cookies_) { + CookieToCrumbs(header, ®ular_headers_); + } else { + DecomposeRepresentation(header, ®ular_headers_); + } + } else if (!header.first.empty() && + header.first[0] == kPseudoHeaderPrefix) { + DecomposeRepresentation(header, &pseudo_headers_); } else { DecomposeRepresentation(header, ®ular_headers_); } - } else if (!header.first.empty() && - header.first[0] == kPseudoHeaderPrefix) { - DecomposeRepresentation(header, &pseudo_headers_); - } else { - DecomposeRepresentation(header, ®ular_headers_); } } header_it_ = std::make_unique<RepresentationIterator>(pseudo_headers_,
diff --git a/quiche/http2/hpack/hpack_encoder_test.cc b/quiche/http2/hpack/hpack_encoder_test.cc index a924341..14e4178 100644 --- a/quiche/http2/hpack/hpack_encoder_test.cc +++ b/quiche/http2/hpack/hpack_encoder_test.cc
@@ -821,7 +821,9 @@ } quiche::HttpHeaderBlock headers; // A header field to be crumbled: "spam: foo\0bar". - headers["spam"] = std::string("foo\0bar", 7); + headers["spam"] = "foo"; + headers.AppendValueOrAddHeader("spam", "bar"); + EXPECT_EQ(headers["spam"], std::string("foo\0bar", 7)); ExpectIndexedLiteral("spam", "foo"); expected_.AppendPrefix(kLiteralIncrementalIndexOpcode);