Add CookieCrumbling argument to QpackEncoder constructor, and add QuicSpdySession::DisableCookieCrumbling() method.
This QUICHE API change necessitates a change in Envoy.
The same change can be done to public Envoy when rolling the QUICHE change,
and the patch file can be removed when rolling the public Envoy change
internally.
NOT generated via copybara
PiperOrigin-RevId: 658147234
diff --git a/quiche/quic/core/http/end_to_end_test.cc b/quiche/quic/core/http/end_to_end_test.cc
index 6bdf826..e32af3b 100644
--- a/quiche/quic/core/http/end_to_end_test.cc
+++ b/quiche/quic/core/http/end_to_end_test.cc
@@ -28,6 +28,7 @@
#include "quiche/quic/core/http/web_transport_http3.h"
#include "quiche/quic/core/io/quic_default_event_loop.h"
#include "quiche/quic/core/io/quic_event_loop.h"
+#include "quiche/quic/core/qpack/value_splitting_header_list.h"
#include "quiche/quic/core/quic_connection.h"
#include "quiche/quic/core/quic_constants.h"
#include "quiche/quic/core/quic_data_writer.h"
@@ -3970,7 +3971,8 @@
NoopDecoderStreamErrorDelegate decoder_stream_error_delegate;
NoopQpackStreamSenderDelegate encoder_stream_sender_delegate;
QpackEncoder qpack_encoder(&decoder_stream_error_delegate,
- HuffmanEncoding::kEnabled);
+ HuffmanEncoding::kEnabled,
+ CookieCrumbling::kEnabled);
qpack_encoder.set_qpack_stream_sender_delegate(
&encoder_stream_sender_delegate);
diff --git a/quiche/quic/core/http/metadata_decoder_test.cc b/quiche/quic/core/http/metadata_decoder_test.cc
index 2740a58..37e2ae5 100644
--- a/quiche/quic/core/http/metadata_decoder_test.cc
+++ b/quiche/quic/core/http/metadata_decoder_test.cc
@@ -19,7 +19,8 @@
protected:
std::string EncodeHeaders(quiche::HttpHeaderBlock& headers) {
quic::NoopDecoderStreamErrorDelegate delegate;
- quic::QpackEncoder encoder(&delegate, quic::HuffmanEncoding::kDisabled);
+ quic::QpackEncoder encoder(&delegate, quic::HuffmanEncoding::kDisabled,
+ quic::CookieCrumbling::kDisabled);
return encoder.EncodeHeaderList(id_, headers,
/*encoder_stream_sent_byte_count=*/nullptr);
}
diff --git a/quiche/quic/core/http/quic_spdy_server_stream_base_test.cc b/quiche/quic/core/http/quic_spdy_server_stream_base_test.cc
index 88cb723..0f2d0f9 100644
--- a/quiche/quic/core/http/quic_spdy_server_stream_base_test.cc
+++ b/quiche/quic/core/http/quic_spdy_server_stream_base_test.cc
@@ -9,6 +9,7 @@
#include "absl/memory/memory.h"
#include "quiche/quic/core/crypto/null_encrypter.h"
+#include "quiche/quic/core/qpack/value_splitting_header_list.h"
#include "quiche/quic/platform/api/quic_flags.h"
#include "quiche/quic/platform/api/quic_test.h"
#include "quiche/quic/test_tools/qpack/qpack_test_utils.h"
@@ -305,7 +306,8 @@
quic::test::NoopQpackStreamSenderDelegate encoder_stream_sender_delegate;
NoopDecoderStreamErrorDelegate decoder_stream_error_delegate;
auto qpack_encoder = std::make_unique<quic::QpackEncoder>(
- &decoder_stream_error_delegate, HuffmanEncoding::kEnabled);
+ &decoder_stream_error_delegate, HuffmanEncoding::kEnabled,
+ CookieCrumbling::kEnabled);
qpack_encoder->set_qpack_stream_sender_delegate(
&encoder_stream_sender_delegate);
std::string payload =
diff --git a/quiche/quic/core/http/quic_spdy_session.cc b/quiche/quic/core/http/quic_spdy_session.cc
index e946627..a40f74f 100644
--- a/quiche/quic/core/http/quic_spdy_session.cc
+++ b/quiche/quic/core/http/quic_spdy_session.cc
@@ -541,7 +541,8 @@
headers_stream_ = headers_stream.get();
ActivateStream(std::move(headers_stream));
} else {
- qpack_encoder_ = std::make_unique<QpackEncoder>(this, huffman_encoding_);
+ qpack_encoder_ = std::make_unique<QpackEncoder>(this, huffman_encoding_,
+ cookie_crumbling_);
qpack_decoder_ =
std::make_unique<QpackDecoder>(qpack_maximum_dynamic_table_capacity_,
qpack_maximum_blocked_streams_, this);
diff --git a/quiche/quic/core/http/quic_spdy_session.h b/quiche/quic/core/http/quic_spdy_session.h
index 3c2c11b..a6e4e1a 100644
--- a/quiche/quic/core/http/quic_spdy_session.h
+++ b/quiche/quic/core/http/quic_spdy_session.h
@@ -25,6 +25,7 @@
#include "quiche/quic/core/qpack/qpack_encoder.h"
#include "quiche/quic/core/qpack/qpack_receive_stream.h"
#include "quiche/quic/core/qpack/qpack_send_stream.h"
+#include "quiche/quic/core/qpack/value_splitting_header_list.h"
#include "quiche/quic/core/quic_session.h"
#include "quiche/quic/core/quic_stream_priority.h"
#include "quiche/quic/core/quic_time.h"
@@ -569,6 +570,11 @@
huffman_encoding_ = HuffmanEncoding::kDisabled;
}
+ // Disables cookie crumbling for QPACK headers.
+ void DisableCookieCrumbling() {
+ cookie_crumbling_ = CookieCrumbling::kDisabled;
+ }
+
private:
friend class test::QuicSpdySessionPeer;
@@ -617,6 +623,7 @@
bool ValidateWebTransportSettingsConsistency();
HuffmanEncoding huffman_encoding_ = HuffmanEncoding::kEnabled;
+ CookieCrumbling cookie_crumbling_ = CookieCrumbling::kEnabled;
std::unique_ptr<QpackEncoder> qpack_encoder_;
std::unique_ptr<QpackDecoder> qpack_decoder_;
diff --git a/quiche/quic/core/http/quic_spdy_stream_test.cc b/quiche/quic/core/http/quic_spdy_stream_test.cc
index 2984396..5ede717 100644
--- a/quiche/quic/core/http/quic_spdy_stream_test.cc
+++ b/quiche/quic/core/http/quic_spdy_stream_test.cc
@@ -24,6 +24,7 @@
#include "quiche/quic/core/http/quic_spdy_session.h"
#include "quiche/quic/core/http/spdy_utils.h"
#include "quiche/quic/core/http/web_transport_http3.h"
+#include "quiche/quic/core/qpack/value_splitting_header_list.h"
#include "quiche/quic/core/quic_connection.h"
#include "quiche/quic/core/quic_stream_sequencer_buffer.h"
#include "quiche/quic/core/quic_utils.h"
@@ -398,7 +399,7 @@
std::string EncodeQpackHeaders(const HttpHeaderBlock& header) {
NoopQpackStreamSenderDelegate encoder_stream_sender_delegate;
auto qpack_encoder = std::make_unique<QpackEncoder>(
- session_.get(), HuffmanEncoding::kEnabled);
+ session_.get(), HuffmanEncoding::kEnabled, CookieCrumbling::kEnabled);
qpack_encoder->set_qpack_stream_sender_delegate(
&encoder_stream_sender_delegate);
// QpackEncoder does not use the dynamic table by default,
@@ -2737,14 +2738,14 @@
quiche::HttpHeaderBlock headers;
headers.AppendValueOrAddHeader("key1", "val1");
headers.AppendValueOrAddHeader("key2", "val2");
- quic::NoopDecoderStreamErrorDelegate delegate;
- QpackEncoder qpack_encoder(&delegate, quic::HuffmanEncoding::kDisabled);
+ NoopDecoderStreamErrorDelegate delegate;
+ QpackEncoder qpack_encoder(&delegate, HuffmanEncoding::kDisabled,
+ CookieCrumbling::kEnabled);
std::string metadata_frame_payload = qpack_encoder.EncodeHeaderList(
stream_->id(), headers,
/* encoder_stream_sent_byte_count = */ nullptr);
std::string metadata_frame_header =
- quic::HttpEncoder::SerializeMetadataFrameHeader(
- metadata_frame_payload.size());
+ HttpEncoder::SerializeMetadataFrameHeader(metadata_frame_payload.size());
std::string metadata_frame = metadata_frame_header + metadata_frame_payload;
EXPECT_CALL(debug_visitor,
@@ -2775,14 +2776,14 @@
quiche::HttpHeaderBlock headers;
headers.AppendValueOrAddHeader("key1", "val1");
headers.AppendValueOrAddHeader("key2", "val2");
- quic::NoopDecoderStreamErrorDelegate delegate;
- QpackEncoder qpack_encoder(&delegate, quic::HuffmanEncoding::kDisabled);
+ NoopDecoderStreamErrorDelegate delegate;
+ QpackEncoder qpack_encoder(&delegate, HuffmanEncoding::kDisabled,
+ CookieCrumbling::kEnabled);
std::string metadata_frame_payload = qpack_encoder.EncodeHeaderList(
stream_->id(), headers,
/* encoder_stream_sent_byte_count = */ nullptr);
std::string metadata_frame_header =
- quic::HttpEncoder::SerializeMetadataFrameHeader(
- metadata_frame_payload.size());
+ HttpEncoder::SerializeMetadataFrameHeader(metadata_frame_payload.size());
std::string metadata_frame = metadata_frame_header + metadata_frame_payload;
EXPECT_CALL(metadata_visitor, OnMetadataComplete(metadata_frame.size(), _))
@@ -2812,14 +2813,14 @@
quiche::HttpHeaderBlock headers;
headers.AppendValueOrAddHeader("key1", "val1");
headers.AppendValueOrAddHeader("key2", "val2");
- quic::NoopDecoderStreamErrorDelegate delegate;
- QpackEncoder qpack_encoder(&delegate, quic::HuffmanEncoding::kDisabled);
+ NoopDecoderStreamErrorDelegate delegate;
+ QpackEncoder qpack_encoder(&delegate, HuffmanEncoding::kDisabled,
+ CookieCrumbling::kEnabled);
std::string metadata_frame_payload = qpack_encoder.EncodeHeaderList(
stream_->id(), headers,
/* encoder_stream_sent_byte_count = */ nullptr);
std::string metadata_frame_header =
- quic::HttpEncoder::SerializeMetadataFrameHeader(
- metadata_frame_payload.size());
+ HttpEncoder::SerializeMetadataFrameHeader(metadata_frame_payload.size());
std::string metadata_frame = metadata_frame_header + metadata_frame_payload;
EXPECT_CALL(*session_, WritevData(_, _, _, _, _, _)).Times(AnyNumber());
diff --git a/quiche/quic/core/qpack/fuzzer/qpack_round_trip_fuzzer.cc b/quiche/quic/core/qpack/fuzzer/qpack_round_trip_fuzzer.cc
index 08daf1b..cdaa12b 100644
--- a/quiche/quic/core/qpack/fuzzer/qpack_round_trip_fuzzer.cc
+++ b/quiche/quic/core/qpack/fuzzer/qpack_round_trip_fuzzer.cc
@@ -50,8 +50,10 @@
public:
EncodingEndpoint(uint64_t maximum_dynamic_table_capacity,
uint64_t maximum_blocked_streams,
- HuffmanEncoding huffman_encoding)
- : encoder_(&decoder_stream_error_delegate, huffman_encoding) {
+ HuffmanEncoding huffman_encoding,
+ CookieCrumbling cookie_crumbling)
+ : encoder_(&decoder_stream_error_delegate, huffman_encoding,
+ cookie_crumbling) {
encoder_.SetMaximumDynamicTableCapacity(maximum_dynamic_table_capacity);
encoder_.SetMaximumBlockedStreams(maximum_blocked_streams);
}
@@ -571,7 +573,8 @@
return header_list;
}
-// Splits |*header_list| header values along '\0' or ';' separators.
+// Splits |*header_list| header values. Cookie header is split along ';'
+// separator if crumbling is enabled. Other headers are split along '\0'.
QuicHeaderList SplitHeaderList(const quiche::HttpHeaderBlock& header_list,
CookieCrumbling cookie_crumbling) {
QuicHeaderList split_header_list;
@@ -607,10 +610,14 @@
const uint64_t maximum_blocked_streams = provider.ConsumeIntegral<uint8_t>();
// Set up encoder.
+ const CookieCrumbling cookie_crumbling = provider.ConsumeBool()
+ ? CookieCrumbling::kEnabled
+ : CookieCrumbling::kDisabled;
EncodingEndpoint encoder(maximum_dynamic_table_capacity,
maximum_blocked_streams,
provider.ConsumeBool() ? HuffmanEncoding::kEnabled
- : HuffmanEncoding::kDisabled);
+ : HuffmanEncoding::kDisabled,
+ cookie_crumbling);
// Set up decoder.
DecodingEndpoint decoder(maximum_dynamic_table_capacity,
@@ -647,10 +654,11 @@
// TODO(bnc): Randomly cancel the stream.
- // Encoder splits |header_list| header values along '\0' or ';' separators.
+ // Encoder splits |header_list| header values along '\0' or ';' separators
+ // (unless cookie crumbling is disabled).
// Do the same here so that we get matching results.
QuicHeaderList expected_header_list =
- SplitHeaderList(header_list, CookieCrumbling::kEnabled);
+ SplitHeaderList(header_list, cookie_crumbling);
decoder.AddExpectedHeaderList(stream_id, std::move(expected_header_list));
header_block_transmitter.SendEncodedHeaderBlock(
diff --git a/quiche/quic/core/qpack/qpack_encoder.cc b/quiche/quic/core/qpack/qpack_encoder.cc
index 0bdac87..c373a00 100644
--- a/quiche/quic/core/qpack/qpack_encoder.cc
+++ b/quiche/quic/core/qpack/qpack_encoder.cc
@@ -35,8 +35,9 @@
QpackEncoder::QpackEncoder(
DecoderStreamErrorDelegate* decoder_stream_error_delegate,
- HuffmanEncoding huffman_encoding)
+ HuffmanEncoding huffman_encoding, CookieCrumbling cookie_crumbling)
: huffman_encoding_(huffman_encoding),
+ cookie_crumbling_(cookie_crumbling),
decoder_stream_error_delegate_(decoder_stream_error_delegate),
decoder_stream_receiver_(this),
encoder_stream_sender_(huffman_encoding),
diff --git a/quiche/quic/core/qpack/qpack_encoder.h b/quiche/quic/core/qpack/qpack_encoder.h
index a631ac4..492bbbe 100644
--- a/quiche/quic/core/qpack/qpack_encoder.h
+++ b/quiche/quic/core/qpack/qpack_encoder.h
@@ -16,6 +16,7 @@
#include "quiche/quic/core/qpack/qpack_encoder_stream_sender.h"
#include "quiche/quic/core/qpack/qpack_header_table.h"
#include "quiche/quic/core/qpack/qpack_instructions.h"
+#include "quiche/quic/core/qpack/value_splitting_header_list.h"
#include "quiche/quic/core/quic_error_codes.h"
#include "quiche/quic/core/quic_types.h"
#include "quiche/quic/platform/api/quic_export.h"
@@ -46,7 +47,8 @@
};
QpackEncoder(DecoderStreamErrorDelegate* decoder_stream_error_delegate,
- HuffmanEncoding huffman_encoding);
+ HuffmanEncoding huffman_encoding,
+ CookieCrumbling cookie_crumbling);
~QpackEncoder() override;
// Encode a header list. If |encoder_stream_sent_byte_count| is not null,
@@ -148,6 +150,7 @@
uint64_t required_insert_count) const;
const HuffmanEncoding huffman_encoding_;
+ const CookieCrumbling cookie_crumbling_;
DecoderStreamErrorDelegate* const decoder_stream_error_delegate_;
QpackDecoderStreamReceiver decoder_stream_receiver_;
QpackEncoderStreamSender encoder_stream_sender_;
diff --git a/quiche/quic/core/qpack/qpack_encoder_test.cc b/quiche/quic/core/qpack/qpack_encoder_test.cc
index 13b7b6b..75b1194 100644
--- a/quiche/quic/core/qpack/qpack_encoder_test.cc
+++ b/quiche/quic/core/qpack/qpack_encoder_test.cc
@@ -11,6 +11,7 @@
#include "absl/strings/str_cat.h"
#include "absl/strings/string_view.h"
#include "quiche/quic/core/qpack/qpack_instruction_encoder.h"
+#include "quiche/quic/core/qpack/value_splitting_header_list.h"
#include "quiche/quic/platform/api/quic_flags.h"
#include "quiche/quic/platform/api/quic_test.h"
#include "quiche/quic/test_tools/qpack/qpack_encoder_peer.h"
@@ -58,7 +59,8 @@
protected:
QpackEncoderTest()
: huffman_encoding_(GetParam()),
- encoder_(&decoder_stream_error_delegate_, huffman_encoding_),
+ encoder_(&decoder_stream_error_delegate_, huffman_encoding_,
+ CookieCrumbling::kEnabled),
encoder_stream_sent_byte_count_(0) {
encoder_.set_qpack_stream_sender_delegate(&encoder_stream_sender_delegate_);
encoder_.SetMaximumBlockedStreams(1);
@@ -241,7 +243,8 @@
OnDecoderStreamError(QUIC_QPACK_DECODER_STREAM_INTEGER_TOO_LARGE,
Eq("Encoded integer too large.")));
- QpackEncoder encoder(&decoder_stream_error_delegate_, huffman_encoding_);
+ QpackEncoder encoder(&decoder_stream_error_delegate_, huffman_encoding_,
+ CookieCrumbling::kEnabled);
encoder.set_qpack_stream_sender_delegate(&encoder_stream_sender_delegate_);
std::string input;
ASSERT_TRUE(absl::HexStringToBytes("ffffffffffffffffffffff", &input));
@@ -1065,6 +1068,82 @@
EXPECT_EQ(expected_output, Encode(header_list3));
}
+TEST_P(QpackEncoderTest, CookieCrumblingEnabledNoDynamicTable) {
+ EXPECT_CALL(encoder_stream_sender_delegate_, NumBytesBuffered())
+ .WillRepeatedly(Return(0));
+
+ quiche::HttpHeaderBlock header_list;
+ header_list["cookie"] = "foo; bar";
+
+ std::string expected_output;
+ if (HuffmanEnabled()) {
+ ASSERT_TRUE(
+ absl::HexStringToBytes("0000" // prefix
+ "55" // name of static entry 5
+ "8294e7" // with literal value "bar"
+ "55" // name of static entry 5
+ "03626172", // with literal value "bar"
+ &expected_output));
+ } else {
+ ASSERT_TRUE(
+ absl::HexStringToBytes("0000" // prefix
+ "55" // name of static entry 5
+ "03666f6f" // with literal value "foo"
+ "55" // name of static entry 5
+ "03626172", // with literal value "bar"
+ &expected_output));
+ }
+ EXPECT_EQ(expected_output, Encode(header_list));
+
+ EXPECT_EQ(0u, encoder_stream_sent_byte_count_);
+}
+
+TEST_P(QpackEncoderTest, CookieCrumblingEnabledDynamicTable) {
+ EXPECT_CALL(encoder_stream_sender_delegate_, NumBytesBuffered())
+ .WillRepeatedly(Return(0));
+ encoder_.SetMaximumBlockedStreams(1);
+ encoder_.SetMaximumDynamicTableCapacity(4096);
+ encoder_.SetDynamicTableCapacity(4096);
+
+ quiche::HttpHeaderBlock header_list;
+ header_list["cookie"] = "foo; bar";
+
+ // Set Dynamic Table Capacity instruction.
+ std::string set_dyanamic_table_capacity;
+ ASSERT_TRUE(absl::HexStringToBytes("3fe11f", &set_dyanamic_table_capacity));
+
+ // Insert entries into the dynamic table.
+ std::string insert_entries;
+ if (HuffmanEnabled()) {
+ ASSERT_TRUE(absl::HexStringToBytes(
+ "c5" // insert with name reference, static index 5
+ "8294e7" // with literal value "foo"
+ "c5" // insert with name reference, static index 5
+ "03626172", // with literal value "bar"
+ &insert_entries));
+ } else {
+ ASSERT_TRUE(absl::HexStringToBytes(
+ "c5" // insert with name reference, static index 5
+ "03666f6f" // with literal value "foo"
+ "c5" // insert with name reference, static index 5
+ "03626172", // with literal value "bar"
+ &insert_entries));
+ }
+ EXPECT_CALL(encoder_stream_sender_delegate_,
+ WriteStreamData(Eq(
+ absl::StrCat(set_dyanamic_table_capacity, insert_entries))));
+
+ std::string expected_output;
+ ASSERT_TRUE(
+ absl::HexStringToBytes("0300" // prefix
+ "81" // dynamic entry with relative index 0
+ "80", // dynamic entry with relative index 1
+ &expected_output));
+ EXPECT_EQ(expected_output, Encode(header_list));
+
+ EXPECT_EQ(insert_entries.size(), encoder_stream_sent_byte_count_);
+}
+
} // namespace
} // namespace test
} // namespace quic
diff --git a/quiche/quic/core/qpack/qpack_round_trip_test.cc b/quiche/quic/core/qpack/qpack_round_trip_test.cc
index a03660b..bda7e40 100644
--- a/quiche/quic/core/qpack/qpack_round_trip_test.cc
+++ b/quiche/quic/core/qpack/qpack_round_trip_test.cc
@@ -8,6 +8,7 @@
#include "absl/strings/string_view.h"
#include "quiche/quic/core/qpack/qpack_decoder.h"
#include "quiche/quic/core/qpack/qpack_encoder.h"
+#include "quiche/quic/core/qpack/value_splitting_header_list.h"
#include "quiche/quic/platform/api/quic_test.h"
#include "quiche/quic/test_tools/qpack/qpack_decoder_test_utils.h"
#include "quiche/quic/test_tools/qpack/qpack_test_utils.h"
@@ -29,7 +30,7 @@
NoopDecoderStreamErrorDelegate decoder_stream_error_delegate;
NoopQpackStreamSenderDelegate encoder_stream_sender_delegate;
QpackEncoder encoder(&decoder_stream_error_delegate,
- HuffmanEncoding::kEnabled);
+ HuffmanEncoding::kEnabled, CookieCrumbling::kEnabled);
encoder.set_qpack_stream_sender_delegate(&encoder_stream_sender_delegate);
std::string encoded_header_block =
encoder.EncodeHeaderList(/* stream_id = */ 1, header_list, nullptr);
