diff options
author | jgraettinger@chromium.org <jgraettinger@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-03-10 20:43:27 +0000 |
---|---|---|
committer | jgraettinger@chromium.org <jgraettinger@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-03-10 20:43:27 +0000 |
commit | 31f8a14090bc8d68accd26e5cb4c8f1223ae7691 (patch) | |
tree | 2ded6f09950e26e0cc85062ba52d56a095c1db77 /net | |
parent | 328b27c33f21a1f401114ddfe7654c7b9565c06c (diff) | |
download | chromium_src-31f8a14090bc8d68accd26e5cb4c8f1223ae7691.zip chromium_src-31f8a14090bc8d68accd26e5cb4c8f1223ae7691.tar.gz chromium_src-31f8a14090bc8d68accd26e5cb4c8f1223ae7691.tar.bz2 |
Implement HPACK draft '06 maximum size context update
HpackEncodingContext now separately tracks maximum table size setting,
and maximum table size. Handlers for each context update have been added,
and clear-reference-set update semantics have been removed from
ProcessIndexedHeader().
HpackDecoder now handles maximum-size update opcodes.
Also refactored ProcessNextHeaderRepresentation() into
opcode-specific handlers for readability.
Added test peers to HpackEncodingContext & HpackDecoder (to simplify inspection
of the header table), and tests for new functionality.
This CL completes changes required for HPACK draft '06.
Update remaining references to '05.
This lands server change 61957108 by jgraettinger.
BUG=339578
Review URL: https://codereview.chromium.org/181543006
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@256029 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'net')
-rw-r--r-- | net/spdy/hpack_constants.h | 15 | ||||
-rw-r--r-- | net/spdy/hpack_decoder.cc | 144 | ||||
-rw-r--r-- | net/spdy/hpack_decoder.h | 39 | ||||
-rw-r--r-- | net/spdy/hpack_decoder_test.cc | 96 | ||||
-rw-r--r-- | net/spdy/hpack_encoder.h | 4 | ||||
-rw-r--r-- | net/spdy/hpack_encoding_context.cc | 46 | ||||
-rw-r--r-- | net/spdy/hpack_encoding_context.h | 35 | ||||
-rw-r--r-- | net/spdy/hpack_encoding_context_test.cc | 106 | ||||
-rw-r--r-- | net/spdy/hpack_entry.h | 3 | ||||
-rw-r--r-- | net/spdy/hpack_header_table.cc | 5 | ||||
-rw-r--r-- | net/spdy/hpack_header_table.h | 3 | ||||
-rw-r--r-- | net/spdy/hpack_input_stream.cc | 2 | ||||
-rw-r--r-- | net/spdy/hpack_input_stream.h | 2 | ||||
-rw-r--r-- | net/spdy/hpack_output_stream.h | 3 | ||||
-rw-r--r-- | net/spdy/hpack_string_util.h | 3 |
15 files changed, 353 insertions, 153 deletions
diff --git a/net/spdy/hpack_constants.h b/net/spdy/hpack_constants.h index ecae49e..a1f2d34 100644 --- a/net/spdy/hpack_constants.h +++ b/net/spdy/hpack_constants.h @@ -29,6 +29,8 @@ struct HpackHuffmanSymbol { uint16 id; }; +const uint32 kDefaultHeaderTableSizeSetting = 4096; + // The marker for a string literal that is stored unmodified (i.e., // without Huffman encoding) (from 4.1.2). const HpackPrefix kStringLiteralIdentityEncoded = { 0x0, 1 }; @@ -37,6 +39,19 @@ const HpackPrefix kStringLiteralIdentityEncoded = { 0x0, 1 }; // encoding (from 4.1.2). const HpackPrefix kStringLiteralHuffmanEncoded = { 0x1, 1 }; +// The opcode for an encoding context update (from 4.2). +// This is an indexed header representation with special index zero, +// and as such this opcode must be tested prior to |kIndexedOpcode|. +const HpackPrefix kEncodingContextOpcode = { 0x80, 8 }; + +// Follows an |kEncodingContextOpcode| to indicate the reference set should be +// cleared. (Section 4.4). +const HpackPrefix kEncodingContextEmptyReferenceSet = { 0x80, 8 }; + +// Follows an |kEncodingContextOpcode| to indicate the encoder is using a new +// maximum headers table size. (Section 4.4). +const HpackPrefix kEncodingContextNewMaximumSize = { 0x0, 1 }; + // The opcode for an indexed header field (from 4.2). const HpackPrefix kIndexedOpcode = { 0x1, 1 }; diff --git a/net/spdy/hpack_decoder.cc b/net/spdy/hpack_decoder.cc index f44bcca5..72d8be6 100644 --- a/net/spdy/hpack_decoder.cc +++ b/net/spdy/hpack_decoder.cc @@ -5,6 +5,7 @@ #include "net/spdy/hpack_decoder.h" #include "base/basictypes.h" +#include "base/logging.h" #include "net/spdy/hpack_constants.h" #include "net/spdy/hpack_output_stream.h" @@ -20,8 +21,8 @@ HpackDecoder::HpackDecoder(const HpackHuffmanTable& table, HpackDecoder::~HpackDecoder() {} -void HpackDecoder::SetMaxHeadersSize(uint32 max_size) { - context_.SetMaxSize(max_size); +void HpackDecoder::ApplyHeaderTableSizeSetting(uint32 max_size) { + context_.ApplyHeaderTableSizeSetting(max_size); } bool HpackDecoder::DecodeHeaderSet(StringPiece input, @@ -29,7 +30,7 @@ bool HpackDecoder::DecodeHeaderSet(StringPiece input, HpackInputStream input_stream(max_string_literal_size_, input); while (input_stream.HasMoreData()) { // May add to |header_list|. - if (!ProcessNextHeaderRepresentation(&input_stream, header_list)) + if (!DecodeNextOpcode(&input_stream, header_list)) return false; } @@ -48,82 +49,105 @@ bool HpackDecoder::DecodeHeaderSet(StringPiece input, return true; } -bool HpackDecoder::ProcessNextHeaderRepresentation( - HpackInputStream* input_stream, HpackHeaderPairVector* header_list) { - // Touches are used below to track which headers have been emitted. - - // Implements 4.2. Indexed Header Field Representation. +bool HpackDecoder::DecodeNextOpcode(HpackInputStream* input_stream, + HpackHeaderPairVector* header_list) { + // Implements 4.4: Encoding context update. Context updates are a special-case + // of indexed header, and must be tested prior to |kIndexedOpcode| below. + if (input_stream->MatchPrefixAndConsume(kEncodingContextOpcode)) { + return DecodeNextContextUpdate(input_stream); + } + // Implements 4.2: Indexed Header Field Representation. if (input_stream->MatchPrefixAndConsume(kIndexedOpcode)) { - uint32 index_or_zero = 0; - if (!input_stream->DecodeNextUint32(&index_or_zero)) - return false; + return DecodeNextIndexedHeader(input_stream, header_list); + } + // Implements 4.3.1: Literal Header Field without Indexing. + if (input_stream->MatchPrefixAndConsume(kLiteralNoIndexOpcode)) { + return DecodeNextLiteralHeader(input_stream, false, header_list); + } + // Implements 4.3.2: Literal Header Field with Incremental Indexing. + if (input_stream->MatchPrefixAndConsume(kLiteralIncrementalIndexOpcode)) { + return DecodeNextLiteralHeader(input_stream, true, header_list); + } + // Unrecognized opcode. + return false; +} - if (index_or_zero > context_.GetEntryCount()) +bool HpackDecoder::DecodeNextContextUpdate(HpackInputStream* input_stream) { + if (input_stream->MatchPrefixAndConsume(kEncodingContextEmptyReferenceSet)) { + return context_.ProcessContextUpdateEmptyReferenceSet(); + } + if (input_stream->MatchPrefixAndConsume(kEncodingContextNewMaximumSize)) { + uint32 size = 0; + if (!input_stream->DecodeNextUint32(&size)) { return false; - - bool emitted = false; - if (index_or_zero > 0) { - uint32 index = index_or_zero; - // The index will be put into the reference set. - if (!context_.IsReferencedAt(index)) { - header_list->push_back( - HpackHeaderPair(context_.GetNameAt(index).as_string(), - context_.GetValueAt(index).as_string())); - emitted = true; - } } - - uint32 new_index = 0; - std::vector<uint32> removed_referenced_indices; - context_.ProcessIndexedHeader( - index_or_zero, &new_index, &removed_referenced_indices); - - if (emitted && new_index > 0) - context_.AddTouchesAt(new_index, 0); - - return true; + return context_.ProcessContextUpdateNewMaximumSize(size); } + // Unrecognized encoding context update. + return false; +} - // Implements 4.3.1. Literal Header Field without Indexing. - if (input_stream->MatchPrefixAndConsume(kLiteralNoIndexOpcode)) { - StringPiece name; - if (!DecodeNextName(input_stream, &name)) - return false; +bool HpackDecoder::DecodeNextIndexedHeader(HpackInputStream* input_stream, + HpackHeaderPairVector* header_list) { + uint32 index = 0; + if (!input_stream->DecodeNextUint32(&index)) + return false; - StringPiece value; - if (!DecodeNextStringLiteral(input_stream, false, &value)) - return false; + // If index == 0, |kEncodingContextOpcode| would have matched. + CHECK_NE(index, 0u); + if (index > context_.GetEntryCount()) + return false; + + bool emitted = false; + // The index will be put into the reference set. + if (!context_.IsReferencedAt(index)) { header_list->push_back( - HpackHeaderPair(name.as_string(), value.as_string())); - return true; + HpackHeaderPair(context_.GetNameAt(index).as_string(), + context_.GetValueAt(index).as_string())); + emitted = true; } - // Implements 4.3.2. Literal Header Field with Incremental Indexing. - if (input_stream->MatchPrefixAndConsume(kLiteralIncrementalIndexOpcode)) { - StringPiece name; - if (!DecodeNextName(input_stream, &name)) - return false; + uint32 new_index = 0; + std::vector<uint32> removed_referenced_indices; + if (!context_.ProcessIndexedHeader( + index, &new_index, &removed_referenced_indices)) { + return false; + } + if (emitted && new_index > 0) + context_.AddTouchesAt(new_index, 0); - StringPiece value; - if (!DecodeNextStringLiteral(input_stream, false, &value)) - return false; + return true; +} - header_list->push_back( - HpackHeaderPair(name.as_string(), value.as_string())); +bool HpackDecoder::DecodeNextLiteralHeader(HpackInputStream* input_stream, + bool should_index, + HpackHeaderPairVector* header_list) { + StringPiece name; + if (!DecodeNextName(input_stream, &name)) + return false; - uint32 new_index = 0; - std::vector<uint32> removed_referenced_indices; - context_.ProcessLiteralHeaderWithIncrementalIndexing( - name, value, &new_index, &removed_referenced_indices); + StringPiece value; + if (!DecodeNextStringLiteral(input_stream, false, &value)) + return false; - if (new_index > 0) - context_.AddTouchesAt(new_index, 0); + header_list->push_back( + HpackHeaderPair(name.as_string(), value.as_string())); + if (!should_index) return true; + + uint32 new_index = 0; + std::vector<uint32> removed_referenced_indices; + if (!context_.ProcessLiteralHeaderWithIncrementalIndexing( + name, value, &new_index, &removed_referenced_indices)) { + return false; } - return false; + if (new_index > 0) + context_.AddTouchesAt(new_index, 0); + + return true; } bool HpackDecoder::DecodeNextName( diff --git a/net/spdy/hpack_decoder.h b/net/spdy/hpack_decoder.h index d8933c4..dbb4728 100644 --- a/net/spdy/hpack_decoder.h +++ b/net/spdy/hpack_decoder.h @@ -15,23 +15,30 @@ #include "net/spdy/hpack_encoding_context.h" #include "net/spdy/hpack_input_stream.h" -namespace net { - // An HpackDecoder decodes header sets as outlined in -// http://tools.ietf.org/html/draft-ietf-httpbis-header-compression-05 +// http://tools.ietf.org/html/draft-ietf-httpbis-header-compression-06 + +namespace net { class HpackHuffmanTable; +namespace test { +class HpackDecoderPeer; +} // namespace test + class NET_EXPORT_PRIVATE HpackDecoder { public: - // |table| is an initialized HPACK request or response Huffman table, - // having an externally-managed lifetime which spans beyond HpackDecoder. + friend class test::HpackDecoderPeer; + + // |table| is an initialized HPACK Huffman table, having an + // externally-managed lifetime which spans beyond HpackDecoder. explicit HpackDecoder(const HpackHuffmanTable& table, uint32 max_string_literal_size); ~HpackDecoder(); - // Set the maximum size of the headers table used by the decoder. - void SetMaxHeadersSize(uint32 max_size); + // Called upon acknowledgement of SETTINGS_HEADER_TABLE_SIZE. + // See HpackEncodingContext::ApplyHeaderTableSizeSetting(). + void ApplyHeaderTableSizeSetting(uint32 max_size); // Decodes the given string into the given header set. Returns // whether or not the decoding was successful. @@ -58,13 +65,17 @@ class NET_EXPORT_PRIVATE HpackDecoder { const HpackHuffmanTable& huffman_table_; std::string huffman_key_buffer_, huffman_value_buffer_; - // Tries to process the next header representation and maybe emit - // headers into |header_list| according to it. Returns true if - // successful, or false if an error was encountered. - bool ProcessNextHeaderRepresentation( - HpackInputStream* input_stream, - HpackHeaderPairVector* header_list); - + // Handlers for decoding HPACK opcodes and header representations + // (or parts thereof). These methods emit headers into + // |header_list|, and return true on success and false on error. + bool DecodeNextOpcode(HpackInputStream* input_stream, + HpackHeaderPairVector* header_list); + bool DecodeNextContextUpdate(HpackInputStream* input_stream); + bool DecodeNextIndexedHeader(HpackInputStream* input_stream, + HpackHeaderPairVector* header_list); + bool DecodeNextLiteralHeader(HpackInputStream* input_stream, + bool should_index, + HpackHeaderPairVector* header_list); bool DecodeNextName(HpackInputStream* input_stream, base::StringPiece* next_name); bool DecodeNextStringLiteral(HpackInputStream* input_stream, diff --git a/net/spdy/hpack_decoder_test.cc b/net/spdy/hpack_decoder_test.cc index c7b1961..4663c62 100644 --- a/net/spdy/hpack_decoder_test.cc +++ b/net/spdy/hpack_decoder_test.cc @@ -12,11 +12,40 @@ #include "base/strings/string_piece.h" #include "net/spdy/hpack_encoder.h" #include "net/spdy/hpack_input_stream.h" +#include "net/spdy/hpack_output_stream.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" namespace net { +namespace test { + +class HpackEncodingContextPeer { + public: + explicit HpackEncodingContextPeer(const HpackEncodingContext& context) + : context_(context) {} + const HpackHeaderTable& header_table() { + return context_.header_table_; + } + + private: + const HpackEncodingContext& context_; +}; + +class HpackDecoderPeer { + public: + explicit HpackDecoderPeer(const HpackDecoder& decoder) + : decoder_(decoder) {} + HpackEncodingContextPeer context_peer() { + return HpackEncodingContextPeer(decoder_.context_); + } + + private: + const HpackDecoder& decoder_; +}; + +} // namespace test + namespace { using base::StringPiece; @@ -30,10 +59,9 @@ const size_t kLiteralBound = 1024; class HpackDecoderTest : public ::testing::Test { protected: HpackDecoderTest() - : decoder_(huffman_table_, kLiteralBound) {} - - virtual void SetUp() { - } + : decoder_(huffman_table_, kLiteralBound), + decoder_peer_(decoder_), + context_peer_(decoder_peer_.context_peer()) {} // Utility function to decode a string into a header set, assuming // that the emitted headers have unique names. @@ -49,6 +77,8 @@ class HpackDecoderTest : public ::testing::Test { HpackHuffmanTable huffman_table_; HpackDecoder decoder_; + test::HpackDecoderPeer decoder_peer_; + test::HpackEncodingContextPeer context_peer_; }; // Decoding an encoded name with a valid string literal should work. @@ -118,9 +148,51 @@ TEST_F(HpackDecoderTest, IndexedHeaderBasic) { EXPECT_EQ(expected_header_set2, header_set2); } -// Decoding an indexed header with index 0 should clear the reference -// set. -TEST_F(HpackDecoderTest, IndexedHeaderZero) { +// Test a too-large indexed header. +TEST_F(HpackDecoderTest, InvalidIndexedHeader) { + HpackHeaderPairVector header_list; + // High-bit set, and a prefix of one more than the number of static entries. + EXPECT_FALSE(decoder_.DecodeHeaderSet(StringPiece("\xbd", 1), &header_list)); +} + +TEST_F(HpackDecoderTest, ContextUpdateMaximumSize) { + HpackHeaderPairVector header_list; + EXPECT_EQ(kDefaultHeaderTableSizeSetting, + context_peer_.header_table().max_size()); + { + // Maximum-size update with size 126. Succeeds. + EXPECT_TRUE(decoder_.DecodeHeaderSet(StringPiece("\x80\x7e", 2), + &header_list)); + EXPECT_EQ(126u, context_peer_.header_table().max_size()); + } + string input; + { + // Maximum-size update with kDefaultHeaderTableSizeSetting. Succeeds. + HpackOutputStream output_stream(kuint32max); + output_stream.AppendBits(0x80, 8); // Context update. + output_stream.AppendBits(0x00, 1); // Size update. + output_stream.AppendUint32ForTest(kDefaultHeaderTableSizeSetting); + + output_stream.TakeString(&input); + EXPECT_TRUE(decoder_.DecodeHeaderSet(StringPiece(input), &header_list)); + EXPECT_EQ(kDefaultHeaderTableSizeSetting, + context_peer_.header_table().max_size()); + } + { + // Maximum-size update with kDefaultHeaderTableSizeSetting + 1. Fails. + HpackOutputStream output_stream(kuint32max); + output_stream.AppendBits(0x80, 8); // Context update. + output_stream.AppendBits(0x00, 1); // Size update. + output_stream.AppendUint32ForTest(kDefaultHeaderTableSizeSetting + 1); + + output_stream.TakeString(&input); + EXPECT_FALSE(decoder_.DecodeHeaderSet(StringPiece(input), &header_list)); + EXPECT_EQ(kDefaultHeaderTableSizeSetting, + context_peer_.header_table().max_size()); + } +} + +TEST_F(HpackDecoderTest, ContextUpdateClearReferenceSet) { // Toggle on a couple of headers. std::map<string, string> header_set1 = DecodeUniqueHeaderSet("\x82\x86"); @@ -129,9 +201,9 @@ TEST_F(HpackDecoderTest, IndexedHeaderZero) { expected_header_set1[":path"] = "/index.html"; EXPECT_EQ(expected_header_set1, header_set1); - // Toggle index 0 to clear the reference set. + // Send a context update to clear the reference set. std::map<string, string> header_set2 = - DecodeUniqueHeaderSet("\x80"); + DecodeUniqueHeaderSet("\x80\x80"); std::map<string, string> expected_header_set2; EXPECT_EQ(expected_header_set2, header_set2); } @@ -263,7 +335,7 @@ TEST_F(HpackDecoderTest, SectionD3RequestHuffmanExamples) { Pair(":scheme", "http"), Pair("cache-control", "no-cache"))); - // 80 | == Empty reference set == + // 8080 | == Empty reference set == // | idx = 0 // | flag = 1 // 85 | == Indexed - Add == @@ -291,7 +363,7 @@ TEST_F(HpackDecoderTest, SectionD3RequestHuffmanExamples) { // | custom-value // | -> custom-key: custom-value char third[] = - "\x80\x85\x8c\x8b\x84\x00\x88\x4e\xb0\x8b\x74\x97\x90\xfa\x7f\x89" + "\x80\x80\x85\x8c\x8b\x84\x00\x88\x4e\xb0\x8b\x74\x97\x90\xfa\x7f\x89" "\x4e\xb0\x8b\x74\x97\x9a\x17\xa8\xff"; header_set = DecodeUniqueHeaderSet(StringPiece(third, arraysize(third)-1)); @@ -309,7 +381,7 @@ TEST_F(HpackDecoderTest, SectionD5ResponseHuffmanExamples) { EXPECT_TRUE(huffman_table_.Initialize(&code[0], code.size())); } std::map<string, string> header_set; - decoder_.SetMaxHeadersSize(256); + decoder_.ApplyHeaderTableSizeSetting(256); // 08 | == Literal indexed == // | Indexed name (idx = 8) diff --git a/net/spdy/hpack_encoder.h b/net/spdy/hpack_encoder.h index ea15cc6b..92bdf6b3 100644 --- a/net/spdy/hpack_encoder.h +++ b/net/spdy/hpack_encoder.h @@ -16,8 +16,8 @@ namespace net { // An HpackEncoder encodes header sets as outlined in -// http://tools.ietf.org/html/draft-ietf-httpbis-header-compression-05 -// . +// http://tools.ietf.org/html/draft-ietf-httpbis-header-compression-06 + class NET_EXPORT_PRIVATE HpackEncoder { public: explicit HpackEncoder(uint32 max_string_literal_size); diff --git a/net/spdy/hpack_encoding_context.cc b/net/spdy/hpack_encoding_context.cc index 28d090e..44807ac 100644 --- a/net/spdy/hpack_encoding_context.cc +++ b/net/spdy/hpack_encoding_context.cc @@ -8,6 +8,7 @@ #include "base/logging.h" #include "base/macros.h" +#include "net/spdy/hpack_constants.h" #include "net/spdy/hpack_entry.h" namespace net { @@ -102,7 +103,8 @@ const size_t kStaticEntryCount = arraysize(kStaticTable); const uint32 HpackEncodingContext::kUntouched = HpackEntry::kUntouched; -HpackEncodingContext::HpackEncodingContext() {} +HpackEncodingContext::HpackEncodingContext() + : settings_header_table_size_(kDefaultHeaderTableSizeSetting) {} HpackEncodingContext::~HpackEncodingContext() {} @@ -164,32 +166,36 @@ void HpackEncodingContext::ClearTouchesAt(uint32 index) { header_table_.GetMutableEntry(index)->ClearTouches(); } -void HpackEncodingContext::SetMaxSize(uint32 max_size) { - header_table_.SetMaxSize(max_size); +void HpackEncodingContext::ApplyHeaderTableSizeSetting(uint32 size) { + settings_header_table_size_ = size; + if (size < header_table_.max_size()) { + // Implicit maximum-size context update. + CHECK(ProcessContextUpdateNewMaximumSize(size)); + } } -bool HpackEncodingContext::ProcessIndexedHeader( - uint32 index_or_zero, - uint32* new_index, - std::vector<uint32>* removed_referenced_indices) { - if (index_or_zero > GetEntryCount()) +bool HpackEncodingContext::ProcessContextUpdateNewMaximumSize(uint32 size) { + if (size > settings_header_table_size_) { return false; + } + header_table_.SetMaxSize(size); + return true; +} - if (index_or_zero == 0) { - *new_index = 0; - removed_referenced_indices->clear(); - // Empty the reference set. - for (size_t i = 1; i <= header_table_.GetEntryCount(); ++i) { - HpackEntry* entry = header_table_.GetMutableEntry(i); - if (entry->IsReferenced()) { - removed_referenced_indices->push_back(i); - entry->SetReferenced(false); - } +bool HpackEncodingContext::ProcessContextUpdateEmptyReferenceSet() { + for (size_t i = 1; i <= header_table_.GetEntryCount(); ++i) { + HpackEntry* entry = header_table_.GetMutableEntry(i); + if (entry->IsReferenced()) { + entry->SetReferenced(false); } - return true; } + return true; +} - uint32 index = index_or_zero; +bool HpackEncodingContext::ProcessIndexedHeader(uint32 index, uint32* new_index, + std::vector<uint32>* removed_referenced_indices) { + CHECK_GT(index, 0u); + CHECK_LT(index, GetEntryCount()); if (index <= header_table_.GetEntryCount()) { *new_index = index; diff --git a/net/spdy/hpack_encoding_context.h b/net/spdy/hpack_encoding_context.h index 9a4a2e8..1fdf615 100644 --- a/net/spdy/hpack_encoding_context.h +++ b/net/spdy/hpack_encoding_context.h @@ -16,15 +16,20 @@ #include "net/spdy/hpack_header_table.h" // All section references below are to -// http://tools.ietf.org/html/draft-ietf-httpbis-header-compression-05 -// . +// http://tools.ietf.org/html/draft-ietf-httpbis-header-compression-06 namespace net { +namespace test { +class HpackEncodingContextPeer; +} // namespace test + // An encoding context is simply a header table and its associated // reference set and a static table. class NET_EXPORT_PRIVATE HpackEncodingContext { public: + friend class test::HpackEncodingContextPeer; + // The constant returned by GetTouchesAt() if the indexed entry // hasn't been touched (which is distinct from having a touch count // of 0). @@ -69,20 +74,27 @@ class NET_EXPORT_PRIVATE HpackEncodingContext { // kUntouched. void ClearTouchesAt(uint32 index); - // Sets the maximum size of the encoding text, evicting entries if - // necessary. - void SetMaxSize(uint32 max_size); + // Called upon acknowledgement of SETTINGS_HEADER_TABLE_SIZE. + // If |max_size| is smaller than the current header table size, the change + // is treated as an implicit maximum-size context update. + void ApplyHeaderTableSizeSetting(uint32 max_size); // The Process*() functions below return true on success and false // if an error was encountered. + // Section 4.4. Sets the maximum size of the header table, evicting entries + // as needed. Fails if |max_size| is larger than SETTINGS_HEADER_TABLE_SIZE. + bool ProcessContextUpdateNewMaximumSize(uint32 max_size); + + // Section 4.4. Drops all headers from the reference set. + bool ProcessContextUpdateEmptyReferenceSet(); + // Tries to update the encoding context given an indexed header - // opcode for the given index (or zero) as described in - // 3.2.1. new_index is filled in with the index of a mutable entry, + // opcode for the given index as described in section 3.2.1. + // new_index is filled in with the index of a mutable entry, // or 0 if one was not created. removed_referenced_indices is filled - // in with the indices of all entries removed from the reference - // set. - bool ProcessIndexedHeader(uint32 index_or_zero, + // in with the indices of all entries removed from the reference set. + bool ProcessIndexedHeader(uint32 nonzero_index, uint32* new_index, std::vector<uint32>* removed_referenced_indices); @@ -99,6 +111,9 @@ class NET_EXPORT_PRIVATE HpackEncodingContext { std::vector<uint32>* removed_referenced_indices); private: + // Last acknowledged value for SETTINGS_HEADER_TABLE_SIZE. + uint32 settings_header_table_size_; + HpackHeaderTable header_table_; DISALLOW_COPY_AND_ASSIGN(HpackEncodingContext); diff --git a/net/spdy/hpack_encoding_context_test.cc b/net/spdy/hpack_encoding_context_test.cc index 2a2c6d3..de7f87d 100644 --- a/net/spdy/hpack_encoding_context_test.cc +++ b/net/spdy/hpack_encoding_context_test.cc @@ -7,23 +7,82 @@ #include <vector> #include "base/basictypes.h" +#include "net/spdy/hpack_constants.h" #include "testing/gtest/include/gtest/gtest.h" namespace net { +namespace test { + +class HpackEncodingContextPeer { + public: + explicit HpackEncodingContextPeer(const HpackEncodingContext& context) + : context_(context) {} + + const HpackHeaderTable& header_table() { + return context_.header_table_; + } + uint32 settings_header_table_size() { + return context_.settings_header_table_size_; + } + + private: + const HpackEncodingContext& context_; +}; + +} // namespace test + namespace { -// Try to process an indexed header with an invalid index. That should -// fail. -TEST(HpackEncodingContextTest, IndexedHeaderInvalid) { - HpackEncodingContext encoding_context; +TEST(HpackEncodingContextTest, ApplyHeaderTableSizeSetting) { + HpackEncodingContext context; + test::HpackEncodingContextPeer peer(context); - uint32 new_index = 0; - std::vector<uint32> removed_referenced_indices; - EXPECT_FALSE( - encoding_context.ProcessIndexedHeader(kuint32max, - &new_index, - &removed_referenced_indices)); + // Default setting and table size are kDefaultHeaderTableSizeSetting. + EXPECT_EQ(kDefaultHeaderTableSizeSetting, + peer.settings_header_table_size()); + EXPECT_EQ(kDefaultHeaderTableSizeSetting, + peer.header_table().max_size()); + + // Applying a larger table size setting doesn't affect the headers table. + context.ApplyHeaderTableSizeSetting(kDefaultHeaderTableSizeSetting * 2); + + EXPECT_EQ(kDefaultHeaderTableSizeSetting * 2, + peer.settings_header_table_size()); + EXPECT_EQ(kDefaultHeaderTableSizeSetting, + peer.header_table().max_size()); + + // Applying a smaller size setting does update the headers table. + context.ApplyHeaderTableSizeSetting(kDefaultHeaderTableSizeSetting / 2); + + EXPECT_EQ(kDefaultHeaderTableSizeSetting / 2, + peer.settings_header_table_size()); + EXPECT_EQ(kDefaultHeaderTableSizeSetting / 2, + peer.header_table().max_size()); +} + +TEST(HpackEncodingContextTest, ProcessContextUpdateNewMaximumSize) { + HpackEncodingContext context; + test::HpackEncodingContextPeer peer(context); + + EXPECT_EQ(kDefaultHeaderTableSizeSetting, + peer.settings_header_table_size()); + + // Shrink maximum size by half. Succeeds. + EXPECT_TRUE(context.ProcessContextUpdateNewMaximumSize( + kDefaultHeaderTableSizeSetting / 2)); + EXPECT_EQ(kDefaultHeaderTableSizeSetting / 2, + peer.header_table().max_size()); + + // Double maximum size. Succeeds. + EXPECT_TRUE(context.ProcessContextUpdateNewMaximumSize( + kDefaultHeaderTableSizeSetting)); + EXPECT_EQ(kDefaultHeaderTableSizeSetting, peer.header_table().max_size()); + + // One beyond table size setting. Fails. + EXPECT_FALSE(context.ProcessContextUpdateNewMaximumSize( + kDefaultHeaderTableSizeSetting + 1)); + EXPECT_EQ(kDefaultHeaderTableSizeSetting, peer.header_table().max_size()); } // Try to process an indexed header with an index for a static @@ -75,7 +134,7 @@ TEST(HpackEncodingContextTest, IndexedHeaderStatic) { // fit. That should succeed without making a copy. TEST(HpackEncodingContextTest, IndexedHeaderStaticCopyDoesNotFit) { HpackEncodingContext encoding_context; - encoding_context.SetMaxSize(0); + encoding_context.ProcessContextUpdateNewMaximumSize(0); uint32 new_index = 0; std::vector<uint32> removed_referenced_indices; @@ -88,13 +147,13 @@ TEST(HpackEncodingContextTest, IndexedHeaderStaticCopyDoesNotFit) { EXPECT_EQ(0u, encoding_context.GetMutableEntryCount()); } -// Add a bunch of new headers and then try to process an indexed -// header with index 0. That should clear the reference set. -TEST(HpackEncodingContextTest, IndexedHeaderZero) { +// Add a bunch of new headers and then process a context update to empty the +// reference set. Expect it to be empty. +TEST(HpackEncodingContextTest, ProcessContextUpdateEmptyReferenceSet) { HpackEncodingContext encoding_context; + test::HpackEncodingContextPeer peer(encoding_context); uint32 kEntryCount = 50; - std::vector<uint32> expected_removed_referenced_indices; for (uint32 i = 1; i <= kEntryCount; ++i) { uint32 new_index = 0; @@ -106,16 +165,15 @@ TEST(HpackEncodingContextTest, IndexedHeaderZero) { EXPECT_EQ(1u, new_index); EXPECT_TRUE(removed_referenced_indices.empty()); EXPECT_EQ(i, encoding_context.GetMutableEntryCount()); - expected_removed_referenced_indices.push_back(i); } - uint32 new_index = 0; - std::vector<uint32> removed_referenced_indices; - EXPECT_TRUE( - encoding_context.ProcessIndexedHeader(0, &new_index, - &removed_referenced_indices)); - EXPECT_EQ(0u, new_index); - EXPECT_EQ(expected_removed_referenced_indices, removed_referenced_indices); + for (uint32 i = 1; i <= kEntryCount; ++i) { + EXPECT_TRUE(peer.header_table().GetEntry(i).IsReferenced()); + } + encoding_context.ProcessContextUpdateEmptyReferenceSet(); + for (uint32 i = 1; i <= kEntryCount; ++i) { + EXPECT_FALSE(peer.header_table().GetEntry(i).IsReferenced()); + } } // NOTE: It's too onerous to try to test invalid input to @@ -145,7 +203,7 @@ TEST(HpackEncodingContextTest, LiteralHeaderIncrementalIndexing) { // into the table. TEST(HpackEncodingContextTest, LiteralHeaderIncrementalIndexingDoesNotFit) { HpackEncodingContext encoding_context; - encoding_context.SetMaxSize(0); + encoding_context.ProcessContextUpdateNewMaximumSize(0); uint32 index = 0; std::vector<uint32> removed_referenced_indices; diff --git a/net/spdy/hpack_entry.h b/net/spdy/hpack_entry.h index 7b85109..7cfd14e 100644 --- a/net/spdy/hpack_entry.h +++ b/net/spdy/hpack_entry.h @@ -18,8 +18,7 @@ namespace net { // All section references below are to -// http://tools.ietf.org/html/draft-ietf-httpbis-header-compression-05 -// . +// http://tools.ietf.org/html/draft-ietf-httpbis-header-compression-06 // A structure for an entry in the header table (3.1.2) and the // reference set (3.1.3). This structure also keeps track of how many diff --git a/net/spdy/hpack_header_table.cc b/net/spdy/hpack_header_table.cc index 97a2a02..1320bfb 100644 --- a/net/spdy/hpack_header_table.cc +++ b/net/spdy/hpack_header_table.cc @@ -5,11 +5,14 @@ #include "net/spdy/hpack_header_table.h" #include "base/logging.h" +#include "net/spdy/hpack_constants.h" #include "net/spdy/hpack_string_util.h" namespace net { -HpackHeaderTable::HpackHeaderTable() : size_(0), max_size_(4096) {} +HpackHeaderTable::HpackHeaderTable() + : size_(0), + max_size_(kDefaultHeaderTableSizeSetting) {} HpackHeaderTable::~HpackHeaderTable() {} diff --git a/net/spdy/hpack_header_table.h b/net/spdy/hpack_header_table.h index a97d4a7..792fcfb 100644 --- a/net/spdy/hpack_header_table.h +++ b/net/spdy/hpack_header_table.h @@ -17,8 +17,7 @@ namespace net { // All section references below are to -// http://tools.ietf.org/html/draft-ietf-httpbis-header-compression-05 -// . +// http://tools.ietf.org/html/draft-ietf-httpbis-header-compression-06 // A data structure for both the header table (described in 3.1.2) and // the reference set (3.1.3). This structure also keeps track of how diff --git a/net/spdy/hpack_input_stream.cc b/net/spdy/hpack_input_stream.cc index f7981a3..ee5fab0 100644 --- a/net/spdy/hpack_input_stream.cc +++ b/net/spdy/hpack_input_stream.cc @@ -35,7 +35,7 @@ bool HpackInputStream::MatchPrefixAndConsume(HpackPrefix prefix) { return false; if ((next_octet >> (8 - prefix.bit_size)) == prefix.bits) { - bit_offset_ = prefix.bit_size; + ConsumeBits(prefix.bit_size); return true; } diff --git a/net/spdy/hpack_input_stream.h b/net/spdy/hpack_input_stream.h index 1a38dc6..4281332 100644 --- a/net/spdy/hpack_input_stream.h +++ b/net/spdy/hpack_input_stream.h @@ -16,7 +16,7 @@ #include "net/spdy/hpack_huffman_table.h" // All section references below are to -// http://tools.ietf.org/html/draft-ietf-httpbis-header-compression-05 +// http://tools.ietf.org/html/draft-ietf-httpbis-header-compression-06 namespace net { diff --git a/net/spdy/hpack_output_stream.h b/net/spdy/hpack_output_stream.h index 67c7739..a88e522 100644 --- a/net/spdy/hpack_output_stream.h +++ b/net/spdy/hpack_output_stream.h @@ -16,8 +16,7 @@ #include "net/spdy/hpack_encoding_context.h" // All section references below are to -// http://tools.ietf.org/html/draft-ietf-httpbis-header-compression-05 -// . +// http://tools.ietf.org/html/draft-ietf-httpbis-header-compression-06 namespace net { diff --git a/net/spdy/hpack_string_util.h b/net/spdy/hpack_string_util.h index 0924a79..81e8557 100644 --- a/net/spdy/hpack_string_util.h +++ b/net/spdy/hpack_string_util.h @@ -11,8 +11,7 @@ namespace net { // All section references below are to -// http://tools.ietf.org/html/draft-ietf-httpbis-header-compression-05 -// . +// http://tools.ietf.org/html/draft-ietf-httpbis-header-compression-06 // A constant-time StringPiece comparison function, as suggested // by section 6. |