Close read side but not write side in QuicSpdyStream::OnStreamReset().
This bug was found by ClusterFuzz at https://crbug.com/1177662.
Protected by FLAGS_quic_reloadable_flag_quic_fix_on_stream_reset.
PiperOrigin-RevId: 357252071
Change-Id: Ie672d4d877fa44c2fd11530d6e73a600a7cfd11e
diff --git a/quic/core/http/quic_spdy_stream.cc b/quic/core/http/quic_spdy_stream.cc
index aa5f7bf..83905b5 100644
--- a/quic/core/http/quic_spdy_stream.cc
+++ b/quic/core/http/quic_spdy_stream.cc
@@ -725,6 +725,9 @@
}
void QuicSpdyStream::OnStreamReset(const QuicRstStreamFrame& frame) {
+ // TODO(bnc): Merge the two blocks below when both
+ // quic_abort_qpack_on_stream_reset and quic_fix_on_stream_reset are
+ // deprecated.
if (frame.error_code != QUIC_STREAM_NO_ERROR) {
if (VersionUsesHttp3(transport_version()) && !fin_received() &&
spdy_session_->qpack_decoder()) {
@@ -739,6 +742,18 @@
return;
}
+ if (GetQuicReloadableFlag(quic_fix_on_stream_reset) &&
+ VersionUsesHttp3(transport_version())) {
+ QUIC_RELOADABLE_FLAG_COUNT(quic_fix_on_stream_reset);
+ if (!fin_received() && spdy_session_->qpack_decoder()) {
+ spdy_session_->qpack_decoder()->OnStreamReset(id());
+ qpack_decoded_headers_accumulator_.reset();
+ }
+
+ QuicStream::OnStreamReset(frame);
+ return;
+ }
+
QUIC_DVLOG(1) << ENDPOINT
<< "Received QUIC_STREAM_NO_ERROR, not discarding response";
set_rst_received(true);
diff --git a/quic/core/http/quic_spdy_stream_test.cc b/quic/core/http/quic_spdy_stream_test.cc
index 06f779e..0e35c28 100644
--- a/quic/core/http/quic_spdy_stream_test.cc
+++ b/quic/core/http/quic_spdy_stream_test.cc
@@ -1142,10 +1142,19 @@
Initialize(kShouldProcessData);
ProcessHeaders(false, headers_);
+ EXPECT_CALL(*session_, WritevData(_, _, _, _, _, _)).Times(AnyNumber());
+
stream_->OnStreamReset(QuicRstStreamFrame(
kInvalidControlFrameId, stream_->id(), QUIC_STREAM_NO_ERROR, 0));
- EXPECT_TRUE(stream_->write_side_closed());
- EXPECT_FALSE(stream_->reading_stopped());
+
+ if (GetQuicReloadableFlag(quic_fix_on_stream_reset) && UsesHttp3()) {
+ // RESET_STREAM should close the read side but not the write side.
+ EXPECT_TRUE(stream_->read_side_closed());
+ EXPECT_FALSE(stream_->write_side_closed());
+ } else {
+ EXPECT_TRUE(stream_->write_side_closed());
+ EXPECT_FALSE(stream_->reading_stopped());
+ }
}
// Tests that on if the peer sends too much data (i.e. violates the flow control
@@ -3029,6 +3038,39 @@
EXPECT_EQ(headers_frame_payload_length, write_headers_return_value);
}
+// Regression test for https://crbug.com/1177662.
+// RESET_STREAM with QUIC_STREAM_NO_ERROR should not be treated in a special
+// way: it should close the read side but not the write side.
+TEST_P(QuicSpdyStreamTest, TwoResetStreamFrames) {
+ if (!UsesHttp3()) {
+ return;
+ }
+
+ Initialize(kShouldProcessData);
+
+ EXPECT_CALL(*session_, WritevData(_, _, _, _, _, _)).Times(AnyNumber());
+
+ QuicRstStreamFrame rst_frame1(kInvalidControlFrameId, stream_->id(),
+ QUIC_STREAM_CANCELLED, /* bytes_written = */ 0);
+ stream_->OnStreamReset(rst_frame1);
+ EXPECT_TRUE(stream_->read_side_closed());
+ EXPECT_FALSE(stream_->write_side_closed());
+
+ QuicRstStreamFrame rst_frame2(kInvalidControlFrameId, stream_->id(),
+ QUIC_STREAM_NO_ERROR, /* bytes_written = */ 0);
+ if (GetQuicReloadableFlag(quic_fix_on_stream_reset)) {
+ stream_->OnStreamReset(rst_frame2);
+ EXPECT_TRUE(stream_->read_side_closed());
+ EXPECT_FALSE(stream_->write_side_closed());
+ } else {
+ EXPECT_CALL(*session_, MaybeSendRstStreamFrame(
+ stream_->id(), QUIC_RST_ACKNOWLEDGEMENT, _));
+ EXPECT_QUIC_BUG(
+ stream_->OnStreamReset(rst_frame2),
+ "The stream should've already sent RST in response to STOP_SENDING");
+ }
+}
+
} // namespace
} // namespace test
} // namespace quic
diff --git a/quic/core/quic_flags_list.h b/quic/core/quic_flags_list.h
index 6289379..6f809a7 100644
--- a/quic/core/quic_flags_list.h
+++ b/quic/core/quic_flags_list.h
@@ -44,6 +44,7 @@
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_enable_version_rfcv1, false)
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_encrypted_control_frames, false)
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_encrypted_goaway, true)
+QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_fix_on_stream_reset, true)
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_fix_willing_and_able_to_write2, true)
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_goaway_with_max_stream_id, true)
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_parse_accept_ch_frame, true)
