Add header_list_size_limit_exceeded argument to OnHeadersDecoded().
The motivation is to allow tests to call OnHeadersDecoded() directly without
QuicSpdyStream having a QpackDecodedHeadersAccumulator object. This happens in
Envoy at
https://github.com/envoyproxy/envoy/blob/master/test/extensions/quic_listeners/quiche/envoy_quic_server_stream_test.cc#L110,
which crashes since cr/282822239 added a reference to
|qpack_decoded_headers_accumulator_| in QuicSpdyStream::OnHeadersDecoded().
gfe-relnote: n/a, change to QUIC v99-only code. Protected by existing disabled gfe2_reloadable_flag_quic_enable_version_99.
PiperOrigin-RevId: 284213476
Change-Id: I71737f85d8a00db900565dd16c69510844ef3999
diff --git a/quic/core/http/quic_spdy_stream.cc b/quic/core/http/quic_spdy_stream.cc
index f7a13f6..78d8593 100644
--- a/quic/core/http/quic_spdy_stream.cc
+++ b/quic/core/http/quic_spdy_stream.cc
@@ -538,9 +538,9 @@
}
}
-void QuicSpdyStream::OnHeadersDecoded(QuicHeaderList headers) {
- header_list_size_limit_exceeded_ =
- qpack_decoded_headers_accumulator_->header_list_size_limit_exceeded();
+void QuicSpdyStream::OnHeadersDecoded(QuicHeaderList headers,
+ bool header_list_size_limit_exceeded) {
+ header_list_size_limit_exceeded_ = header_list_size_limit_exceeded;
qpack_decoded_headers_accumulator_.reset();
QuicSpdySession::LogHeaderCompressionRatioHistogram(
diff --git a/quic/core/http/quic_spdy_stream.h b/quic/core/http/quic_spdy_stream.h
index 8610dbb..c4333f3 100644
--- a/quic/core/http/quic_spdy_stream.h
+++ b/quic/core/http/quic_spdy_stream.h
@@ -214,7 +214,8 @@
bool IsClosed() { return sequencer()->IsClosed(); }
// QpackDecodedHeadersAccumulator::Visitor implementation.
- void OnHeadersDecoded(QuicHeaderList headers) override;
+ void OnHeadersDecoded(QuicHeaderList headers,
+ bool header_list_size_limit_exceeded) override;
void OnHeaderDecodingError(QuicStringPiece error_message) override;
protected:
diff --git a/quic/core/qpack/fuzzer/qpack_round_trip_fuzzer.cc b/quic/core/qpack/fuzzer/qpack_round_trip_fuzzer.cc
index 43a637d..018bf09 100644
--- a/quic/core/qpack/fuzzer/qpack_round_trip_fuzzer.cc
+++ b/quic/core/qpack/fuzzer/qpack_round_trip_fuzzer.cc
@@ -264,8 +264,10 @@
virtual ~VerifyingDecoder() = default;
// QpackDecodedHeadersAccumulator::Visitor implementation.
- void OnHeadersDecoded(QuicHeaderList headers) override {
+ void OnHeadersDecoded(QuicHeaderList headers,
+ bool header_list_size_limit_exceeded) override {
// Verify headers.
+ CHECK(!header_list_size_limit_exceeded);
CHECK(expected_header_list_ == headers);
// Might destroy |this|.
diff --git a/quic/core/qpack/qpack_decoded_headers_accumulator.cc b/quic/core/qpack/qpack_decoded_headers_accumulator.cc
index e16aa5a..b1f8012 100644
--- a/quic/core/qpack/qpack_decoded_headers_accumulator.cc
+++ b/quic/core/qpack/qpack_decoded_headers_accumulator.cc
@@ -62,7 +62,8 @@
uncompressed_header_bytes_without_overhead_, compressed_header_bytes_);
// Might destroy |this|.
- visitor_->OnHeadersDecoded(std::move(quic_header_list_));
+ visitor_->OnHeadersDecoded(std::move(quic_header_list_),
+ header_list_size_limit_exceeded_);
}
void QpackDecodedHeadersAccumulator::OnDecodingErrorDetected(
diff --git a/quic/core/qpack/qpack_decoded_headers_accumulator.h b/quic/core/qpack/qpack_decoded_headers_accumulator.h
index 0a2db6a..96b527e 100644
--- a/quic/core/qpack/qpack_decoded_headers_accumulator.h
+++ b/quic/core/qpack/qpack_decoded_headers_accumulator.h
@@ -34,13 +34,15 @@
public:
virtual ~Visitor() = default;
- // Called when headers are successfully decoded. If header list size
- // exceeds the limit specified via |max_header_list_size| in
- // QpackDecodedHeadersAccumulator constructor, then |headers| will be empty,
- // but will still have the correct compressed and uncompressed size
- // information. However, header_list_size_limit_exceeded() is recommended
- // instead of headers.empty() to check whether header size exceeds limit.
- virtual void OnHeadersDecoded(QuicHeaderList headers) = 0;
+ // Called when headers are successfully decoded. If the uncompressed header
+ // list size including an overhead for each header field exceeds the limit
+ // specified via |max_header_list_size| in QpackDecodedHeadersAccumulator
+ // constructor, then |header_list_size_limit_exceeded| will be true, and
+ // |headers| will be empty but will still have the correct compressed and
+ // uncompressed size
+ // information.
+ virtual void OnHeadersDecoded(QuicHeaderList headers,
+ bool header_list_size_limit_exceeded) = 0;
// Called when an error has occurred.
virtual void OnHeaderDecodingError(QuicStringPiece error_message) = 0;
@@ -68,13 +70,6 @@
// Must not be called more that once.
void EndHeaderBlock();
- // Returns true if the uncompressed size of the header list, including an
- // overhead for each header field, exceeds |max_header_list_size| passed in
- // the constructor.
- bool header_list_size_limit_exceeded() const {
- return header_list_size_limit_exceeded_;
- }
-
private:
std::unique_ptr<QpackProgressiveDecoder> decoder_;
Visitor* visitor_;
diff --git a/quic/core/qpack/qpack_decoded_headers_accumulator_test.cc b/quic/core/qpack/qpack_decoded_headers_accumulator_test.cc
index 6616517..2334e89 100644
--- a/quic/core/qpack/qpack_decoded_headers_accumulator_test.cc
+++ b/quic/core/qpack/qpack_decoded_headers_accumulator_test.cc
@@ -41,7 +41,9 @@
class MockVisitor : public QpackDecodedHeadersAccumulator::Visitor {
public:
~MockVisitor() override = default;
- MOCK_METHOD1(OnHeadersDecoded, void(QuicHeaderList headers));
+ MOCK_METHOD2(OnHeadersDecoded,
+ void(QuicHeaderList headers,
+ bool header_list_size_limit_exceeded));
MOCK_METHOD1(OnHeaderDecodingError, void(QuicStringPiece error_message));
};
@@ -89,9 +91,9 @@
accumulator_.Decode(encoded_data);
QuicHeaderList header_list;
- EXPECT_CALL(visitor_, OnHeadersDecoded(_)).WillOnce(SaveArg<0>(&header_list));
+ EXPECT_CALL(visitor_, OnHeadersDecoded(_, false))
+ .WillOnce(SaveArg<0>(&header_list));
accumulator_.EndHeaderBlock();
- EXPECT_FALSE(accumulator_.header_list_size_limit_exceeded());
EXPECT_EQ(0u, header_list.uncompressed_header_bytes());
EXPECT_EQ(encoded_data.size(), header_list.compressed_header_bytes());
@@ -119,9 +121,9 @@
accumulator_.Decode(encoded_data);
QuicHeaderList header_list;
- EXPECT_CALL(visitor_, OnHeadersDecoded(_)).WillOnce(SaveArg<0>(&header_list));
+ EXPECT_CALL(visitor_, OnHeadersDecoded(_, false))
+ .WillOnce(SaveArg<0>(&header_list));
accumulator_.EndHeaderBlock();
- EXPECT_FALSE(accumulator_.header_list_size_limit_exceeded());
EXPECT_THAT(header_list, ElementsAre(Pair("foo", "bar")));
EXPECT_EQ(strlen("foo") + strlen("bar"),
@@ -145,9 +147,8 @@
"0f" // second byte of a two-byte long Indexed Header Field instruction
));
- EXPECT_CALL(visitor_, OnHeadersDecoded(_));
+ EXPECT_CALL(visitor_, OnHeadersDecoded(_, true));
accumulator_.EndHeaderBlock();
- EXPECT_TRUE(accumulator_.header_list_size_limit_exceeded());
}
// Test that header list limit enforcement works with blocked encoding.
@@ -169,9 +170,8 @@
EXPECT_CALL(decoder_stream_sender_delegate_,
WriteStreamData(Eq(kHeaderAcknowledgement)));
- EXPECT_CALL(visitor_, OnHeadersDecoded(_));
+ EXPECT_CALL(visitor_, OnHeadersDecoded(_, true));
qpack_decoder_.OnInsertWithoutNameReference("foo", "bar");
- EXPECT_TRUE(accumulator_.header_list_size_limit_exceeded());
}
TEST_F(QpackDecodedHeadersAccumulatorTest, BlockedDecoding) {
@@ -187,10 +187,10 @@
WriteStreamData(Eq(kHeaderAcknowledgement)));
QuicHeaderList header_list;
- EXPECT_CALL(visitor_, OnHeadersDecoded(_)).WillOnce(SaveArg<0>(&header_list));
+ EXPECT_CALL(visitor_, OnHeadersDecoded(_, false))
+ .WillOnce(SaveArg<0>(&header_list));
qpack_decoder_.OnInsertWithoutNameReference("foo", "bar");
- EXPECT_FALSE(accumulator_.header_list_size_limit_exceeded());
EXPECT_THAT(header_list, ElementsAre(Pair("foo", "bar")));
EXPECT_EQ(strlen("foo") + strlen("bar"),
header_list.uncompressed_header_bytes());
@@ -213,9 +213,9 @@
accumulator_.Decode(QuicTextUtils::HexDecode("80"));
QuicHeaderList header_list;
- EXPECT_CALL(visitor_, OnHeadersDecoded(_)).WillOnce(SaveArg<0>(&header_list));
+ EXPECT_CALL(visitor_, OnHeadersDecoded(_, false))
+ .WillOnce(SaveArg<0>(&header_list));
accumulator_.EndHeaderBlock();
- EXPECT_FALSE(accumulator_.header_list_size_limit_exceeded());
EXPECT_THAT(header_list, ElementsAre(Pair("foo", "bar"), Pair("foo", "bar")));
}
