Blocked decoding part 2: QpackProgressiveDecoder.
gfe-relnote: n/a, QUIC v99-only change.
PiperOrigin-RevId: 257158970
Change-Id: If3aad9939945daf170042038978ab7739078a8bd
diff --git a/quic/core/qpack/qpack_decoder_test.cc b/quic/core/qpack/qpack_decoder_test.cc
index 185aab9..af3a6a2 100644
--- a/quic/core/qpack/qpack_decoder_test.cc
+++ b/quic/core/qpack/qpack_decoder_test.cc
@@ -14,6 +14,7 @@
#include "net/third_party/quiche/src/spdy/core/spdy_header_block.h"
using ::testing::Eq;
+using ::testing::Mock;
using ::testing::Sequence;
using ::testing::StrictMock;
using ::testing::Values;
@@ -40,17 +41,36 @@
qpack_decoder_.DecodeEncoderStreamData(data);
}
- void DecodeHeaderBlock(QuicStringPiece data) {
+ // Set up |progressive_decoder_|.
+ void StartDecoding() {
+ progressive_decoder_ =
+ qpack_decoder_.CreateProgressiveDecoder(/* stream_id = */ 1, &handler_);
+ }
+
+ // Pass header block data to QpackProgressiveDecoder::Decode()
+ // in fragments dictated by |fragment_mode_|.
+ void DecodeData(QuicStringPiece data) {
auto fragment_size_generator =
FragmentModeToFragmentSizeGenerator(fragment_mode_);
- auto progressive_decoder =
- qpack_decoder_.CreateProgressiveDecoder(/* stream_id = */ 1, &handler_);
while (!data.empty()) {
size_t fragment_size = std::min(fragment_size_generator(), data.size());
- progressive_decoder->Decode(data.substr(0, fragment_size));
+ progressive_decoder_->Decode(data.substr(0, fragment_size));
data = data.substr(fragment_size);
}
- progressive_decoder->EndHeaderBlock();
+ }
+
+ // Signal end of header block to QpackProgressiveDecoder.
+ void EndDecoding() {
+ progressive_decoder_->EndHeaderBlock();
+ // |progressive_decoder_| is kept alive so that it can
+ // handle callbacks later in case of blocked decoding.
+ }
+
+ // Decode an entire header block.
+ void DecodeHeaderBlock(QuicStringPiece data) {
+ StartDecoding();
+ DecodeData(data);
+ EndDecoding();
}
StrictMock<MockEncoderStreamErrorDelegate> encoder_stream_error_delegate_;
@@ -60,6 +80,7 @@
private:
QpackDecoder qpack_decoder_;
const FragmentMode fragment_mode_;
+ std::unique_ptr<QpackProgressiveDecoder> progressive_decoder_;
};
INSTANTIATE_TEST_SUITE_P(,
@@ -387,8 +408,8 @@
// This must cause the entry to be evicted.
DecodeEncoderStreamData(QuicTextUtils::HexDecode("3f01"));
- EXPECT_CALL(handler_,
- OnDecodingErrorDetected(Eq("Dynamic table entry not found.")));
+ EXPECT_CALL(handler_, OnDecodingErrorDetected(
+ Eq("Dynamic table entry already evicted.")));
DecodeHeaderBlock(QuicTextUtils::HexDecode(
"0200" // Required Insert Count 1 and Delta Base 0.
@@ -446,6 +467,9 @@
TEST_P(QpackDecoderTest, InvalidDynamicEntryWhenBaseIsZero) {
EXPECT_CALL(handler_, OnDecodingErrorDetected(Eq("Invalid relative index.")));
+ // Add literal entry with name "foo" and value "bar".
+ DecodeEncoderStreamData(QuicTextUtils::HexDecode("6294e703626172"));
+
DecodeHeaderBlock(QuicTextUtils::HexDecode(
"0280" // Required Insert Count is 1. Base 1 - 1 - 0 = 0 is explicitly
// permitted by the spec.
@@ -465,8 +489,36 @@
// Add literal entry with name "foo" and value "bar".
DecodeEncoderStreamData(QuicTextUtils::HexDecode("6294e703626172"));
- EXPECT_CALL(handler_,
- OnDecodingErrorDetected(Eq("Dynamic table entry not found.")));
+ EXPECT_CALL(handler_, OnDecodingErrorDetected(Eq("Invalid relative index.")));
+
+ DecodeHeaderBlock(QuicTextUtils::HexDecode(
+ "0200" // Required Insert Count 1 and Delta Base 0.
+ // Base is 1 + 0 = 1.
+ "81")); // Indexed Header Field instruction addressing relative index 1.
+ // This is absolute index -1, which is invalid.
+
+ EXPECT_CALL(handler_, OnDecodingErrorDetected(Eq("Invalid relative index.")));
+
+ DecodeHeaderBlock(QuicTextUtils::HexDecode(
+ "0200" // Required Insert Count 1 and Delta Base 0.
+ // Base is 1 + 0 = 1.
+ "4100")); // Literal Header Field with Name Reference instruction
+ // addressing relative index 1. This is absolute index -1,
+ // which is invalid.
+}
+
+TEST_P(QpackDecoderTest, EvictedDynamicTableEntry) {
+ // Update dynamic table capacity to 128.
+ DecodeEncoderStreamData(QuicTextUtils::HexDecode("3f61"));
+
+ // Add literal entry with name "foo" and value "bar", size 32 + 3 + 3 = 38.
+ // This fits in the table three times.
+ DecodeEncoderStreamData(QuicTextUtils::HexDecode("6294e703626172"));
+ // Duplicate entry four times. This evicts the first two instances.
+ DecodeEncoderStreamData(QuicTextUtils::HexDecode("00000000"));
+
+ EXPECT_CALL(handler_, OnDecodingErrorDetected(
+ Eq("Dynamic table entry already evicted.")));
DecodeHeaderBlock(QuicTextUtils::HexDecode(
"0500" // Required Insert Count 4 and Delta Base 0.
@@ -474,16 +526,8 @@
"82")); // Indexed Header Field instruction addressing relative index 2.
// This is absolute index 1. Such entry does not exist.
- EXPECT_CALL(handler_, OnDecodingErrorDetected(Eq("Invalid relative index.")));
-
- DecodeHeaderBlock(QuicTextUtils::HexDecode(
- "0500" // Required Insert Count 4 and Delta Base 0.
- // Base is 4 + 0 = 4.
- "84")); // Indexed Header Field instruction addressing relative index 4.
- // This is absolute index -1, which is invalid.
-
- EXPECT_CALL(handler_,
- OnDecodingErrorDetected(Eq("Dynamic table entry not found.")));
+ EXPECT_CALL(handler_, OnDecodingErrorDetected(
+ Eq("Dynamic table entry already evicted.")));
DecodeHeaderBlock(QuicTextUtils::HexDecode(
"0500" // Required Insert Count 4 and Delta Base 0.
@@ -492,22 +536,8 @@
// addressing relative index 2. This is absolute index 1. Such
// entry does not exist.
- EXPECT_CALL(handler_, OnDecodingErrorDetected(Eq("Invalid relative index.")));
-
- DecodeHeaderBlock(QuicTextUtils::HexDecode(
- "0500" // Required Insert Count 4 and Delta Base 0.
- // Base is 4 + 0 = 4.
- "4400")); // Literal Header Field with Name Reference instruction
- // addressing relative index 4. This is absolute index -1,
- // which is invalid.
-}
-
-TEST_P(QpackDecoderTest, InvalidDynamicEntryByPostBaseIndex) {
- // Add literal entry with name "foo" and value "bar".
- DecodeEncoderStreamData(QuicTextUtils::HexDecode("6294e703626172"));
-
- EXPECT_CALL(handler_,
- OnDecodingErrorDetected(Eq("Dynamic table entry not found.")));
+ EXPECT_CALL(handler_, OnDecodingErrorDetected(
+ Eq("Dynamic table entry already evicted.")));
DecodeHeaderBlock(QuicTextUtils::HexDecode(
"0380" // Required Insert Count 2 and Delta Base 0 with sign bit set.
@@ -516,8 +546,8 @@
// entry with post-base index 0, absolute index 1. Such entry
// does not exist.
- EXPECT_CALL(handler_,
- OnDecodingErrorDetected(Eq("Dynamic table entry not found.")));
+ EXPECT_CALL(handler_, OnDecodingErrorDetected(
+ Eq("Dynamic table entry already evicted.")));
DecodeHeaderBlock(QuicTextUtils::HexDecode(
"0380" // Required Insert Count 2 and Delta Base 0 with sign bit set.
@@ -590,6 +620,9 @@
}
TEST_P(QpackDecoderTest, NonZeroRequiredInsertCountButNoDynamicEntries) {
+ // Add literal entry with name "foo" and value "bar".
+ DecodeEncoderStreamData(QuicTextUtils::HexDecode("6294e703626172"));
+
EXPECT_CALL(handler_, OnHeaderDecoded(Eq(":method"), Eq("GET")));
EXPECT_CALL(handler_,
OnDecodingErrorDetected(Eq("Required Insert Count too large.")));
@@ -600,6 +633,9 @@
}
TEST_P(QpackDecoderTest, AddressEntryNotAllowedByRequiredInsertCount) {
+ // Add literal entry with name "foo" and value "bar".
+ DecodeEncoderStreamData(QuicTextUtils::HexDecode("6294e703626172"));
+
EXPECT_CALL(
handler_,
OnDecodingErrorDetected(
@@ -655,7 +691,9 @@
TEST_P(QpackDecoderTest, PromisedRequiredInsertCountLargerThanActual) {
// Add literal entry with name "foo" and value "bar".
DecodeEncoderStreamData(QuicTextUtils::HexDecode("6294e703626172"));
- // Duplicate entry.
+ // Duplicate entry twice so that decoding of header blocks with Required
+ // Insert Count not exceeding 3 is not blocked.
+ DecodeEncoderStreamData(QuicTextUtils::HexDecode("00"));
DecodeEncoderStreamData(QuicTextUtils::HexDecode("00"));
EXPECT_CALL(handler_, OnHeaderDecoded(Eq("foo"), Eq("bar")));
@@ -707,6 +745,86 @@
// count of 2, even though Required Insert Count is 3.
}
+TEST_P(QpackDecoderTest, BlockedDecoding) {
+ DecodeHeaderBlock(QuicTextUtils::HexDecode(
+ "0200" // Required Insert Count 1 and Delta Base 0.
+ // Base is 1 + 0 = 1.
+ "80")); // Indexed Header Field instruction addressing dynamic table
+ // entry with relative index 0, absolute index 0.
+
+ EXPECT_CALL(handler_, OnHeaderDecoded(Eq("foo"), Eq("bar")));
+ EXPECT_CALL(handler_, OnDecodingCompleted());
+ EXPECT_CALL(decoder_stream_sender_delegate_,
+ WriteStreamData(Eq(kHeaderAcknowledgement)));
+
+ // Add literal entry with name "foo" and value "bar".
+ DecodeEncoderStreamData(QuicTextUtils::HexDecode("6294e703626172"));
+}
+
+TEST_P(QpackDecoderTest, BlockedDecodingUnblockedBeforeEndOfHeaderBlock) {
+ StartDecoding();
+ DecodeData(QuicTextUtils::HexDecode(
+ "0200" // Required Insert Count 1 and Delta Base 0.
+ // Base is 1 + 0 = 1.
+ "80" // Indexed Header Field instruction addressing dynamic table
+ // entry with relative index 0, absolute index 0.
+ "d1")); // Static table entry with index 17.
+
+ // Add literal entry with name "foo" and value "bar". Decoding is now
+ // unblocked because dynamic table Insert Count reached the Required Insert
+ // Count of the header block. |handler_| methods are called immediately for
+ // the already consumed part of the header block.
+ EXPECT_CALL(handler_, OnHeaderDecoded(Eq("foo"), Eq("bar")));
+ EXPECT_CALL(handler_, OnHeaderDecoded(Eq(":method"), Eq("GET")));
+ DecodeEncoderStreamData(QuicTextUtils::HexDecode("6294e703626172"));
+ Mock::VerifyAndClearExpectations(&handler_);
+
+ // Rest of header block is processed by QpackProgressiveDecoder
+ // in the unblocked state.
+ EXPECT_CALL(handler_, OnHeaderDecoded(Eq("foo"), Eq("bar")));
+ EXPECT_CALL(handler_, OnHeaderDecoded(Eq(":scheme"), Eq("https")));
+ DecodeData(QuicTextUtils::HexDecode(
+ "80" // Indexed Header Field instruction addressing dynamic table
+ // entry with relative index 0, absolute index 0.
+ "d7")); // Static table entry with index 23.
+ Mock::VerifyAndClearExpectations(&handler_);
+
+ EXPECT_CALL(handler_, OnDecodingCompleted());
+ EXPECT_CALL(decoder_stream_sender_delegate_,
+ WriteStreamData(Eq(kHeaderAcknowledgement)));
+ EndDecoding();
+}
+
+// Make sure that Required Insert Count is compared to Insert Count,
+// not size of dynamic table.
+TEST_P(QpackDecoderTest, BlockedDecodingAndEvictedEntries) {
+ // Update dynamic table capacity to 128.
+ // At most three non-empty entries fit in the dynamic table.
+ DecodeEncoderStreamData(QuicTextUtils::HexDecode("3f61"));
+
+ StartDecoding();
+ DecodeHeaderBlock(QuicTextUtils::HexDecode(
+ "0700" // Required Insert Count 6 and Delta Base 0.
+ // Base is 6 + 0 = 6.
+ "80")); // Indexed Header Field instruction addressing dynamic table
+ // entry with relative index 0, absolute index 5.
+
+ // Add literal entry with name "foo" and value "bar".
+ DecodeEncoderStreamData(QuicTextUtils::HexDecode("6294e703626172"));
+
+ // Duplicate entry four times. This evicts the first two instances.
+ DecodeEncoderStreamData(QuicTextUtils::HexDecode("00000000"));
+
+ EXPECT_CALL(handler_, OnHeaderDecoded(Eq("foo"), Eq("baz")));
+ EXPECT_CALL(handler_, OnDecodingCompleted());
+ EXPECT_CALL(decoder_stream_sender_delegate_,
+ WriteStreamData(Eq(kHeaderAcknowledgement)));
+
+ // Add literal entry with name "foo" and value "bar".
+ // Insert Count is now 6, reaching Required Insert Count of the header block.
+ DecodeEncoderStreamData(QuicTextUtils::HexDecode("6294e70362617a"));
+}
+
} // namespace
} // namespace test
} // namespace quic