Adds the ability to skip `Cookie` crumbling in `HpackEncoder`.
This is preparation for fixing the issue described in https://github.com/envoyproxy/envoy/issues/32611.
Protected by new code path, not yet enabled in the GFE binary.
PiperOrigin-RevId: 613330788
diff --git a/quiche/spdy/core/hpack/hpack_encoder.cc b/quiche/spdy/core/hpack/hpack_encoder.cc
index 0891478..1fff60d 100644
--- a/quiche/spdy/core/hpack/hpack_encoder.cc
+++ b/quiche/spdy/core/hpack/hpack_encoder.cc
@@ -86,7 +86,8 @@
listener_(NoOpListener),
should_index_(DefaultPolicy),
enable_compression_(true),
- should_emit_table_size_(false) {}
+ should_emit_table_size_(false),
+ crumble_cookies_(true) {}
HpackEncoder::~HpackEncoder() = default;
@@ -101,7 +102,11 @@
// Note that there can only be one "cookie" header, because header_set is
// a map.
found_cookie = true;
- CookieToCrumbs(header, ®ular_headers);
+ 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);
@@ -302,7 +307,11 @@
// Note that there can only be one "cookie" header, because header_set
// is a map.
found_cookie = true;
- CookieToCrumbs(header, ®ular_headers_);
+ 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_);
@@ -321,7 +330,11 @@
: encoder_(encoder), has_next_(true) {
for (const auto& header : representations) {
if (header.first == "cookie") {
- CookieToCrumbs(header, ®ular_headers_);
+ if (encoder_->crumble_cookies_) {
+ CookieToCrumbs(header, ®ular_headers_);
+ } else {
+ DecomposeRepresentation(header, ®ular_headers_);
+ }
} else if (!header.first.empty() &&
header.first[0] == kPseudoHeaderPrefix) {
pseudo_headers_.push_back(header);
diff --git a/quiche/spdy/core/hpack/hpack_encoder.h b/quiche/spdy/core/hpack/hpack_encoder.h
index 7eacfb3..35007c6 100644
--- a/quiche/spdy/core/hpack/hpack_encoder.h
+++ b/quiche/spdy/core/hpack/hpack_encoder.h
@@ -97,6 +97,12 @@
void DisableCompression() { enable_compression_ = false; }
+ // Disables the deconstruction of Cookie header values into individual
+ // components, as described in
+ // https://httpwg.org/specs/rfc9113.html#CompressCookie. The deconstructed
+ // representation can cause problems for some HTTP/2 endpoints.
+ void DisableCookieCrumbling() { crumble_cookies_ = false; }
+
// Returns the current dynamic table size, including the 32 bytes per entry
// overhead mentioned in RFC 7541 section 4.1.
size_t GetDynamicTableSize() const { return header_table_.size(); }
@@ -142,6 +148,7 @@
IndexingPolicy should_index_;
bool enable_compression_;
bool should_emit_table_size_;
+ bool crumble_cookies_;
};
} // namespace spdy
diff --git a/quiche/spdy/core/hpack/hpack_encoder_test.cc b/quiche/spdy/core/hpack/hpack_encoder_test.cc
index 4b609c4..1d23317 100644
--- a/quiche/spdy/core/hpack/hpack_encoder_test.cc
+++ b/quiche/spdy/core/hpack/hpack_encoder_test.cc
@@ -325,6 +325,39 @@
EXPECT_GE(kInitialDynamicTableSize, encoder_.GetDynamicTableSize());
}
+TEST_P(HpackEncoderTestWithDefaultStrategy, WithoutCookieCrumbling) {
+ EXPECT_EQ(kInitialDynamicTableSize, encoder_.GetDynamicTableSize());
+ encoder_.SetHeaderListener(
+ [this](absl::string_view name, absl::string_view value) {
+ this->SaveHeaders(name, value);
+ });
+ encoder_.DisableCookieCrumbling();
+
+ const std::vector<std::pair<absl::string_view, absl::string_view>>
+ header_list = {{"cookie", "val1; val2;val3"},
+ {":path", "/home"},
+ {"accept", "text/html, text/plain,application/xml"},
+ {"cookie", "val4"},
+ {"withnul", absl::string_view("one\0two", 7)}};
+ ExpectNonIndexedLiteralWithNameIndex(peer_.table()->GetByName(":path"),
+ "/home");
+ ExpectIndexedLiteral(peer_.table()->GetByName("cookie"), "val1; val2;val3");
+ ExpectIndexedLiteral(peer_.table()->GetByName("accept"),
+ "text/html, text/plain,application/xml");
+ ExpectIndexedLiteral(peer_.table()->GetByName("cookie"), "val4");
+ ExpectIndexedLiteral("withnul", absl::string_view("one\0two", 7));
+
+ CompareWithExpectedEncoding(header_list);
+ EXPECT_THAT(
+ headers_observed_,
+ ElementsAre(Pair(":path", "/home"), Pair("cookie", "val1; val2;val3"),
+ Pair("accept", "text/html, text/plain,application/xml"),
+ Pair("cookie", "val4"),
+ Pair("withnul", absl::string_view("one\0two", 7))));
+ // Insertions and evictions have happened over the course of the test.
+ EXPECT_GE(kInitialDynamicTableSize, encoder_.GetDynamicTableSize());
+}
+
TEST_P(HpackEncoderTestWithDefaultStrategy, DynamicTableGrows) {
EXPECT_EQ(kInitialDynamicTableSize, encoder_.GetDynamicTableSize());
peer_.table()->SetMaxSize(4096);
@@ -446,6 +479,15 @@
CompareWithExpectedEncoding(headers);
}
+TEST_P(HpackEncoderTest, CookieHeaderIsNotCrumbled) {
+ encoder_.DisableCookieCrumbling();
+ ExpectIndexedLiteral(peer_.table()->GetByName("cookie"), "a=bb; c=dd; e=ff");
+
+ Http2HeaderBlock headers;
+ headers["cookie"] = "a=bb; c=dd; e=ff";
+ CompareWithExpectedEncoding(headers);
+}
+
TEST_P(HpackEncoderTest, MultiValuedHeadersNotCrumbled) {
ExpectIndexedLiteral("foo", "bar, baz");
Http2HeaderBlock headers;
