HPACK: Check pseudo-header order in decoder.

Treat a header block with a pseudo-header field following a regular one as
malformed, as required by HTTP/2 draft-14 Section 8.1.2.1.

This lands server change 74067938 by bnc.

BUG=400336

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

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

net/spdy/hpack_decoder.cc

@@ -22,6 +22,7 @@ const char kCookieKey[] = "cookie";
 
 HpackDecoder::HpackDecoder(const HpackHuffmanTable& table)
     : max_string_literal_size_(kDefaultMaxStringLiteralSize),
+      regular_header_seen_(false),
       huffman_table_(table) {}
 
 HpackDecoder::~HpackDecoder() {}
@@ -44,6 +45,7 @@ bool HpackDecoder::HandleControlFrameHeadersData(SpdyStreamId id,
 bool HpackDecoder::HandleControlFrameHeadersComplete(SpdyStreamId id) {
   HpackInputStream input_stream(max_string_literal_size_,
                                 headers_block_buffer_);
+  regular_header_seen_ = false;
   while (input_stream.HasMoreData()) {
     if (!DecodeNextOpcode(&input_stream)) {
       headers_block_buffer_.clear();
@@ -60,10 +62,19 @@ bool HpackDecoder::HandleControlFrameHeadersComplete(SpdyStreamId id) {
   return true;
 }
 
-void HpackDecoder::HandleHeaderRepresentation(StringPiece name,
+bool HpackDecoder::HandleHeaderRepresentation(StringPiece name,
                                               StringPiece value) {
   typedef std::pair<std::map<string, string>::iterator, bool> InsertResult;
 
+  // Fail if pseudo-header follows regular header.
+  if (name.size() > 0) {
+    if (name[0] == kPseudoHeaderPrefix) {
+      if (regular_header_seen_) return false;
+    } else {
+      regular_header_seen_ = true;
+    }
+  }
+
   if (name == kCookieKey) {
     if (cookie_value_.empty()) {
       cookie_value_.assign(value.data(), value.size());
@@ -81,6 +92,7 @@ void HpackDecoder::HandleHeaderRepresentation(StringPiece name,
                                   value.end());
     }
   }
+  return true;
 }
 
 bool HpackDecoder::DecodeNextOpcode(HpackInputStream* input_stream) {
@@ -131,8 +143,7 @@ bool HpackDecoder::DecodeNextIndexedHeader(HpackInputStream* input_stream) {
   if (entry == NULL)
     return false;
 
-  HandleHeaderRepresentation(entry->name(), entry->value());
-  return true;
+  return HandleHeaderRepresentation(entry->name(), entry->value());
 }
 
 bool HpackDecoder::DecodeNextLiteralHeader(HpackInputStream* input_stream,
@@ -145,7 +156,7 @@ bool HpackDecoder::DecodeNextLiteralHeader(HpackInputStream* input_stream,
   if (!DecodeNextStringLiteral(input_stream, false, &value))
     return false;
 
-  HandleHeaderRepresentation(name, value);
+  if (!HandleHeaderRepresentation(name, value)) return false;
 
   if (!should_index)
     return true;

net/spdy/hpack_decoder.h

@@ -77,9 +77,13 @@ class NET_EXPORT_PRIVATE HpackDecoder {
   //    Note that this may be too accomodating, as the sender's HTTP2 layer
   //    should have already joined and delimited these values.
   //
+  // Returns false if a pseudo-header field follows a regular header one, which
+  // MUST be treated as malformed, as per sections 8.1.2.1. of the HTTP2 draft
+  // specification.
+  //
   // TODO(jgraettinger): This method will eventually emit to the
   // SpdyHeadersHandlerInterface visitor.
-  void HandleHeaderRepresentation(base::StringPiece name,
+  bool HandleHeaderRepresentation(base::StringPiece name,
                                   base::StringPiece value);
 
   const uint32 max_string_literal_size_;
@@ -94,6 +98,9 @@ class NET_EXPORT_PRIVATE HpackDecoder {
   std::string headers_block_buffer_;
   std::map<std::string, std::string> decoded_block_;
 
+  // Flag to keep track of having seen a regular header field.
+  bool regular_header_seen_;
+
   // Huffman table to be applied to decoded Huffman literals,
   // and scratch space for storing those decoded literals.
   const HpackHuffmanTable& huffman_table_;

net/spdy/hpack_decoder_test.cc

@@ -257,6 +257,23 @@ TEST_F(HpackDecoderTest, InvalidIndexedHeader) {
   EXPECT_FALSE(DecodeHeaderBlock(StringPiece("\xbe", 1)));
 }
 
+// Test that a header block with a pseudo-header field following a regular one
+// is treated as malformed.  (HTTP2 draft-14 8.1.2.1., HPACK draft-09 3.1.)
+
+TEST_F(HpackDecoderTest, InvalidPseudoHeaderPositionStatic) {
+  // Okay: ":path" (static entry 4) followed by "allow" (static entry 20).
+  EXPECT_TRUE(DecodeHeaderBlock(a2b_hex("8494")));
+  // Malformed: "allow" (static entry 20) followed by ":path" (static entry 4).
+  EXPECT_FALSE(DecodeHeaderBlock(a2b_hex("9484")));
+}
+
+TEST_F(HpackDecoderTest, InvalidPseudoHeaderPositionLiteral) {
+  // Okay: literal ":bar" followed by literal "foo".
+  EXPECT_TRUE(DecodeHeaderBlock(a2b_hex("40043a626172004003666f6f00")));
+  // Malformed: literal "foo" followed by literal ":bar".
+  EXPECT_FALSE(DecodeHeaderBlock(a2b_hex("4003666f6f0040043a62617200")));
+}
+
 TEST_F(HpackDecoderTest, ContextUpdateMaximumSize) {
   EXPECT_EQ(kDefaultHeaderTableSizeSetting,
             decoder_peer_.header_table()->max_size());