Split cookies along ';' instead of '\0' in ValueSplittingHeaderList. This is described at https://httpwg.org/specs/rfc7540.html#CompressCookie. gfe-relnote: n/a. Change in QPACK code used in QUIC v99 only. PiperOrigin-RevId: 252041422 Change-Id: Id4e99993d8c25cccdedd334488fd6b5cae3058d0
diff --git a/quic/core/qpack/fuzzer/qpack_round_trip_fuzzer.cc b/quic/core/qpack/fuzzer/qpack_round_trip_fuzzer.cc index 5b7f8d3..5f752ea 100644 --- a/quic/core/qpack/fuzzer/qpack_round_trip_fuzzer.cc +++ b/quic/core/qpack/fuzzer/qpack_round_trip_fuzzer.cc
@@ -143,8 +143,8 @@ CHECK(handler.decoding_completed()); CHECK(!handler.decoding_error_detected()); - // Encoder splits |header_list| header keys along '\0' characters. Do the - // same so that we get matching results. + // Encoder splits |header_list| header values along '\0' or ';' separators. + // Do the same here so that we get matching results. ValueSplittingHeaderList splitting_header_list(&header_list); spdy::SpdyHeaderBlock expected_header_list; for (const auto& header : splitting_header_list) {
diff --git a/quic/core/qpack/value_splitting_header_list.cc b/quic/core/qpack/value_splitting_header_list.cc index d8a0ee8..c1387b9 100644 --- a/quic/core/qpack/value_splitting_header_list.cc +++ b/quic/core/qpack/value_splitting_header_list.cc
@@ -5,6 +5,14 @@ #include "net/third_party/quiche/src/quic/core/qpack/value_splitting_header_list.h" namespace quic { +namespace { + +const char kCookieKey[] = "cookie"; +const char kCookieSeparator = ';'; +const char kOptionalSpaceAfterCookieSeparator = ' '; +const char kNonCookieSeparator = '\0'; + +} // namespace ValueSplittingHeaderList::const_iterator::const_iterator( const spdy::SpdyHeaderBlock* header_list, @@ -59,15 +67,24 @@ } const QuicStringPiece name = header_list_iterator_->first; + const QuicStringPiece original_value = header_list_iterator_->second; - value_end_ = header_list_iterator_->second.find('\0', value_start_); - const QuicStringPiece::size_type value_length = - value_end_ == QuicStringPiece::npos ? QuicStringPiece::npos - : value_end_ - value_start_; + if (name == kCookieKey) { + value_end_ = original_value.find(kCookieSeparator, value_start_); + } else { + value_end_ = original_value.find(kNonCookieSeparator, value_start_); + } + const QuicStringPiece value = - header_list_iterator_->second.substr(value_start_, value_length); - + original_value.substr(value_start_, value_end_ - value_start_); header_field_ = std::make_pair(name, value); + + // Skip character after ';' separator if it is a space. + if (name == kCookieKey && value_end_ != QuicStringPiece::npos && + value_end_ + 1 < original_value.size() && + original_value[value_end_ + 1] == kOptionalSpaceAfterCookieSeparator) { + ++value_end_; + } } ValueSplittingHeaderList::ValueSplittingHeaderList(
diff --git a/quic/core/qpack/value_splitting_header_list.h b/quic/core/qpack/value_splitting_header_list.h index 8d994ae..fee3043 100644 --- a/quic/core/qpack/value_splitting_header_list.h +++ b/quic/core/qpack/value_splitting_header_list.h
@@ -11,8 +11,9 @@ namespace quic { -// A wrapper class around SpdyHeaderBlock that splits header values along '\0' -// characters. +// A wrapper class around SpdyHeaderBlock that splits header values along ';' +// separators (while also removing optional space following separator) for +// cookies and along '\0' separators for other header fields. class QUIC_EXPORT_PRIVATE ValueSplittingHeaderList { public: using value_type = spdy::SpdyHeaderBlock::value_type; @@ -34,7 +35,7 @@ const value_type* operator->() const; private: - // Find next '\0' character; update |value_end_| and |header_field_|. + // Find next separator; update |value_end_| and |header_field_|. void UpdateHeaderField(); const spdy::SpdyHeaderBlock* const header_list_;
diff --git a/quic/core/qpack/value_splitting_header_list_test.cc b/quic/core/qpack/value_splitting_header_list_test.cc index 9ece65d..03a5eb0 100644 --- a/quic/core/qpack/value_splitting_header_list_test.cc +++ b/quic/core/qpack/value_splitting_header_list_test.cc
@@ -4,6 +4,7 @@ #include "net/third_party/quiche/src/quic/core/qpack/value_splitting_header_list.h" +#include "net/third_party/quiche/src/quic/platform/api/quic_arraysize.h" #include "net/third_party/quiche/src/quic/platform/api/quic_test.h" namespace quic { @@ -17,10 +18,11 @@ spdy::SpdyHeaderBlock block; block["foo"] = QuicStringPiece("bar\0baz", 7); block["baz"] = "qux"; + block["cookie"] = "foo; bar"; ValueSplittingHeaderList headers(&block); ValueSplittingHeaderList::const_iterator it1 = headers.begin(); - const int kEnd = 4; + const int kEnd = 6; for (int i = 0; i < kEnd; ++i) { // Compare to begin(). if (i == 0) { @@ -73,47 +75,75 @@ EXPECT_EQ(headers.begin(), headers.end()); } -TEST(ValueSplittingHeaderListTest, Simple) { - spdy::SpdyHeaderBlock block; - block["foo"] = "bar"; - block["baz"] = "qux"; +struct { + const char* name; + QuicStringPiece value; + std::vector<const char*> expected_values; +} kTestData[]{ + // Empty value. + {"foo", "", {""}}, + // Trivial case. + {"foo", "bar", {"bar"}}, + // Simple split. + {"foo", {"bar\0baz", 7}, {"bar", "baz"}}, + {"cookie", "foo;bar", {"foo", "bar"}}, + {"cookie", "foo; bar", {"foo", "bar"}}, + // Empty fragments with \0 separator. + {"foo", {"\0", 1}, {"", ""}}, + {"bar", {"foo\0", 4}, {"foo", ""}}, + {"baz", {"\0bar", 4}, {"", "bar"}}, + {"qux", {"\0foobar\0", 8}, {"", "foobar", ""}}, + // Empty fragments with ";" separator. + {"cookie", ";", {"", ""}}, + {"cookie", "foo;", {"foo", ""}}, + {"cookie", ";bar", {"", "bar"}}, + {"cookie", ";foobar;", {"", "foobar", ""}}, + // Empty fragments with "; " separator. + {"cookie", "; ", {"", ""}}, + {"cookie", "foo; ", {"foo", ""}}, + {"cookie", "; bar", {"", "bar"}}, + {"cookie", "; foobar; ", {"", "foobar", ""}}, +}; - ValueSplittingHeaderList headers(&block); - EXPECT_THAT(headers, ElementsAre(Pair("foo", "bar"), Pair("baz", "qux"))); - EXPECT_NE(headers.begin(), headers.end()); +TEST(ValueSplittingHeaderListTest, Split) { + for (size_t i = 0; i < QUIC_ARRAYSIZE(kTestData); ++i) { + spdy::SpdyHeaderBlock block; + block[kTestData[i].name] = kTestData[i].value; + + ValueSplittingHeaderList headers(&block); + auto it = headers.begin(); + for (const char* expected_value : kTestData[i].expected_values) { + ASSERT_NE(it, headers.end()); + EXPECT_EQ(it->first, kTestData[i].name); + EXPECT_EQ(it->second, expected_value); + ++it; + } + EXPECT_EQ(it, headers.end()); + } } -TEST(ValueSplittingHeaderListTest, EmptyValue) { +TEST(ValueSplittingHeaderListTest, MultipleFields) { spdy::SpdyHeaderBlock block; - block["foo"] = ""; - - ValueSplittingHeaderList headers(&block); - EXPECT_THAT(headers, ElementsAre(Pair("foo", ""))); -} - -TEST(ValueSplittingHeaderListTest, SimpleSplit) { - spdy::SpdyHeaderBlock block; - block["foo"] = QuicStringPiece("bar\0baz", 7); - block["baz"] = QuicStringPiece("foo\0foo", 7); + block["foo"] = QuicStringPiece("bar\0baz\0", 8); + block["cookie"] = "foo; bar"; + block["bar"] = QuicStringPiece("qux\0foo", 7); ValueSplittingHeaderList headers(&block); EXPECT_THAT(headers, ElementsAre(Pair("foo", "bar"), Pair("foo", "baz"), - Pair("baz", "foo"), Pair("baz", "foo"))); + Pair("foo", ""), Pair("cookie", "foo"), + Pair("cookie", "bar"), Pair("bar", "qux"), + Pair("bar", "foo"))); } -TEST(ValueSplittingHeaderListTest, EmptyFragments) { +TEST(ValueSplittingHeaderListTest, CookieStartsWithSpace) { spdy::SpdyHeaderBlock block; - block["foo"] = QuicStringPiece("\0", 1); - block["bar"] = QuicStringPiece("foo\0", 4); - block["baz"] = QuicStringPiece("\0bar", 4); - block["qux"] = QuicStringPiece("\0foobar\0", 8); + block["foo"] = "bar"; + block["cookie"] = " foo"; + block["bar"] = "baz"; ValueSplittingHeaderList headers(&block); - EXPECT_THAT( - headers, - ElementsAre(Pair("foo", ""), Pair("foo", ""), Pair("bar", "foo"), - Pair("bar", ""), Pair("baz", ""), Pair("baz", "bar"), - Pair("qux", ""), Pair("qux", "foobar"), Pair("qux", ""))); + EXPECT_THAT(headers, ElementsAre(Pair("foo", "bar"), Pair("cookie", " foo"), + Pair("bar", "baz"))); } } // namespace