Reset stream if stream frame contains data that goes beyond stream's close offset.
This fixes http://crbug/1002119
gfe-relnote: protected by gfe2_reloadable_flag_quic_rst_if_stream_frame_beyond_close_offset.
PiperOrigin-RevId: 268732204
Change-Id: I66f845e320de89e8634b1bfb0725f599fc43dfd6
diff --git a/quic/core/quic_error_codes.cc b/quic/core/quic_error_codes.cc
index 1be1786..e69cd60 100644
--- a/quic/core/quic_error_codes.cc
+++ b/quic/core/quic_error_codes.cc
@@ -29,6 +29,7 @@
RETURN_STRING_LITERAL(QUIC_INVALID_PROMISE_METHOD);
RETURN_STRING_LITERAL(QUIC_PUSH_STREAM_TIMED_OUT);
RETURN_STRING_LITERAL(QUIC_HEADERS_TOO_LARGE);
+ RETURN_STRING_LITERAL(QUIC_DATA_AFTER_CLOSE_OFFSET);
RETURN_STRING_LITERAL(QUIC_STREAM_TTL_EXPIRED);
}
// Return a default value so that we return this when |error| doesn't match
diff --git a/quic/core/quic_error_codes.h b/quic/core/quic_error_codes.h
index e07094c..298029f 100644
--- a/quic/core/quic_error_codes.h
+++ b/quic/core/quic_error_codes.h
@@ -54,6 +54,8 @@
QUIC_HEADERS_TOO_LARGE,
// The data is not likely arrive in time.
QUIC_STREAM_TTL_EXPIRED,
+ // The stream received data that goes beyond its close offset.
+ QUIC_DATA_AFTER_CLOSE_OFFSET,
// No error. Used as bound while iterating.
QUIC_STREAM_LAST_ERROR,
};
diff --git a/quic/core/quic_session_test.cc b/quic/core/quic_session_test.cc
index a75d948..90c7e83 100644
--- a/quic/core/quic_session_test.cc
+++ b/quic/core/quic_session_test.cc
@@ -2717,6 +2717,25 @@
EXPECT_FALSE(session_.WillingAndAbleToWrite());
}
+// Regression test for
+// https://bugs.chromium.org/p/chromium/issues/detail?id=1002119
+TEST_P(QuicSessionTestServer, StreamFrameReceivedAfterFin) {
+ TestStream* stream = session_.CreateOutgoingBidirectionalStream();
+ QuicStreamFrame frame(stream->id(), true, 0, ",");
+ session_.OnStreamFrame(frame);
+
+ QuicStreamFrame frame1(stream->id(), false, 1, ",");
+ if (GetQuicReloadableFlag(quic_rst_if_stream_frame_beyond_close_offset)) {
+ EXPECT_CALL(*connection_, SendControlFrame(_));
+ EXPECT_CALL(*connection_,
+ OnStreamReset(stream->id(), QUIC_DATA_AFTER_CLOSE_OFFSET));
+ session_.OnStreamFrame(frame1);
+ EXPECT_TRUE(connection_->connected());
+ } else {
+ EXPECT_DEBUG_DEATH(session_.OnStreamFrame(frame1), "Check failed");
+ }
+}
+
} // namespace
} // namespace test
} // namespace quic
diff --git a/quic/core/quic_stream.cc b/quic/core/quic_stream.cc
index 43b8db8..ae302b9 100644
--- a/quic/core/quic_stream.cc
+++ b/quic/core/quic_stream.cc
@@ -6,6 +6,7 @@
#include <string>
+#include "net/third_party/quiche/src/quic/core/quic_error_codes.h"
#include "net/third_party/quiche/src/quic/core/quic_flow_controller.h"
#include "net/third_party/quiche/src/quic/core/quic_session.h"
#include "net/third_party/quiche/src/quic/core/quic_utils.h"
@@ -163,6 +164,12 @@
return;
}
+ if (GetQuicReloadableFlag(quic_rst_if_stream_frame_beyond_close_offset) &&
+ frame.offset + frame.data_length > sequencer_.close_offset()) {
+ Reset(QUIC_DATA_AFTER_CLOSE_OFFSET);
+ return;
+ }
+
if (frame.fin) {
fin_received_ = true;
}
@@ -387,6 +394,15 @@
frame.data_length, ". ", sequencer_.DebugString()));
return;
}
+
+ if (GetQuicReloadableFlag(quic_rst_if_stream_frame_beyond_close_offset)) {
+ QUIC_RELOADABLE_FLAG_COUNT(quic_rst_if_stream_frame_beyond_close_offset);
+ if (frame.offset + frame.data_length > sequencer_.close_offset()) {
+ Reset(QUIC_DATA_AFTER_CLOSE_OFFSET);
+ return;
+ }
+ }
+
if (frame.fin) {
fin_received_ = true;
if (fin_sent_) {
diff --git a/quic/core/quic_stream_sequencer.cc b/quic/core/quic_stream_sequencer.cc
index dc7b0cd..8f84fb9 100644
--- a/quic/core/quic_stream_sequencer.cc
+++ b/quic/core/quic_stream_sequencer.cc
@@ -38,6 +38,7 @@
QuicStreamSequencer::~QuicStreamSequencer() {}
void QuicStreamSequencer::OnStreamFrame(const QuicStreamFrame& frame) {
+ DCHECK_LE(frame.offset + frame.data_length, close_offset_);
++num_frames_received_;
const QuicStreamOffset byte_offset = frame.offset;
const size_t data_len = frame.data_length;
diff --git a/quic/core/quic_stream_sequencer_test.cc b/quic/core/quic_stream_sequencer_test.cc
index b57fa6e..8ad845b 100644
--- a/quic/core/quic_stream_sequencer_test.cc
+++ b/quic/core/quic_stream_sequencer_test.cc
@@ -370,15 +370,11 @@
EXPECT_TRUE(sequencer_->IsClosed());
}
-TEST_F(QuicStreamSequencerTest, MutipleOffsets) {
+TEST_F(QuicStreamSequencerTest, MultipleOffsets) {
OnFinFrame(3, "");
EXPECT_EQ(3u, QuicStreamSequencerPeer::GetCloseOffset(sequencer_.get()));
EXPECT_CALL(stream_, Reset(QUIC_MULTIPLE_TERMINATION_OFFSETS));
- OnFinFrame(5, "");
- EXPECT_EQ(3u, QuicStreamSequencerPeer::GetCloseOffset(sequencer_.get()));
-
- EXPECT_CALL(stream_, Reset(QUIC_MULTIPLE_TERMINATION_OFFSETS));
OnFinFrame(1, "");
EXPECT_EQ(3u, QuicStreamSequencerPeer::GetCloseOffset(sequencer_.get()));
@@ -761,10 +757,9 @@
// Regression test for https://crbug.com/992486.
TEST_F(QuicStreamSequencerTest, CorruptFinFrames) {
SetQuicReloadableFlag(quic_no_stream_data_after_reset, true);
- EXPECT_CALL(stream_, OnDataAvailable()).Times(1);
EXPECT_CALL(stream_, Reset(QUIC_MULTIPLE_TERMINATION_OFFSETS));
- OnFinFrame(0u, "");
+ OnFinFrame(2u, "");
OnFinFrame(0u, "a");
EXPECT_FALSE(sequencer_->HasBytesToRead());
}
