Do not call MarkConsumed() from OnHeadersFramePayload() after decoding error. gfe-relnote: n/a, change to QUIC v99-only code. Protected by existing disabled gfe2_reloadable_flag_quic_enable_version_99. PiperOrigin-RevId: 282593548 Change-Id: I7d43611cb21d2059b5d4a0ec0d09078a3c084169
diff --git a/quic/core/http/quic_spdy_stream.cc b/quic/core/http/quic_spdy_stream.cc index fe990fc..b0c4343 100644 --- a/quic/core/http/quic_spdy_stream.cc +++ b/quic/core/http/quic_spdy_stream.cc
@@ -919,10 +919,14 @@ } qpack_decoded_headers_accumulator_->Decode(payload); - sequencer()->MarkConsumed(body_manager_.OnNonBody(payload.size())); // |qpack_decoded_headers_accumulator_| is reset if an error is detected. - return qpack_decoded_headers_accumulator_ != nullptr; + if (!qpack_decoded_headers_accumulator_) { + return false; + } + + sequencer()->MarkConsumed(body_manager_.OnNonBody(payload.size())); + return true; } bool QuicSpdyStream::OnHeadersFrameEnd() {
diff --git a/quic/core/http/quic_spdy_stream_test.cc b/quic/core/http/quic_spdy_stream_test.cc index 61e6f37..d7c6b93 100644 --- a/quic/core/http/quic_spdy_stream_test.cc +++ b/quic/core/http/quic_spdy_stream_test.cc
@@ -1862,6 +1862,48 @@ stream_->OnStreamFrame(frame); } +// Regression test for https://crbug.com/1027895: a HEADERS frame triggers an +// error in QuicSpdyStream::OnHeadersFramePayload(). This closes the +// connection, freeing the buffer of QuicStreamSequencer. Therefore +// QuicStreamSequencer::MarkConsumed() must not be called from +// QuicSpdyStream::OnHeadersFramePayload(). +TEST_P(QuicSpdyStreamTest, DoNotMarkConsumedAfterQpackDecodingError) { + if (!UsesHttp3()) { + return; + } + + Initialize(kShouldProcessData); + connection_->AdvanceTime(QuicTime::Delta::FromSeconds(1)); + + testing::InSequence s; + EXPECT_CALL( + *connection_, + CloseConnection(QUIC_QPACK_DECOMPRESSION_FAILED, + MatchesRegex("Error decoding headers on stream \\d+: " + "Invalid relative index."), + _)) + .WillOnce( + (Invoke([this](QuicErrorCode error, const std::string& error_details, + ConnectionCloseBehavior connection_close_behavior) { + connection_->ReallyCloseConnection(error, error_details, + connection_close_behavior); + }))); + EXPECT_CALL(*connection_, SendConnectionClosePacket(_, _)); + EXPECT_CALL(*session_, OnConnectionClosed(_, _)) + .WillOnce(Invoke([this](const QuicConnectionCloseFrame& frame, + ConnectionCloseSource source) { + session_->ReallyOnConnectionClosed(frame, source); + })); + EXPECT_CALL(*session_, SendRstStream(stream_->id(), _, _)); + EXPECT_CALL(*session_, SendRstStream(stream2_->id(), _, _)); + + // Invalid headers: Required Insert Count is zero, but the header block + // contains a dynamic table reference. + std::string headers = HeadersFrame(QuicTextUtils::HexDecode("000080")); + QuicStreamFrame frame(stream_->id(), false, 0, headers); + stream_->OnStreamFrame(frame); +} + TEST_P(QuicSpdyStreamTest, ImmediateHeaderDecodingWithDynamicTableEntries) { if (!UsesHttp3()) { return;