diff --git a/quiche/spdy/core/hpack/hpack_encoder.cc b/quiche/spdy/core/hpack/hpack_encoder.cc index 0891478b5..1fff60d81 100644 --- a/quiche/spdy/core/hpack/hpack_encoder.cc +++ b/quiche/spdy/core/hpack/hpack_encoder.cc @@ -86,7 +86,8 @@ HpackEncoder::HpackEncoder() 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 @@ std::string HpackEncoder::EncodeHeaderBlock( // 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 @@ HpackEncoder::Encoderator::Encoderator(const Http2HeaderBlock& header_set, // 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 @@ HpackEncoder::Encoderator::Encoderator(const Representations& representations, : 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 7eacfb315..35007c674 100644 --- a/quiche/spdy/core/hpack/hpack_encoder.h +++ b/quiche/spdy/core/hpack/hpack_encoder.h @@ -97,6 +97,12 @@ class QUICHE_EXPORT HpackEncoder { 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 @@ class QUICHE_EXPORT HpackEncoder { 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 4b609c469..1d23317cc 100644 --- a/quiche/spdy/core/hpack/hpack_encoder_test.cc +++ b/quiche/spdy/core/hpack/hpack_encoder_test.cc @@ -325,6 +325,39 @@ TEST_P(HpackEncoderTestWithDefaultStrategy, EncodeRepresentations) { 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> + 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 @@ TEST_P(HpackEncoderTest, CookieHeaderIsCrumbled) { 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;