Pass maximum table capacity and max blocked streams in QpackDecoder constructor.
These two limits are set by the decoder using SETTINGS_QPACK_MAX_TABLE_CAPACITY
and SETTINGS_QPACK_BLOCKED_STREAMS, therefore they can be decided by
QuicSpdyStream or higher layers by the time QpackDecoder is constructed.
Therefore the constructor seems the most appropriate place to communicate these
to QpackDecoder.
(This is in contrast with the value of these settings in the encoder, which
are received via SETTINGS from the peer after QpackEncoder is instantiated.)
gfe-relnote: n/a, change to QUIC v99-only code. Protected by existing disabled gfe2_reloadable_flag_quic_enable_version_99.
PiperOrigin-RevId: 261601336
Change-Id: I1449b63e67e387714a3bc30b6488571277281475
diff --git a/quic/core/http/quic_spdy_session.cc b/quic/core/http/quic_spdy_session.cc
index 2a83d24..ad13dc1 100644
--- a/quic/core/http/quic_spdy_session.cc
+++ b/quic/core/http/quic_spdy_session.cc
@@ -365,11 +365,14 @@
qpack_encoder_ =
QuicMakeUnique<QpackEncoder>(this, &encoder_stream_sender_delegate_);
qpack_decoder_ =
- QuicMakeUnique<QpackDecoder>(this, &decoder_stream_sender_delegate_);
+ QuicMakeUnique<QpackDecoder>(kDefaultQpackMaxDynamicTableCapacity,
+ /* maximum_blocked_streams = */ 0, this,
+ &decoder_stream_sender_delegate_);
+ // TODO(b/112770235): Set sensible limit on maximum number of blocked
+ // streams.
// TODO(b/112770235): Send SETTINGS_QPACK_MAX_TABLE_CAPACITY with value
- // kDefaultQpackMaxDynamicTableCapacity.
- qpack_decoder_->SetMaximumDynamicTableCapacity(
- kDefaultQpackMaxDynamicTableCapacity);
+ // kDefaultQpackMaxDynamicTableCapacity, and SETTINGS_QPACK_BLOCKED_STREAMS
+ // with limit on maximum number of blocked streams.
}
if (VersionHasStreamType(connection()->transport_version())) {
diff --git a/quic/core/qpack/fuzzer/qpack_decoder_fuzzer.cc b/quic/core/qpack/fuzzer/qpack_decoder_fuzzer.cc
index 1a2aa4d..e447bc8 100644
--- a/quic/core/qpack/fuzzer/qpack_decoder_fuzzer.cc
+++ b/quic/core/qpack/fuzzer/qpack_decoder_fuzzer.cc
@@ -32,9 +32,12 @@
NoopEncoderStreamErrorDelegate encoder_stream_error_delegate;
NoopQpackStreamSenderDelegate decoder_stream_sender_delegate;
- QpackDecode(&encoder_stream_error_delegate, &decoder_stream_sender_delegate,
- &handler, fragment_size_generator,
- provider.ConsumeRemainingBytesAsString());
+ // TODO(b/112770235): Fuzz dynamic table and blocked streams.
+ QpackDecode(
+ /* maximum_dynamic_table_capacity = */ 0,
+ /* maximum_blocked_streams = */ 0, &encoder_stream_error_delegate,
+ &decoder_stream_sender_delegate, &handler, fragment_size_generator,
+ provider.ConsumeRemainingBytesAsString());
return 0;
}
diff --git a/quic/core/qpack/fuzzer/qpack_round_trip_fuzzer.cc b/quic/core/qpack/fuzzer/qpack_round_trip_fuzzer.cc
index a677ab4..fceaac5 100644
--- a/quic/core/qpack/fuzzer/qpack_round_trip_fuzzer.cc
+++ b/quic/core/qpack/fuzzer/qpack_round_trip_fuzzer.cc
@@ -137,8 +137,12 @@
TestHeadersHandler handler;
NoopEncoderStreamErrorDelegate encoder_stream_error_delegate;
NoopQpackStreamSenderDelegate decoder_stream_sender_delegate;
- QpackDecode(&encoder_stream_error_delegate, &decoder_stream_sender_delegate,
- &handler, fragment_size_generator, encoded_header_block);
+ // TODO(b/112770235): Fuzz dynamic table and blocked streams.
+ QpackDecode(
+ /* maximum_dynamic_table_capacity = */ 0,
+ /* maximum_blocked_streams = */ 0, &encoder_stream_error_delegate,
+ &decoder_stream_sender_delegate, &handler, fragment_size_generator,
+ encoded_header_block);
// Since header block has been produced by encoding a header list, it must be
// valid.
diff --git a/quic/core/qpack/offline/qpack_offline_decoder.cc b/quic/core/qpack/offline/qpack_offline_decoder.cc
index 20af074..267c679 100644
--- a/quic/core/qpack/offline/qpack_offline_decoder.cc
+++ b/quic/core/qpack/offline/qpack_offline_decoder.cc
@@ -20,7 +20,6 @@
QpackOfflineDecoder::QpackOfflineDecoder()
: encoder_stream_error_detected_(false),
- qpack_decoder_(this, &decoder_stream_sender_delegate_),
max_blocked_streams_(0) {}
bool QpackOfflineDecoder::DecodeAndVerifyOfflineData(
@@ -93,8 +92,9 @@
<< "\" as an integer.";
return false;
}
-
- qpack_decoder_.SetMaximumDynamicTableCapacity(maximum_dynamic_table_capacity);
+ qpack_decoder_ = QuicMakeUnique<QpackDecoder>(
+ maximum_dynamic_table_capacity, max_blocked_streams_, this,
+ &decoder_stream_sender_delegate_);
return true;
}
@@ -133,7 +133,7 @@
// Process data.
if (stream_id == 0) {
- qpack_decoder_.DecodeEncoderStreamData(data);
+ qpack_decoder_->DecodeEncoderStreamData(data);
if (encoder_stream_error_detected_) {
QUIC_LOG(ERROR) << "Error detected on encoder stream.";
@@ -141,7 +141,7 @@
}
} else {
auto headers_handler = QuicMakeUnique<test::TestHeadersHandler>();
- auto progressive_decoder = qpack_decoder_.CreateProgressiveDecoder(
+ auto progressive_decoder = qpack_decoder_->CreateProgressiveDecoder(
stream_id, headers_handler.get());
progressive_decoder->Decode(data);
@@ -183,6 +183,7 @@
}
// Enforce limit on blocked streams.
+ // TODO(b/112770235): Move this logic to QpackDecoder.
uint64_t blocked_streams_count = 0;
for (const auto& decoder : decoders_) {
if (!decoder.headers_handler->decoding_completed()) {
diff --git a/quic/core/qpack/offline/qpack_offline_decoder.h b/quic/core/qpack/offline/qpack_offline_decoder.h
index 9984d4b..dcb8894 100644
--- a/quic/core/qpack/offline/qpack_offline_decoder.h
+++ b/quic/core/qpack/offline/qpack_offline_decoder.h
@@ -72,7 +72,7 @@
bool encoder_stream_error_detected_;
NoopQpackStreamSenderDelegate decoder_stream_sender_delegate_;
- QpackDecoder qpack_decoder_;
+ std::unique_ptr<QpackDecoder> qpack_decoder_;
uint64_t max_blocked_streams_;
// Objects necessary for decoding, one list element for each header block.
diff --git a/quic/core/qpack/qpack_decoded_headers_accumulator_test.cc b/quic/core/qpack/qpack_decoded_headers_accumulator_test.cc
index 0898327..ed5a4c0 100644
--- a/quic/core/qpack/qpack_decoded_headers_accumulator_test.cc
+++ b/quic/core/qpack/qpack_decoded_headers_accumulator_test.cc
@@ -31,6 +31,9 @@
// Maximum dynamic table capacity.
const size_t kMaxDynamicTableCapacity = 100;
+// Maximum number of blocked streams.
+const uint64_t kMaximumBlockedStreams = 1;
+
// Header Acknowledgement decoder stream instruction with stream_id = 1.
const char* const kHeaderAcknowledgement = "\x81";
@@ -46,7 +49,9 @@
class QpackDecodedHeadersAccumulatorTest : public QuicTest {
protected:
QpackDecodedHeadersAccumulatorTest()
- : qpack_decoder_(&encoder_stream_error_delegate_,
+ : qpack_decoder_(kMaxDynamicTableCapacity,
+ kMaximumBlockedStreams,
+ &encoder_stream_error_delegate_,
&decoder_stream_sender_delegate_),
accumulator_(kTestStreamId,
&qpack_decoder_,
@@ -132,8 +137,6 @@
}
TEST_F(QpackDecodedHeadersAccumulatorTest, BlockedDecoding) {
- qpack_decoder_.SetMaximumDynamicTableCapacity(kMaxDynamicTableCapacity);
-
// Reference to dynamic table entry not yet received.
EXPECT_TRUE(accumulator_.Decode(QuicTextUtils::HexDecode("020080")));
EXPECT_EQ(Status::kBlocked, accumulator_.EndHeaderBlock());
@@ -148,8 +151,6 @@
TEST_F(QpackDecodedHeadersAccumulatorTest,
BlockedDecodingUnblockedBeforeEndOfHeaderBlock) {
- qpack_decoder_.SetMaximumDynamicTableCapacity(kMaxDynamicTableCapacity);
-
// Reference to dynamic table entry not yet received.
EXPECT_TRUE(accumulator_.Decode(QuicTextUtils::HexDecode("020080")));
diff --git a/quic/core/qpack/qpack_decoder.cc b/quic/core/qpack/qpack_decoder.cc
index d0e5ad9..995bb3f 100644
--- a/quic/core/qpack/qpack_decoder.cc
+++ b/quic/core/qpack/qpack_decoder.cc
@@ -12,22 +12,22 @@
namespace quic {
QpackDecoder::QpackDecoder(
+ uint64_t maximum_dynamic_table_capacity,
+ uint64_t maximum_blocked_streams,
EncoderStreamErrorDelegate* encoder_stream_error_delegate,
QpackStreamSenderDelegate* decoder_stream_sender_delegate)
: encoder_stream_error_delegate_(encoder_stream_error_delegate),
encoder_stream_receiver_(this),
- decoder_stream_sender_(decoder_stream_sender_delegate) {
+ decoder_stream_sender_(decoder_stream_sender_delegate),
+ maximum_blocked_streams_(maximum_blocked_streams) {
DCHECK(encoder_stream_error_delegate_);
DCHECK(decoder_stream_sender_delegate);
+
+ header_table_.SetMaximumDynamicTableCapacity(maximum_dynamic_table_capacity);
}
QpackDecoder::~QpackDecoder() {}
-void QpackDecoder::SetMaximumDynamicTableCapacity(
- uint64_t maximum_dynamic_table_capacity) {
- header_table_.SetMaximumDynamicTableCapacity(maximum_dynamic_table_capacity);
-}
-
void QpackDecoder::OnStreamReset(QuicStreamId stream_id) {
decoder_stream_sender_.SendStreamCancellation(stream_id);
}
diff --git a/quic/core/qpack/qpack_decoder.h b/quic/core/qpack/qpack_decoder.h
index 9f8a3e3..a0f233e 100644
--- a/quic/core/qpack/qpack_decoder.h
+++ b/quic/core/qpack/qpack_decoder.h
@@ -34,14 +34,12 @@
virtual void OnEncoderStreamError(QuicStringPiece error_message) = 0;
};
- QpackDecoder(EncoderStreamErrorDelegate* encoder_stream_error_delegate,
+ QpackDecoder(uint64_t maximum_dynamic_table_capacity,
+ uint64_t maximum_blocked_streams,
+ EncoderStreamErrorDelegate* encoder_stream_error_delegate,
QpackStreamSenderDelegate* decoder_stream_sender_delegate);
~QpackDecoder() override;
- // Set maximum capacity of dynamic table.
- // This method must only be called at most once.
- void SetMaximumDynamicTableCapacity(uint64_t maximum_dynamic_table_capacity);
-
// Signal to the peer's encoder that a stream is reset. This lets the peer's
// encoder know that no more header blocks will be processed on this stream,
// therefore references to dynamic table entries shall not prevent their
@@ -94,6 +92,7 @@
QpackEncoderStreamReceiver encoder_stream_receiver_;
QpackDecoderStreamSender decoder_stream_sender_;
QpackHeaderTable header_table_;
+ const uint64_t maximum_blocked_streams_;
};
} // namespace quic
diff --git a/quic/core/qpack/qpack_decoder_test.cc b/quic/core/qpack/qpack_decoder_test.cc
index af3a6a2..4b57456 100644
--- a/quic/core/qpack/qpack_decoder_test.cc
+++ b/quic/core/qpack/qpack_decoder_test.cc
@@ -26,14 +26,18 @@
// Header Acknowledgement decoder stream instruction with stream_id = 1.
const char* const kHeaderAcknowledgement = "\x81";
+// TODO(b/112770235) Change this constant, enforce the limit and add tests.
+const uint64_t kMaximumBlockedStreams = 0;
+
class QpackDecoderTest : public QuicTestWithParam<FragmentMode> {
protected:
QpackDecoderTest()
- : qpack_decoder_(&encoder_stream_error_delegate_,
- &decoder_stream_sender_delegate_),
- fragment_mode_(GetParam()) {
- qpack_decoder_.SetMaximumDynamicTableCapacity(1024);
- }
+ : qpack_decoder_(
+ /* maximum_dynamic_table_capacity = */ 1024,
+ kMaximumBlockedStreams,
+ &encoder_stream_error_delegate_,
+ &decoder_stream_sender_delegate_),
+ fragment_mode_(GetParam()) {}
~QpackDecoderTest() override = default;
@@ -566,7 +570,7 @@
DecodeEncoderStreamData(QuicTextUtils::HexDecode("3fe10f"));
}
-TEST_P(QpackDecoderTest, SetMaximumDynamicTableCapacity) {
+TEST_P(QpackDecoderTest, SetDynamicTableCapacity) {
// Update dynamic table capacity to 128, which does not exceed the maximum.
DecodeEncoderStreamData(QuicTextUtils::HexDecode("3f61"));
}
diff --git a/quic/core/qpack/qpack_decoder_test_utils.cc b/quic/core/qpack/qpack_decoder_test_utils.cc
index 173270d..ce990b5 100644
--- a/quic/core/qpack/qpack_decoder_test_utils.cc
+++ b/quic/core/qpack/qpack_decoder_test_utils.cc
@@ -64,12 +64,15 @@
}
void QpackDecode(
+ uint64_t maximum_dynamic_table_capacity,
+ uint64_t maximum_blocked_streams,
QpackDecoder::EncoderStreamErrorDelegate* encoder_stream_error_delegate,
QpackStreamSenderDelegate* decoder_stream_sender_delegate,
QpackProgressiveDecoder::HeadersHandlerInterface* handler,
const FragmentSizeGenerator& fragment_size_generator,
QuicStringPiece data) {
- QpackDecoder decoder(encoder_stream_error_delegate,
+ QpackDecoder decoder(maximum_dynamic_table_capacity, maximum_blocked_streams,
+ encoder_stream_error_delegate,
decoder_stream_sender_delegate);
auto progressive_decoder =
decoder.CreateProgressiveDecoder(/* stream_id = */ 1, handler);
diff --git a/quic/core/qpack/qpack_decoder_test_utils.h b/quic/core/qpack/qpack_decoder_test_utils.h
index d5f494c..6505b60 100644
--- a/quic/core/qpack/qpack_decoder_test_utils.h
+++ b/quic/core/qpack/qpack_decoder_test_utils.h
@@ -89,6 +89,8 @@
};
void QpackDecode(
+ uint64_t maximum_dynamic_table_capacity,
+ uint64_t maximum_blocked_streams,
QpackDecoder::EncoderStreamErrorDelegate* encoder_stream_error_delegate,
QpackStreamSenderDelegate* decoder_stream_sender_delegate,
QpackProgressiveDecoder::HeadersHandlerInterface* handler,
diff --git a/quic/core/qpack/qpack_round_trip_test.cc b/quic/core/qpack/qpack_round_trip_test.cc
index 1e9de2d..6ad6003 100644
--- a/quic/core/qpack/qpack_round_trip_test.cc
+++ b/quic/core/qpack/qpack_round_trip_test.cc
@@ -37,9 +37,12 @@
TestHeadersHandler handler;
NoopEncoderStreamErrorDelegate encoder_stream_error_delegate;
NoopQpackStreamSenderDelegate decoder_stream_sender_delegate;
- QpackDecode(&encoder_stream_error_delegate, &decoder_stream_sender_delegate,
- &handler, FragmentModeToFragmentSizeGenerator(GetParam()),
- encoded_header_block);
+ // TODO(b/112770235): Test dynamic table and blocked streams.
+ QpackDecode(
+ /* maximum_dynamic_table_capacity = */ 0,
+ /* maximum_blocked_streams = */ 0, &encoder_stream_error_delegate,
+ &decoder_stream_sender_delegate, &handler,
+ FragmentModeToFragmentSizeGenerator(GetParam()), encoded_header_block);
EXPECT_TRUE(handler.decoding_completed());
EXPECT_FALSE(handler.decoding_error_detected());
