Adds HpackEncoder::EncodeRepresentations(), where the caller can specify an explicit list of (key, value) pairs. This method does not split values on \0. gfe-relnote: Adds a new method to HpackEncoder which is not yet used in production code. Not protected. PiperOrigin-RevId: 285121328 Change-Id: Ieffbc569af94e9d9f4c350241d390149feb77019
diff --git a/spdy/core/hpack/hpack_encoder.cc b/spdy/core/hpack/hpack_encoder.cc index af76c3b..39a4de8 100644 --- a/spdy/core/hpack/hpack_encoder.cc +++ b/spdy/core/hpack/hpack_encoder.cc
@@ -281,6 +281,7 @@ class HpackEncoder::Encoderator : public ProgressiveEncoder { public: Encoderator(const SpdyHeaderBlock& header_set, HpackEncoder* encoder); + Encoderator(const Representations& representations, HpackEncoder* encoder); // Encoderator is neither copyable nor movable. Encoderator(const Encoderator&) = delete; @@ -328,6 +329,25 @@ encoder_->MaybeEmitTableSize(); } +HpackEncoder::Encoderator::Encoderator(const Representations& representations, + HpackEncoder* encoder) + : encoder_(encoder), has_next_(true) { + for (const auto& header : representations) { + if (header.first == "cookie") { + CookieToCrumbs(header, ®ular_headers_); + } else if (!header.first.empty() && + header.first[0] == kPseudoHeaderPrefix) { + pseudo_headers_.push_back(header); + } else { + regular_headers_.push_back(header); + } + } + header_it_ = std::make_unique<RepresentationIterator>(pseudo_headers_, + regular_headers_); + + encoder_->MaybeEmitTableSize(); +} + void HpackEncoder::Encoderator::Next(size_t max_encoded_bytes, std::string* output) { SPDY_BUG_IF(!has_next_) @@ -363,4 +383,9 @@ return std::make_unique<Encoderator>(header_set, this); } +std::unique_ptr<HpackEncoder::ProgressiveEncoder> +HpackEncoder::EncodeRepresentations(const Representations& representations) { + return std::make_unique<Encoderator>(representations, this); +} + } // namespace spdy
diff --git a/spdy/core/hpack/hpack_encoder.h b/spdy/core/hpack/hpack_encoder.h index a24d69b..d3bfc95 100644 --- a/spdy/core/hpack/hpack_encoder.h +++ b/spdy/core/hpack/hpack_encoder.h
@@ -71,6 +71,12 @@ // SpdyHeaderBlock and this object. std::unique_ptr<ProgressiveEncoder> EncodeHeaderSet( const SpdyHeaderBlock& header_set); + // Returns a ProgressiveEncoder which must be outlived by this HpackEncoder. + // The encoder will not attempt to split any \0-delimited values in + // |representations|. If such splitting is desired, it must be performed by + // the caller when constructing the list of representations. + std::unique_ptr<ProgressiveEncoder> EncodeRepresentations( + const Representations& representations); // Called upon a change to SETTINGS_HEADER_TABLE_SIZE. Specifically, this // is to be called after receiving (and sending an acknowledgement for) a
diff --git a/spdy/core/hpack/hpack_encoder_test.cc b/spdy/core/hpack/hpack_encoder_test.cc index 9a73a88..fb09301 100644 --- a/spdy/core/hpack/hpack_encoder_test.cc +++ b/spdy/core/hpack/hpack_encoder_test.cc
@@ -71,13 +71,8 @@ // non-incremental encoding path. static bool EncodeHeaderSet(HpackEncoder* encoder, const SpdyHeaderBlock& header_set, - std::string* output, - bool use_incremental) { - if (use_incremental) { - return EncodeIncremental(encoder, header_set, output); - } else { - return encoder->EncodeHeaderSet(header_set, output); - } + std::string* output) { + return encoder->EncodeHeaderSet(header_set, output); } static bool EncodeIncremental(HpackEncoder* encoder, @@ -97,6 +92,23 @@ return true; } + static bool EncodeRepresentations(HpackEncoder* encoder, + const Representations& representations, + std::string* output) { + std::unique_ptr<HpackEncoder::ProgressiveEncoder> encoderator = + encoder->EncodeRepresentations(representations); + std::string output_buffer; + http2::test::Http2Random random; + encoderator->Next(random.UniformInRange(0, 16), &output_buffer); + while (encoderator->HasNext()) { + std::string second_buffer; + encoderator->Next(random.UniformInRange(0, 16), &second_buffer); + output_buffer.append(second_buffer); + } + *output = std::move(output_buffer); + return true; + } + private: HpackEncoder* encoder_; }; @@ -108,19 +120,23 @@ using testing::ElementsAre; using testing::Pair; -class HpackEncoderTest : public ::testing::TestWithParam<bool> { +enum EncodeStrategy { + kDefault, + kIncremental, + kRepresentations, +}; + +class HpackEncoderTestBase : public ::testing::Test { protected: typedef test::HpackEncoderPeer::Representations Representations; - HpackEncoderTest() + HpackEncoderTestBase() : encoder_(ObtainHpackHuffmanTable()), peer_(&encoder_), static_(peer_.table()->GetByIndex(1)), headers_storage_(1024 /* block size */) {} void SetUp() override { - use_incremental_ = GetParam(); - // Populate dynamic entries into the table fixture. For simplicity each // entry has name.size() + value.size() == 10. key_1_ = peer_.table()->TryAddEntry("key1", "value1"); @@ -181,11 +197,37 @@ expected_.AppendPrefix(kHeaderTableSizeUpdateOpcode); expected_.AppendUint32(size); } + Representations MakeRepresentations(const SpdyHeaderBlock& header_set) { + Representations r; + for (const auto& header : header_set) { + r.push_back(header); + } + return r; + } void CompareWithExpectedEncoding(const SpdyHeaderBlock& header_set) { std::string expected_out, actual_out; expected_.TakeString(&expected_out); - EXPECT_TRUE(test::HpackEncoderPeer::EncodeHeaderSet( - &encoder_, header_set, &actual_out, use_incremental_)); + switch (strategy_) { + case kDefault: + EXPECT_TRUE(test::HpackEncoderPeer::EncodeHeaderSet( + &encoder_, header_set, &actual_out)); + break; + case kIncremental: + EXPECT_TRUE(test::HpackEncoderPeer::EncodeIncremental( + &encoder_, header_set, &actual_out)); + break; + case kRepresentations: + EXPECT_TRUE(test::HpackEncoderPeer::EncodeRepresentations( + &encoder_, MakeRepresentations(header_set), &actual_out)); + break; + } + EXPECT_EQ(expected_out, actual_out); + } + void CompareWithExpectedEncoding(const Representations& representations) { + std::string expected_out, actual_out; + expected_.TakeString(&expected_out); + EXPECT_TRUE(test::HpackEncoderPeer::EncodeRepresentations( + &encoder_, representations, &actual_out)); EXPECT_EQ(expected_out, actual_out); } size_t IndexOf(const HpackEntry* entry) { @@ -205,12 +247,53 @@ std::vector<std::pair<SpdyStringPiece, SpdyStringPiece>> headers_observed_; HpackOutputStream expected_; - bool use_incremental_; + EncodeStrategy strategy_ = kDefault; +}; + +TEST_F(HpackEncoderTestBase, EncodeRepresentations) { + encoder_.SetHeaderListener( + [this](SpdyStringPiece name, SpdyStringPiece value) { + this->SaveHeaders(name, value); + }); + const std::vector<std::pair<SpdyStringPiece, SpdyStringPiece>> header_list = { + {"cookie", "val1; val2;val3"}, + {":path", "/home"}, + {"accept", "text/html, text/plain,application/xml"}, + {"cookie", "val4"}, + {"withnul", SpdyStringPiece("one\0two", 7)}}; + ExpectNonIndexedLiteral(":path", "/home"); + ExpectIndexedLiteral(peer_.table()->GetByName("cookie"), "val1"); + ExpectIndexedLiteral(peer_.table()->GetByName("cookie"), "val2"); + ExpectIndexedLiteral(peer_.table()->GetByName("cookie"), "val3"); + ExpectIndexedLiteral(peer_.table()->GetByName("accept"), + "text/html, text/plain,application/xml"); + ExpectIndexedLiteral(peer_.table()->GetByName("cookie"), "val4"); + ExpectIndexedLiteral("withnul", SpdyStringPiece("one\0two", 7)); + + CompareWithExpectedEncoding(header_list); + EXPECT_THAT( + headers_observed_, + ElementsAre(Pair(":path", "/home"), Pair("cookie", "val1"), + Pair("cookie", "val2"), Pair("cookie", "val3"), + Pair("accept", "text/html, text/plain,application/xml"), + Pair("cookie", "val4"), + Pair("withnul", std::string("one\0two", 7)))); +} + +class HpackEncoderTest : public HpackEncoderTestBase, + public ::testing::WithParamInterface<EncodeStrategy> { + protected: + void SetUp() override { + strategy_ = GetParam(); + HpackEncoderTestBase::SetUp(); + } }; INSTANTIATE_TEST_SUITE_P(HpackEncoderTests, HpackEncoderTest, - ::testing::Bool()); + ::testing::Values(kDefault, + kIncremental, + kRepresentations)); TEST_P(HpackEncoderTest, SingleDynamicIndex) { encoder_.SetHeaderListener( @@ -343,7 +426,7 @@ ExpectNonIndexedLiteral(":path", "/index.html"); ExpectNonIndexedLiteral("cookie", "foo=bar"); ExpectNonIndexedLiteral("cookie", "baz=bing"); - if (use_incremental_) { + if (strategy_ != kDefault) { // BUG: encodes as a \0-delimited value. Should be separate entries. ExpectNonIndexedLiteral("hello", std::string("goodbye\0aloha", 13)); } else { @@ -361,7 +444,7 @@ CompareWithExpectedEncoding(headers); - if (use_incremental_) { + if (strategy_ != kDefault) { // BUG: value for "hello" encodes as \0-delimited. Should be separate // entries. EXPECT_THAT( @@ -536,6 +619,11 @@ // Test that encoded headers do not have \0-delimited multiple values, as this // became disallowed in HTTP/2 draft-14. TEST_P(HpackEncoderTest, CrumbleNullByteDelimitedValue) { + if (strategy_ == kRepresentations) { + // When HpackEncoder is asked to encode a list of Representations, the + // caller must crumble null-delimited values. + return; + } SpdyHeaderBlock headers; // A header field to be crumbled: "spam: foo\0bar". headers["spam"] = std::string("foo\0bar", 7);