Send Set Dynamic Table Capacity instruction.
Do not change dynamic table capacity in
QpackHeaderTable::SetMaximumDynamicTableCapacity(), because according to
https://quicwg.org/base-drafts/draft-ietf-quic-qpack.html#eviction, "The initial
capacity of the dynamic table is zero.". Instead, add
QpackEncoder::SetDynamicTableCapacity() to send Set Dynamic Table Capacity
instruction.
Roundtrip tests were passing because QpackHeaderTable::SetMaximumDynamicTableCapacity() incorrectly updated the capacity for both the encoder and decoder context.
gfe-relnote: n/a, change to QUIC v99-only code. Protected by existing disabled gfe2_reloadable_flag_quic_enable_version_99.
PiperOrigin-RevId: 266358801
Change-Id: I23f10f224139dee79e9305c42c85b75cba61a999
diff --git a/quic/core/http/quic_spdy_session.cc b/quic/core/http/quic_spdy_session.cc
index 101b33d..73f63bb 100644
--- a/quic/core/http/quic_spdy_session.cc
+++ b/quic/core/http/quic_spdy_session.cc
@@ -694,9 +694,13 @@
QUIC_DVLOG(1)
<< "SETTINGS_QPACK_MAX_TABLE_CAPACITY received with value "
<< value;
- // TODO(b/112770235): Limit value to
- // qpack_maximum_dynamic_table_capacity_.
+ // Communicate |value| to encoder, because it is used for encoding
+ // Required Insert Count.
qpack_encoder_->SetMaximumDynamicTableCapacity(value);
+ // However, limit the dynamic table capacity to
+ // |qpack_maximum_dynamic_table_capacity_|.
+ qpack_encoder_->SetDynamicTableCapacity(
+ std::min(value, qpack_maximum_dynamic_table_capacity_));
break;
case SETTINGS_MAX_HEADER_LIST_SIZE:
QUIC_DVLOG(1) << "SETTINGS_MAX_HEADER_LIST_SIZE received with value "
diff --git a/quic/core/http/quic_spdy_stream_test.cc b/quic/core/http/quic_spdy_stream_test.cc
index 3798461..e09cbf4 100644
--- a/quic/core/http/quic_spdy_stream_test.cc
+++ b/quic/core/http/quic_spdy_stream_test.cc
@@ -1921,6 +1921,7 @@
testing::InSequence s;
Initialize(kShouldProcessData);
+ session_->qpack_decoder()->OnSetDynamicTableCapacity(1024);
auto decoder_send_stream =
QuicSpdySessionPeer::GetQpackDecoderSendStream(session_.get());
@@ -1983,6 +1984,7 @@
testing::InSequence s;
Initialize(kShouldProcessData);
+ session_->qpack_decoder()->OnSetDynamicTableCapacity(1024);
// HEADERS frame referencing first dynamic table entry.
std::string headers = HeadersFrame(QuicTextUtils::HexDecode("020080"));
@@ -2040,6 +2042,7 @@
}
Initialize(kShouldProcessData);
+ session_->qpack_decoder()->OnSetDynamicTableCapacity(1024);
// HEADERS frame only referencing entry with absolute index 0 but with
// Required Insert Count = 2, which is incorrect.
@@ -2078,6 +2081,7 @@
testing::InSequence s;
Initialize(kShouldProcessData);
+ session_->qpack_decoder()->OnSetDynamicTableCapacity(1024);
// HEADERS frame referencing first dynamic table entry.
std::string headers = HeadersFrame(QuicTextUtils::HexDecode("020080"));
diff --git a/quic/core/qpack/fuzzer/qpack_round_trip_fuzzer.cc b/quic/core/qpack/fuzzer/qpack_round_trip_fuzzer.cc
index 624eca5..a26a6c3 100644
--- a/quic/core/qpack/fuzzer/qpack_round_trip_fuzzer.cc
+++ b/quic/core/qpack/fuzzer/qpack_round_trip_fuzzer.cc
@@ -45,6 +45,10 @@
encoder_.set_qpack_stream_sender_delegate(delegate);
}
+ void SetDynamicTableCapacity(uint64_t maximum_dynamic_table_capacity) {
+ encoder_.SetDynamicTableCapacity(maximum_dynamic_table_capacity);
+ }
+
QpackStreamReceiver* decoder_stream_receiver() {
return encoder_.decoder_stream_receiver();
}
@@ -596,6 +600,10 @@
decoder.encoder_stream_receiver(), &provider);
encoder.set_qpack_stream_sender_delegate(&encoder_stream_transmitter);
+ // Use a dynamic table as large as the peer allows. This sends data on the
+ // encoder stream, so it can only be done after delegate is set.
+ encoder.SetDynamicTableCapacity(maximum_dynamic_table_capacity);
+
// Transmit decoder stream data from encoder to decoder.
DelayedStreamDataTransmitter decoder_stream_transmitter(
encoder.decoder_stream_receiver(), &provider);
diff --git a/quic/core/qpack/qpack_decoded_headers_accumulator_test.cc b/quic/core/qpack/qpack_decoded_headers_accumulator_test.cc
index 2a0a222..1d60660 100644
--- a/quic/core/qpack/qpack_decoded_headers_accumulator_test.cc
+++ b/quic/core/qpack/qpack_decoded_headers_accumulator_test.cc
@@ -134,6 +134,8 @@
EXPECT_TRUE(accumulator_.Decode(QuicTextUtils::HexDecode("020080")));
EXPECT_EQ(Status::kBlocked, accumulator_.EndHeaderBlock());
+ // Set dynamic table capacity.
+ qpack_decoder_.OnSetDynamicTableCapacity(kMaxDynamicTableCapacity);
// Adding dynamic table entry unblocks decoding.
EXPECT_CALL(decoder_stream_sender_delegate_,
WriteStreamData(Eq(kHeaderAcknowledgement)));
@@ -147,6 +149,8 @@
// Reference to dynamic table entry not yet received.
EXPECT_TRUE(accumulator_.Decode(QuicTextUtils::HexDecode("020080")));
+ // Set dynamic table capacity.
+ qpack_decoder_.OnSetDynamicTableCapacity(kMaxDynamicTableCapacity);
// Adding dynamic table entry unblocks decoding.
qpack_decoder_.OnInsertWithoutNameReference("foo", "bar");
diff --git a/quic/core/qpack/qpack_decoder_test.cc b/quic/core/qpack/qpack_decoder_test.cc
index 0a4e5e2..ef711b9 100644
--- a/quic/core/qpack/qpack_decoder_test.cc
+++ b/quic/core/qpack/qpack_decoder_test.cc
@@ -301,6 +301,7 @@
TEST_P(QpackDecoderTest, DynamicTable) {
DecodeEncoderStreamData(QuicTextUtils::HexDecode(
+ "3fe107" // Set dynamic table capacity to 1024.
"6294e703626172" // Add literal entry with name "foo" and value "bar".
"80035a5a5a" // Add entry with name of dynamic table entry index 0
// (relative index) and value "ZZZ".
@@ -382,6 +383,8 @@
}
TEST_P(QpackDecoderTest, DecreasingDynamicTableCapacityEvictsEntries) {
+ // Set dynamic table capacity to 1024.
+ DecodeEncoderStreamData(QuicTextUtils::HexDecode("3fe107"));
// Add literal entry with name "foo" and value "bar".
DecodeEncoderStreamData(QuicTextUtils::HexDecode("6294e703626172"));
@@ -431,6 +434,7 @@
OnEncoderStreamError(Eq("Invalid relative index.")));
DecodeEncoderStreamData(QuicTextUtils::HexDecode(
+ "3fe107" // Set dynamic table capacity to 1024.
"6294e703626172" // Add literal entry with name "foo" and value "bar".
"8100")); // Address dynamic table entry with relative index 1. Such
// entry does not exist. The most recently added and only
@@ -442,6 +446,7 @@
OnEncoderStreamError(Eq("Invalid relative index.")));
DecodeEncoderStreamData(QuicTextUtils::HexDecode(
+ "3fe107" // Set dynamic table capacity to 1024.
"6294e703626172" // Add literal entry with name "foo" and value "bar".
"01")); // Duplicate dynamic table entry with relative index 1. Such
// entry does not exist. The most recently added and only
@@ -458,6 +463,8 @@
TEST_P(QpackDecoderTest, InvalidDynamicEntryWhenBaseIsZero) {
EXPECT_CALL(handler_, OnDecodingErrorDetected(Eq("Invalid relative index.")));
+ // Set dynamic table capacity to 1024.
+ DecodeEncoderStreamData(QuicTextUtils::HexDecode("3fe107"));
// Add literal entry with name "foo" and value "bar".
DecodeEncoderStreamData(QuicTextUtils::HexDecode("6294e703626172"));
@@ -477,6 +484,8 @@
}
TEST_P(QpackDecoderTest, InvalidDynamicEntryByRelativeIndex) {
+ // Set dynamic table capacity to 1024.
+ DecodeEncoderStreamData(QuicTextUtils::HexDecode("3fe107"));
// Add literal entry with name "foo" and value "bar".
DecodeEncoderStreamData(QuicTextUtils::HexDecode("6294e703626172"));
@@ -585,6 +594,8 @@
// Maximum dynamic table capacity is 1024.
// MaxEntries is 1024 / 32 = 32.
+ // Set dynamic table capacity to 1024.
+ DecodeEncoderStreamData(QuicTextUtils::HexDecode("3fe107"));
// Add literal entry with name "foo" and a 600 byte long value. This will fit
// in the dynamic table once but not twice.
DecodeEncoderStreamData(
@@ -611,6 +622,8 @@
}
TEST_P(QpackDecoderTest, NonZeroRequiredInsertCountButNoDynamicEntries) {
+ // Set dynamic table capacity to 1024.
+ DecodeEncoderStreamData(QuicTextUtils::HexDecode("3fe107"));
// Add literal entry with name "foo" and value "bar".
DecodeEncoderStreamData(QuicTextUtils::HexDecode("6294e703626172"));
@@ -624,6 +637,8 @@
}
TEST_P(QpackDecoderTest, AddressEntryNotAllowedByRequiredInsertCount) {
+ // Set dynamic table capacity to 1024.
+ DecodeEncoderStreamData(QuicTextUtils::HexDecode("3fe107"));
// Add literal entry with name "foo" and value "bar".
DecodeEncoderStreamData(QuicTextUtils::HexDecode("6294e703626172"));
@@ -680,6 +695,8 @@
}
TEST_P(QpackDecoderTest, PromisedRequiredInsertCountLargerThanActual) {
+ // Set dynamic table capacity to 1024.
+ DecodeEncoderStreamData(QuicTextUtils::HexDecode("3fe107"));
// Add literal entry with name "foo" and value "bar".
DecodeEncoderStreamData(QuicTextUtils::HexDecode("6294e703626172"));
// Duplicate entry twice so that decoding of header blocks with Required
@@ -748,6 +765,8 @@
EXPECT_CALL(decoder_stream_sender_delegate_,
WriteStreamData(Eq(kHeaderAcknowledgement)));
+ // Set dynamic table capacity to 1024.
+ DecodeEncoderStreamData(QuicTextUtils::HexDecode("3fe107"));
// Add literal entry with name "foo" and value "bar".
DecodeEncoderStreamData(QuicTextUtils::HexDecode("6294e703626172"));
}
@@ -767,6 +786,10 @@
// the already consumed part of the header block.
EXPECT_CALL(handler_, OnHeaderDecoded(Eq("foo"), Eq("bar")));
EXPECT_CALL(handler_, OnHeaderDecoded(Eq(":method"), Eq("GET")));
+
+ // Set dynamic table capacity to 1024.
+ DecodeEncoderStreamData(QuicTextUtils::HexDecode("3fe107"));
+ // Add literal entry with name "foo" and value "bar".
DecodeEncoderStreamData(QuicTextUtils::HexDecode("6294e703626172"));
Mock::VerifyAndClearExpectations(&handler_);
diff --git a/quic/core/qpack/qpack_encoder.cc b/quic/core/qpack/qpack_encoder.cc
index 370887a..d1736f8 100644
--- a/quic/core/qpack/qpack_encoder.cc
+++ b/quic/core/qpack/qpack_encoder.cc
@@ -318,11 +318,15 @@
void QpackEncoder::SetMaximumDynamicTableCapacity(
uint64_t maximum_dynamic_table_capacity) {
- // TODO(b/112770235): Send set dynamic table capacity instruction on encoder
- // stream.
header_table_.SetMaximumDynamicTableCapacity(maximum_dynamic_table_capacity);
}
+void QpackEncoder::SetDynamicTableCapacity(uint64_t dynamic_table_capacity) {
+ encoder_stream_sender_.SendSetDynamicTableCapacity(dynamic_table_capacity);
+ bool success = header_table_.SetDynamicTableCapacity(dynamic_table_capacity);
+ DCHECK(success);
+}
+
void QpackEncoder::SetMaximumBlockedStreams(uint64_t maximum_blocked_streams) {
maximum_blocked_streams_ = maximum_blocked_streams;
}
diff --git a/quic/core/qpack/qpack_encoder.h b/quic/core/qpack/qpack_encoder.h
index 993b76e..4aa69c1 100644
--- a/quic/core/qpack/qpack_encoder.h
+++ b/quic/core/qpack/qpack_encoder.h
@@ -53,12 +53,17 @@
std::string EncodeHeaderList(QuicStreamId stream_id,
const spdy::SpdyHeaderBlock& header_list);
- // Set maximum capacity of dynamic table, measured in bytes.
- // Called when SETTINGS_QPACK_MAX_TABLE_CAPACITY is received.
- // Sends set dynamic table capacity instruction on encoder stream.
- // TODO(b/112770235): Actually send set dynamic table capacity instruction.
+ // Set maximum dynamic table capacity to |maximum_dynamic_table_capacity|,
+ // measured in bytes. Called when SETTINGS_QPACK_MAX_TABLE_CAPACITY is
+ // received. Encoder needs to know this value so that it can calculate
+ // MaxEntries, used as a modulus to encode Required Insert Count.
void SetMaximumDynamicTableCapacity(uint64_t maximum_dynamic_table_capacity);
+ // Set dynamic table capacity to |dynamic_table_capacity|.
+ // |dynamic_table_capacity| must not exceed maximum dynamic table capacity.
+ // Also sends Set Dynamic Table Capacity instruction on encoder stream.
+ void SetDynamicTableCapacity(uint64_t dynamic_table_capacity);
+
// Set maximum number of blocked streams.
// Called when SETTINGS_QPACK_BLOCKED_STREAMS is received.
void SetMaximumBlockedStreams(uint64_t maximum_blocked_streams);
diff --git a/quic/core/qpack/qpack_encoder_test.cc b/quic/core/qpack/qpack_encoder_test.cc
index f23cb8b..0da6f3e 100644
--- a/quic/core/qpack/qpack_encoder_test.cc
+++ b/quic/core/qpack/qpack_encoder_test.cc
@@ -11,6 +11,8 @@
#include "net/third_party/quiche/src/quic/core/qpack/qpack_utils.h"
#include "net/third_party/quiche/src/quic/platform/api/quic_test.h"
#include "net/third_party/quiche/src/quic/platform/api/quic_text_utils.h"
+#include "net/third_party/quiche/src/quic/test_tools/qpack_encoder_peer.h"
+#include "net/third_party/quiche/src/quic/test_tools/qpack_header_table_peer.h"
using ::testing::_;
using ::testing::Eq;
@@ -182,8 +184,13 @@
}
TEST_F(QpackEncoderTest, DynamicTable) {
- encoder_.SetMaximumDynamicTableCapacity(4096);
encoder_.SetMaximumBlockedStreams(1);
+ encoder_.SetMaximumDynamicTableCapacity(4096);
+
+ // Set Dynamic Table Capacity instruction.
+ EXPECT_CALL(encoder_stream_sender_delegate_,
+ WriteStreamData(Eq(QuicTextUtils::HexDecode("3fe11f"))));
+ encoder_.SetDynamicTableCapacity(4096);
spdy::SpdyHeaderBlock header_list;
header_list["foo"] = "bar";
@@ -214,8 +221,13 @@
// There is no room in the dynamic table after inserting the first entry.
TEST_F(QpackEncoderTest, SmallDynamicTable) {
- encoder_.SetMaximumDynamicTableCapacity(QpackEntry::Size("foo", "bar"));
encoder_.SetMaximumBlockedStreams(1);
+ encoder_.SetMaximumDynamicTableCapacity(QpackEntry::Size("foo", "bar"));
+
+ // Set Dynamic Table Capacity instruction.
+ EXPECT_CALL(encoder_stream_sender_delegate_,
+ WriteStreamData(Eq(QuicTextUtils::HexDecode("3f07"))));
+ encoder_.SetDynamicTableCapacity(QpackEntry::Size("foo", "bar"));
spdy::SpdyHeaderBlock header_list;
header_list["foo"] = "bar";
@@ -243,8 +255,13 @@
}
TEST_F(QpackEncoderTest, BlockedStream) {
- encoder_.SetMaximumDynamicTableCapacity(4096);
encoder_.SetMaximumBlockedStreams(1);
+ encoder_.SetMaximumDynamicTableCapacity(4096);
+
+ // Set Dynamic Table Capacity instruction.
+ EXPECT_CALL(encoder_stream_sender_delegate_,
+ WriteStreamData(Eq(QuicTextUtils::HexDecode("3fe11f"))));
+ encoder_.SetDynamicTableCapacity(4096);
spdy::SpdyHeaderBlock header_list1;
header_list1["foo"] = "bar";
@@ -362,6 +379,10 @@
maximum_dynamic_table_capacity += QpackEntry::Size("one", "foo");
encoder_.SetMaximumDynamicTableCapacity(maximum_dynamic_table_capacity);
+ // Set Dynamic Table Capacity instruction.
+ EXPECT_CALL(encoder_stream_sender_delegate_, WriteStreamData(_));
+ encoder_.SetDynamicTableCapacity(maximum_dynamic_table_capacity);
+
// Insert ten entries into the dynamic table.
EXPECT_CALL(encoder_stream_sender_delegate_, WriteStreamData(_)).Times(10);
@@ -399,6 +420,21 @@
Encode(header_list3));
}
+TEST_F(QpackEncoderTest, DynamicTableCapacityLessThanMaximum) {
+ encoder_.SetMaximumDynamicTableCapacity(1024);
+
+ // Set Dynamic Table Capacity instruction.
+ EXPECT_CALL(encoder_stream_sender_delegate_,
+ WriteStreamData(Eq(QuicTextUtils::HexDecode("3e"))));
+ encoder_.SetDynamicTableCapacity(30);
+
+ QpackHeaderTable* header_table = QpackEncoderPeer::header_table(&encoder_);
+
+ EXPECT_EQ(1024u,
+ QpackHeaderTablePeer::maximum_dynamic_table_capacity(header_table));
+ EXPECT_EQ(30u, QpackHeaderTablePeer::dynamic_table_capacity(header_table));
+}
+
} // namespace
} // namespace test
} // namespace quic
diff --git a/quic/core/qpack/qpack_header_table.cc b/quic/core/qpack/qpack_header_table.cc
index 3c9296d..bb28f72 100644
--- a/quic/core/qpack/qpack_header_table.cc
+++ b/quic/core/qpack/qpack_header_table.cc
@@ -184,7 +184,6 @@
DCHECK_EQ(0u, maximum_dynamic_table_capacity_);
DCHECK_EQ(0u, max_entries_);
- dynamic_table_capacity_ = maximum_dynamic_table_capacity;
maximum_dynamic_table_capacity_ = maximum_dynamic_table_capacity;
max_entries_ = maximum_dynamic_table_capacity / 32;
}
diff --git a/quic/core/qpack/qpack_header_table_test.cc b/quic/core/qpack/qpack_header_table_test.cc
index 592d2cb..00f36e5 100644
--- a/quic/core/qpack/qpack_header_table_test.cc
+++ b/quic/core/qpack/qpack_header_table_test.cc
@@ -30,6 +30,7 @@
QpackHeaderTableTest() {
table_.SetMaximumDynamicTableCapacity(
kMaximumDynamicTableCapacityForTesting);
+ table_.SetDynamicTableCapacity(kMaximumDynamicTableCapacityForTesting);
}
~QpackHeaderTableTest() override = default;
@@ -371,6 +372,7 @@
const uint64_t dynamic_table_capacity = 100;
QpackHeaderTable table;
table.SetMaximumDynamicTableCapacity(dynamic_table_capacity);
+ EXPECT_TRUE(table.SetDynamicTableCapacity(dynamic_table_capacity));
// Empty table can take an entry up to its capacity.
EXPECT_EQ(dynamic_table_capacity,
@@ -455,7 +457,9 @@
TEST_F(QpackHeaderTableTest, DrainingIndex) {
QpackHeaderTable table;
- table.SetMaximumDynamicTableCapacity(4 * QpackEntry::Size("foo", "bar"));
+ table.SetMaximumDynamicTableCapacity(kMaximumDynamicTableCapacityForTesting);
+ EXPECT_TRUE(
+ table.SetDynamicTableCapacity(4 * QpackEntry::Size("foo", "bar")));
// Empty table: no draining entry.
EXPECT_EQ(0u, table.draining_index(0.0));
