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
