commit 992004de8fef4e5dbabe18a444998ffe6aa98bd1
author QUICHE team <quiche-dev@google.com> Thu Dec 12 09:40:29 2019 -0800
committer Copybara-Service <copybara-worker@google.com> Thu Dec 12 09:41:08 2019 -0800
tree 0bff8f37efd72892202fc4c07829b48178b136a7
parent 22781cbb2ffe0c61194b6874a8e16f5a08530b30

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

spdy/core/hpack/hpack_encoder.cc

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, &regular_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, &regular_headers_)
-                      : GatherRepresentation(header, &regular_headers_);
+      DecomposeRepresentation(header, &regular_headers_);
     }
   }
   header_it_ = std::make_unique<RepresentationIterator>(pseudo_headers_,

spdy/core/hpack/hpack_encoder.h

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_;
 

spdy/core/hpack/hpack_encoder_test.cc

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"),