Only send Header Acknowledgement from QpackDecoder if required insert count is not zero.
As prescribed by
https://quicwg.org/base-drafts/draft-ietf-quic-qpack.html#header-acknowledgement.
I have a draft dynamic table encoder CL and a draft round trip fuzzer CL
relaying encoder stream data and decoder stream data between the encoder and
decoder, and I noticed this bug when patching these two CLs together.
gfe-relnote: n/a, change to QUIC v99-only code. Protected by existing disabled gfe2_reloadable_flag_quic_enable_version_99.
PiperOrigin-RevId: 263253859
Change-Id: I766e4746d7ce88c6214f46d9cb5e35877b1f93e8
diff --git a/quic/core/qpack/qpack_decoded_headers_accumulator_test.cc b/quic/core/qpack/qpack_decoded_headers_accumulator_test.cc
index 3f7ae25..2a0a222 100644
--- a/quic/core/qpack/qpack_decoded_headers_accumulator_test.cc
+++ b/quic/core/qpack/qpack_decoded_headers_accumulator_test.cc
@@ -81,9 +81,6 @@
}
TEST_F(QpackDecodedHeadersAccumulatorTest, EmptyHeaderList) {
- EXPECT_CALL(decoder_stream_sender_delegate_,
- WriteStreamData(Eq(kHeaderAcknowledgement)));
-
EXPECT_TRUE(accumulator_.Decode(QuicTextUtils::HexDecode("0000")));
EXPECT_EQ(Status::kSuccess, accumulator_.EndHeaderBlock());
@@ -105,9 +102,6 @@
}
TEST_F(QpackDecodedHeadersAccumulatorTest, Success) {
- EXPECT_CALL(decoder_stream_sender_delegate_,
- WriteStreamData(Eq(kHeaderAcknowledgement)));
-
std::string encoded_data(QuicTextUtils::HexDecode("000023666f6f03626172"));
EXPECT_TRUE(accumulator_.Decode(encoded_data));
EXPECT_EQ(Status::kSuccess, accumulator_.EndHeaderBlock());
@@ -121,9 +115,6 @@
}
TEST_F(QpackDecodedHeadersAccumulatorTest, ExceedingLimit) {
- EXPECT_CALL(decoder_stream_sender_delegate_,
- WriteStreamData(Eq(kHeaderAcknowledgement)));
-
// Total length of header list exceeds kMaxHeaderListSize.
EXPECT_TRUE(accumulator_.Decode(QuicTextUtils::HexDecode(
"0000" // header block prefix
diff --git a/quic/core/qpack/qpack_decoder_test.cc b/quic/core/qpack/qpack_decoder_test.cc
index 5190975..506d60c 100644
--- a/quic/core/qpack/qpack_decoder_test.cc
+++ b/quic/core/qpack/qpack_decoder_test.cc
@@ -104,8 +104,6 @@
TEST_P(QpackDecoderTest, EmptyHeaderBlock) {
EXPECT_CALL(handler_, OnDecodingCompleted());
- EXPECT_CALL(decoder_stream_sender_delegate_,
- WriteStreamData(Eq(kHeaderAcknowledgement)));
DecodeHeaderBlock(QuicTextUtils::HexDecode("0000"));
}
@@ -113,8 +111,6 @@
TEST_P(QpackDecoderTest, LiteralEntryEmptyName) {
EXPECT_CALL(handler_, OnHeaderDecoded(Eq(""), Eq("foo")));
EXPECT_CALL(handler_, OnDecodingCompleted());
- EXPECT_CALL(decoder_stream_sender_delegate_,
- WriteStreamData(Eq(kHeaderAcknowledgement)));
DecodeHeaderBlock(QuicTextUtils::HexDecode("00002003666f6f"));
}
@@ -122,8 +118,6 @@
TEST_P(QpackDecoderTest, LiteralEntryEmptyValue) {
EXPECT_CALL(handler_, OnHeaderDecoded(Eq("foo"), Eq("")));
EXPECT_CALL(handler_, OnDecodingCompleted());
- EXPECT_CALL(decoder_stream_sender_delegate_,
- WriteStreamData(Eq(kHeaderAcknowledgement)));
DecodeHeaderBlock(QuicTextUtils::HexDecode("000023666f6f00"));
}
@@ -131,8 +125,6 @@
TEST_P(QpackDecoderTest, LiteralEntryEmptyNameAndValue) {
EXPECT_CALL(handler_, OnHeaderDecoded(Eq(""), Eq("")));
EXPECT_CALL(handler_, OnDecodingCompleted());
- EXPECT_CALL(decoder_stream_sender_delegate_,
- WriteStreamData(Eq(kHeaderAcknowledgement)));
DecodeHeaderBlock(QuicTextUtils::HexDecode("00002000"));
}
@@ -140,8 +132,6 @@
TEST_P(QpackDecoderTest, SimpleLiteralEntry) {
EXPECT_CALL(handler_, OnHeaderDecoded(Eq("foo"), Eq("bar")));
EXPECT_CALL(handler_, OnDecodingCompleted());
- EXPECT_CALL(decoder_stream_sender_delegate_,
- WriteStreamData(Eq(kHeaderAcknowledgement)));
DecodeHeaderBlock(QuicTextUtils::HexDecode("000023666f6f03626172"));
}
@@ -151,8 +141,6 @@
std::string str(127, 'a');
EXPECT_CALL(handler_, OnHeaderDecoded(Eq("foobaar"), QuicStringPiece(str)));
EXPECT_CALL(handler_, OnDecodingCompleted());
- EXPECT_CALL(decoder_stream_sender_delegate_,
- WriteStreamData(Eq(kHeaderAcknowledgement)));
DecodeHeaderBlock(QuicTextUtils::HexDecode(
"0000" // prefix
@@ -210,8 +198,6 @@
TEST_P(QpackDecoderTest, HuffmanSimple) {
EXPECT_CALL(handler_, OnHeaderDecoded(Eq("custom-key"), Eq("custom-value")));
EXPECT_CALL(handler_, OnDecodingCompleted());
- EXPECT_CALL(decoder_stream_sender_delegate_,
- WriteStreamData(Eq(kHeaderAcknowledgement)));
DecodeHeaderBlock(
QuicTextUtils::HexDecode("00002f0125a849e95ba97d7f8925a849e95bb8e8b4bf"));
@@ -221,8 +207,6 @@
EXPECT_CALL(handler_, OnHeaderDecoded(Eq("custom-key"), Eq("custom-value")))
.Times(4);
EXPECT_CALL(handler_, OnDecodingCompleted());
- EXPECT_CALL(decoder_stream_sender_delegate_,
- WriteStreamData(Eq(kHeaderAcknowledgement)));
DecodeHeaderBlock(QuicTextUtils::HexDecode(
"0000" // Prefix.
@@ -295,8 +279,6 @@
EXPECT_CALL(handler_, OnHeaderDecoded(Eq("location"), Eq("foo")));
EXPECT_CALL(handler_, OnDecodingCompleted());
- EXPECT_CALL(decoder_stream_sender_delegate_,
- WriteStreamData(Eq(kHeaderAcknowledgement)));
DecodeHeaderBlock(QuicTextUtils::HexDecode(
"0000d1dfccd45f108621e9aec2a11f5c8294e75f000554524143455f1000"));
diff --git a/quic/core/qpack/qpack_progressive_decoder.cc b/quic/core/qpack/qpack_progressive_decoder.cc
index e364edc..b3c374e 100644
--- a/quic/core/qpack/qpack_progressive_decoder.cc
+++ b/quic/core/qpack/qpack_progressive_decoder.cc
@@ -316,7 +316,10 @@
return;
}
- decoder_stream_sender_->SendHeaderAcknowledgement(stream_id_);
+ if (required_insert_count_ > 0) {
+ decoder_stream_sender_->SendHeaderAcknowledgement(stream_id_);
+ }
+
handler_->OnDecodingCompleted();
}
