Unhide security fix to QuicSpdyStream (cl/626052950). Also set the external value of --gfe2_reloadable_flag_quic_stop_reading_also_stops_header_decompression to true. To Envoy QUICHE importer: Remove https://github.com/envoyproxy/envoy/blob/main/bazel/external/quiche_stream_fix.patch once this gets imported. PiperOrigin-RevId: 652494002
diff --git a/quiche/common/quiche_feature_flags_list.h b/quiche/common/quiche_feature_flags_list.h index fe07be7..a00bbe5 100755 --- a/quiche/common/quiche_feature_flags_list.h +++ b/quiche/common/quiche_feature_flags_list.h
@@ -45,7 +45,7 @@ QUICHE_FLAG(bool, quiche_reloadable_flag_quic_priority_respect_incremental, false, false, "If true, respect the incremental parameter of each stream in QuicWriteBlockedList.") QUICHE_FLAG(bool, quiche_reloadable_flag_quic_require_handshake_confirmation, true, true, "If true, require handshake confirmation for QUIC connections, functionally disabling 0-rtt handshakes.") QUICHE_FLAG(bool, quiche_reloadable_flag_quic_send_placeholder_ticket_when_encrypt_ticket_fails, false, true, "If true, when TicketCrypter fails to encrypt a session ticket, quic::TlsServerHandshaker will send a placeholder ticket, instead of an empty one, to the client.") -QUICHE_FLAG(bool, quiche_reloadable_flag_quic_stop_reading_also_stops_header_decompression, false, false, "") +QUICHE_FLAG(bool, quiche_reloadable_flag_quic_stop_reading_also_stops_header_decompression, false, true, "If true, QUIC stream will not continue decompressing buffer headers after StopReading() called.") QUICHE_FLAG(bool, quiche_reloadable_flag_quic_test_peer_addr_change_after_normalize, false, false, "If true, QuicConnection::ProcessValidatedPacket will use normalized address to test peer address changes.") QUICHE_FLAG(bool, quiche_reloadable_flag_quic_testonly_default_false, false, false, "A testonly reloadable flag that will always default to false.") QUICHE_FLAG(bool, quiche_reloadable_flag_quic_testonly_default_true, true, true, "A testonly reloadable flag that will always default to true.")
diff --git a/quiche/quic/core/http/quic_spdy_stream.cc b/quiche/quic/core/http/quic_spdy_stream.cc index 7637c4c..71eced9 100644 --- a/quiche/quic/core/http/quic_spdy_stream.cc +++ b/quiche/quic/core/http/quic_spdy_stream.cc
@@ -1825,6 +1825,20 @@ return true; } +void QuicSpdyStream::StopReading() { + QuicStream::StopReading(); + if (GetQuicReloadableFlag( + quic_stop_reading_also_stops_header_decompression) && + VersionUsesHttp3(transport_version()) && !fin_received() && + spdy_session_->qpack_decoder()) { + QUIC_RELOADABLE_FLAG_COUNT( + quic_stop_reading_also_stops_header_decompression); + // Clean up Qpack decoding states. + spdy_session_->qpack_decoder()->OnStreamReset(id()); + qpack_decoded_headers_accumulator_.reset(); + } +} + void QuicSpdyStream::OnInvalidHeaders() { Reset(QUIC_BAD_APPLICATION_PAYLOAD); } void QuicSpdyStream::CloseReadSide() {
diff --git a/quiche/quic/core/http/quic_spdy_stream.h b/quiche/quic/core/http/quic_spdy_stream.h index 4de6fe2..b74cf99 100644 --- a/quiche/quic/core/http/quic_spdy_stream.h +++ b/quiche/quic/core/http/quic_spdy_stream.h
@@ -119,6 +119,7 @@ // QuicStream implementation void OnClose() override; + void StopReading() override; // Override to maybe close the write side after writing. void OnCanWrite() override;
diff --git a/quiche/quic/core/http/quic_spdy_stream_test.cc b/quiche/quic/core/http/quic_spdy_stream_test.cc index 95df725..9ea9a77 100644 --- a/quiche/quic/core/http/quic_spdy_stream_test.cc +++ b/quiche/quic/core/http/quic_spdy_stream_test.cc
@@ -2409,6 +2409,68 @@ stream_->MarkTrailersConsumed(); } +TEST_P(QuicSpdyStreamTest, BlockedHeaderDecodingAndStopReading) { + if (!UsesHttp3()) { + return; + } + Initialize(kShouldProcessData); + testing::InSequence s; + session_->qpack_decoder()->OnSetDynamicTableCapacity(1024); + StrictMock<MockHttp3DebugVisitor> debug_visitor; + session_->set_debug_visitor(&debug_visitor); + + // HEADERS frame referencing first dynamic table entry. + std::string encoded_headers; + ASSERT_TRUE(absl::HexStringToBytes("020080", &encoded_headers)); + std::string headers = HeadersFrame(encoded_headers); + EXPECT_CALL(debug_visitor, + OnHeadersFrameReceived(stream_->id(), encoded_headers.length())); + stream_->OnStreamFrame(QuicStreamFrame(stream_->id(), false, 0, headers)); + + // Decoding is blocked because dynamic table entry has not been received yet. + EXPECT_FALSE(stream_->headers_decompressed()); + + auto decoder_send_stream = + QuicSpdySessionPeer::GetQpackDecoderSendStream(session_.get()); + if (GetQuicReloadableFlag( + quic_stop_reading_also_stops_header_decompression)) { + if (!GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data5)) { + // Stream type. + EXPECT_CALL(*session_, + WritevData(decoder_send_stream->id(), /* write_length = */ 1, + /* offset = */ 0, _, _, _)); + // Send stream cancellation. + EXPECT_CALL(*session_, + WritevData(decoder_send_stream->id(), /* write_length = */ 1, + /* offset = */ 1, _, _, _)); + } + EXPECT_CALL(debug_visitor, OnHeadersDecoded(stream_->id(), _)).Times(0); + } + // Stop reading from now on. Any buffered compressed headers shouldn't be + // decompressed and delivered up. + stream_->StopReading(); + + if (!GetQuicReloadableFlag( + quic_stop_reading_also_stops_header_decompression)) { + if (!GetQuicRestartFlag(quic_opport_bundle_qpack_decoder_data5)) { + // Stream type. + EXPECT_CALL(*session_, + WritevData(decoder_send_stream->id(), /* write_length = */ 1, + /* offset = */ 0, _, _, _)); + // Header acknowledgement. + EXPECT_CALL(*session_, + WritevData(decoder_send_stream->id(), /* write_length = */ 1, + /* offset = */ 1, _, _, _)); + } + EXPECT_CALL(debug_visitor, OnHeadersDecoded(stream_->id(), _)); + } + // Deliver dynamic table entry to decoder. + session_->qpack_decoder()->OnInsertWithoutNameReference("foo", "bar"); + EXPECT_NE( + GetQuicReloadableFlag(quic_stop_reading_also_stops_header_decompression), + stream_->headers_decompressed()); +} + TEST_P(QuicSpdyStreamTest, AsyncErrorDecodingHeaders) { if (!UsesHttp3()) { return;