gfe-relnote: Fixes HPACK encoding of SpdyHeaderBlocks when compression is disabled. No functional change in production. For HTTP/2, compression is always enabled in production. Shinkansen metadata uses a different HpackEncoder API that is not affected by this CL. PiperOrigin-RevId: 285208542 Change-Id: I8c755500bb4c1004d264c7663bc4d6308a0b06ce
diff --git a/spdy/core/hpack/hpack_encoder.cc b/spdy/core/hpack/hpack_encoder.cc index 39a4de8..368754e 100644 --- a/spdy/core/hpack/hpack_encoder.cc +++ b/spdy/core/hpack/hpack_encoder.cc
@@ -271,12 +271,6 @@ } } -// static -void HpackEncoder::GatherRepresentation(const Representation& header_field, - Representations* out) { - out->push_back(std::make_pair(header_field.first, header_field.second)); -} - // Iteratively encodes a SpdyHeaderBlock. class HpackEncoder::Encoderator : public ProgressiveEncoder { public: @@ -306,7 +300,6 @@ HpackEncoder* encoder) : encoder_(encoder), has_next_(true) { // Separate header set into pseudo-headers and regular headers. - const bool use_compression = encoder_->enable_compression_; bool found_cookie = false; for (const auto& header : header_set) { if (!found_cookie && header.first == "cookie") { @@ -316,11 +309,9 @@ CookieToCrumbs(header, ®ular_headers_); } else if (!header.first.empty() && header.first[0] == kPseudoHeaderPrefix) { - use_compression ? DecomposeRepresentation(header, &pseudo_headers_) - : GatherRepresentation(header, &pseudo_headers_); + DecomposeRepresentation(header, &pseudo_headers_); } else { - use_compression ? DecomposeRepresentation(header, ®ular_headers_) - : GatherRepresentation(header, ®ular_headers_); + DecomposeRepresentation(header, ®ular_headers_); } } header_it_ = std::make_unique<RepresentationIterator>(pseudo_headers_,
diff --git a/spdy/core/hpack/hpack_encoder.h b/spdy/core/hpack/hpack_encoder.h index d3bfc95..03c9d92 100644 --- a/spdy/core/hpack/hpack_encoder.h +++ b/spdy/core/hpack/hpack_encoder.h
@@ -137,10 +137,6 @@ static void DecomposeRepresentation(const Representation& header_field, Representations* out); - // Gathers headers without crumbling. Used when compression is not enabled. - static void GatherRepresentation(const Representation& header_field, - Representations* out); - HpackHeaderTable header_table_; HpackOutputStream output_stream_;
diff --git a/spdy/core/hpack/hpack_encoder_test.cc b/spdy/core/hpack/hpack_encoder_test.cc index fb09301..b18abad 100644 --- a/spdy/core/hpack/hpack_encoder_test.cc +++ b/spdy/core/hpack/hpack_encoder_test.cc
@@ -426,8 +426,7 @@ ExpectNonIndexedLiteral(":path", "/index.html"); ExpectNonIndexedLiteral("cookie", "foo=bar"); ExpectNonIndexedLiteral("cookie", "baz=bing"); - if (strategy_ != kDefault) { - // BUG: encodes as a \0-delimited value. Should be separate entries. + if (strategy_ == kRepresentations) { ExpectNonIndexedLiteral("hello", std::string("goodbye\0aloha", 13)); } else { ExpectNonIndexedLiteral("hello", "goodbye"); @@ -444,9 +443,7 @@ CompareWithExpectedEncoding(headers); - if (strategy_ != kDefault) { - // BUG: value for "hello" encodes as \0-delimited. Should be separate - // entries. + if (strategy_ == kRepresentations) { EXPECT_THAT( headers_observed_, ElementsAre(Pair(":path", "/index.html"), Pair("cookie", "foo=bar"),