HPACK: encode :pseudo and indexed headers first.

HTTP/2 header compression HPACK draft-09 requires pseudo-headers (names
starting with ":") to be encoded before regular headers.  Also, headers that
are in the header table are emitted before others, to avoid eviction before
they could be used.

This lands server change 74039742 by bnc.

BUG=395635

Review URL: https://codereview.chromium.org/537663002

Cr-Commit-Position: refs/heads/master@{#293290}

net/spdy/hpack_constants.h

@@ -73,6 +73,9 @@ NET_EXPORT_PRIVATE std::vector<HpackHuffmanSymbol> HpackHuffmanCode();
 // threads. This function is thread-safe.
 NET_EXPORT_PRIVATE const HpackHuffmanTable& ObtainHpackHuffmanTable();
 
+// Pseudo-headers start with a colon.  (HTTP2 8.1.2.1., HPACK 3.1.)
+const char kPseudoHeaderPrefix = ':';
+
 }  // namespace net
 
 #endif  // NET_SPDY_HPACK_CONSTANTS_H_

net/spdy/hpack_encoder.cc

@@ -27,33 +27,65 @@ HpackEncoder::~HpackEncoder() {}
 
 bool HpackEncoder::EncodeHeaderSet(const std::map<string, string>& header_set,
                                    string* output) {
-  // Flatten & crumble headers into an ordered list of representations.
-  Representations full_set;
+  // Separate header set into pseudo-headers and regular headers.
+  Representations pseudo_headers;
+  Representations regular_headers;
   for (std::map<string, string>::const_iterator it = header_set.begin();
        it != header_set.end(); ++it) {
     if (it->first == "cookie") {
-      // |CookieToCrumbs()| produces ordered crumbs.
-      CookieToCrumbs(*it, &full_set);
+      // Note that there can only be one "cookie" header, because header_set is
+      // a map.
+      CookieToCrumbs(*it, &regular_headers);
+    } else if (it->first[0] == kPseudoHeaderPrefix) {
+      pseudo_headers.push_back(make_pair(
+          StringPiece(it->first), StringPiece(it->second)));
     } else {
-      // Note std::map guarantees representations are ordered.
-      full_set.push_back(make_pair(
+      regular_headers.push_back(make_pair(
           StringPiece(it->first), StringPiece(it->second)));
     }
   }
 
-  // Walk this ordered list and encode entries.
-  for (Representations::const_iterator it = full_set.begin();
-       it != full_set.end(); ++it) {
+  // Encode pseudo-headers.
+  for (Representations::const_iterator it = pseudo_headers.begin();
+       it != pseudo_headers.end(); ++it) {
+    HpackEntry* entry = header_table_.GetByNameAndValue(it->first, it->second);
+    if (entry != NULL) {
+      EmitIndex(entry);
+    } else {
+      if (it->first == ":authority") {
+        // :authority is always present and rarely changes, and has moderate
+        // length, therefore it makes a lot of sense to index (insert in the
+        // header table).
+        EmitIndexedLiteral(*it);
+      } else {
+        // Most common pseudo-header fields are represented in the static table,
+        // while uncommon ones are small, so do not index them.
+        EmitNonIndexedLiteral(*it);
+      }
+    }
+  }
+
+  // Encode regular headers that are already in the header table first,
+  // save the rest into another vector.  This way we avoid evicting an entry
+  // from the header table before it can be used.
+  Representations literal_headers;
+  for (Representations::const_iterator it = regular_headers.begin();
+       it != regular_headers.end(); ++it) {
     HpackEntry* entry = header_table_.GetByNameAndValue(it->first, it->second);
     if (entry != NULL) {
       EmitIndex(entry);
     } else {
-      // TODO(bnc): if another entry in the header table is about to be evicted
-      // but it appears in the header list, emit that by index first.
-      EmitIndexedLiteral(*it);
+      literal_headers.push_back(*it);
     }
   }
 
+  // Encode the remaining header fields, while inserting them in the header
+  // table.
+  for (Representations::const_iterator it = literal_headers.begin();
+       it != literal_headers.end(); ++it) {
+    EmitIndexedLiteral(*it);
+  }
+
   output_stream_.TakeString(output);
   return true;
 }
@@ -140,8 +172,8 @@ void HpackEncoder::CookieToCrumbs(const Representation& cookie,
                                   Representations* out) {
   size_t prior_size = out->size();
 
-  // See Section 8.1.3.4 "Compressing the Cookie Header Field" in the HTTP/2
-  // specification at http://tools.ietf.org/html/draft-ietf-httpbis-http2-11
+  // See Section 8.1.2.5. "Compressing the Cookie Header Field" in the HTTP/2
+  // specification at https://tools.ietf.org/html/draft-ietf-httpbis-http2-14.
   // Cookie values are split into individually-encoded HPACK representations.
   for (size_t pos = 0;;) {
     size_t end = cookie.second.find(";", pos);

net/spdy/hpack_encoder_test.cc

@@ -311,20 +311,20 @@ TEST_F(HpackEncoderTest, MultipleEncodingPasses) {
     headers["cookie"] = "c=dd; e=ff";
 
     ExpectIndex(IndexOf(cookie_c_));
+    // key1 by index.
+    ExpectIndex(65);
+    // key2 by index.
+    ExpectIndex(64);
     // This cookie evicts |key1| from the header table.
     ExpectIndexedLiteral(peer_.table()->GetByName("cookie"), "e=ff");
-    // |key1| is inserted to the header table, which evicts |key2|.
-    ExpectIndexedLiteral("key1", "value1");
-    // |key2| is inserted to the header table, which evicts |cookie_a|.
-    ExpectIndexedLiteral("key2", "value2");
 
     CompareWithExpectedEncoding(headers);
   }
   // Header table is:
-  // 65: cookie: c=dd
-  // 64: cookie: e=ff
-  // 63: key1: value1
-  // 62: key2: value2
+  // 65: key2: value2
+  // 64: cookie: a=bb
+  // 63: cookie: c=dd
+  // 62: cookie: e=ff
   // Pass 3.
   {
     map<string, string> headers;
@@ -332,14 +332,39 @@ TEST_F(HpackEncoderTest, MultipleEncodingPasses) {
     headers["key3"] = "value3";
     headers["cookie"] = "e=ff";
 
-    ExpectIndex(64);
-    ExpectIndex(63);
+    // cookie: e=ff by index.
+    ExpectIndex(62);
+    ExpectIndexedLiteral("key1", "value1");
     ExpectIndexedLiteral("key3", "value3");
 
     CompareWithExpectedEncoding(headers);
   }
 }
 
+TEST_F(HpackEncoderTest, PseudoHeadersFirst) {
+  map<string, string> headers;
+  // A pseudo-header to be indexed.
+  headers[":authority"] = "www.example.com";
+  // A pseudo-header that should not be indexed.
+  headers[":path"] = "/spam/eggs.html";
+  // A regular header which precedes ":" alphabetically, should still be encoded
+  // after pseudo-headers.
+  headers["-foo"] = "bar";
+  headers["foo"] = "bar";
+  headers["cookie"] = "c=dd";
+
+  // Pseudo-headers are encoded in alphabetical order.
+  ExpectIndexedLiteral(peer_.table()->GetByName(":authority"),
+                       "www.example.com");
+  ExpectNonIndexedLiteral(":path", "/spam/eggs.html");
+  // Regular headers in the header table are encoded first.
+  ExpectIndex(IndexOf(cookie_a_));
+  // Regular headers not in the header table are encoded, in alphabetical order.
+  ExpectIndexedLiteral("-foo", "bar");
+  ExpectIndexedLiteral("foo", "bar");
+  CompareWithExpectedEncoding(headers);
+}
+
 TEST_F(HpackEncoderTest, CookieToCrumbs) {
   test::HpackEncoderPeer peer(NULL);
   std::vector<StringPiece> out;

net/spdy/hpack_output_stream.cc

@@ -49,7 +49,7 @@ void HpackOutputStream::AppendBytes(StringPiece buffer) {
 }
 
 void HpackOutputStream::AppendUint32(uint32 I) {
-  // The algorithm below is adapted from the pseudocode in 4.1.1.
+  // The algorithm below is adapted from the pseudocode in 6.1.
   size_t N = 8 - bit_offset_;
   uint8 max_first_byte = static_cast<uint8>((1 << N) - 1);
   if (I < max_first_byte) {

net/spdy/hpack_round_trip_test.cc

@@ -102,7 +102,7 @@ TEST_F(HpackRoundTripTest, RequestFixtures) {
     headers[":path"] = "/";
     headers[":scheme"] = "http";
     headers["cache-control"] = "no-cache";
-    headers["cookie"] = "fizzle=fazzle; foo=bar";
+    headers["cookie"] = "foo=bar; fizzle=fazzle";
     EXPECT_TRUE(RoundTrip(headers));
   }
   {