Do not accept fin that would decrease the stream's final offset.
gfe-relnote: protected by gfe2_reloadable_flag_quic_no_decrease_in_final_offset
PiperOrigin-RevId: 276115511
Change-Id: I6c6fa7e6f8a6374aceb7ef0c19756ea3928479c9
diff --git a/quic/core/quic_stream_sequencer.cc b/quic/core/quic_stream_sequencer.cc
index 8f84fb9..ad07830 100644
--- a/quic/core/quic_stream_sequencer.cc
+++ b/quic/core/quic_stream_sequencer.cc
@@ -9,6 +9,7 @@
#include <string>
#include <utility>
+#include "net/third_party/quiche/src/quic/core/quic_error_codes.h"
#include "net/third_party/quiche/src/quic/core/quic_packets.h"
#include "net/third_party/quiche/src/quic/core/quic_stream.h"
#include "net/third_party/quiche/src/quic/core/quic_stream_sequencer_buffer.h"
@@ -26,6 +27,7 @@
QuicStreamSequencer::QuicStreamSequencer(StreamInterface* quic_stream)
: stream_(quic_stream),
buffered_frames_(kStreamReceiveWindowLimit),
+ highest_offset_(0),
close_offset_(std::numeric_limits<QuicStreamOffset>::max()),
blocked_(false),
num_frames_received_(0),
@@ -66,6 +68,7 @@
void QuicStreamSequencer::OnFrameData(QuicStreamOffset byte_offset,
size_t data_len,
const char* data_buffer) {
+ highest_offset_ = std::max(highest_offset_, byte_offset + data_len);
const size_t previous_readable_bytes = buffered_frames_.ReadableBytes();
size_t bytes_written;
std::string error_details;
@@ -123,10 +126,33 @@
// If there is a scheduled close, the new offset should match it.
if (close_offset_ != kMaxOffset && offset != close_offset_) {
- stream_->Reset(QUIC_MULTIPLE_TERMINATION_OFFSETS);
+ if (!GetQuicReloadableFlag(quic_no_decrease_in_final_offset)) {
+ stream_->Reset(QUIC_MULTIPLE_TERMINATION_OFFSETS);
+ return false;
+ }
+ QUIC_RELOADABLE_FLAG_COUNT_N(quic_no_decrease_in_final_offset, 1, 2);
+ stream_->CloseConnectionWithDetails(
+ QUIC_STREAM_SEQUENCER_INVALID_STATE,
+ QuicStrCat("Stream ", stream_->id(),
+ " received new final offset: ", offset,
+ ", which is different from close offset: ", close_offset_));
return false;
}
+ // The final offset should be no less than the highest offset that is
+ // received.
+ if (GetQuicReloadableFlag(quic_no_decrease_in_final_offset)) {
+ QUIC_RELOADABLE_FLAG_COUNT_N(quic_no_decrease_in_final_offset, 2, 2);
+ if (offset < highest_offset_) {
+ stream_->CloseConnectionWithDetails(
+ QUIC_STREAM_SEQUENCER_INVALID_STATE,
+ QuicStrCat(
+ "Stream ", stream_->id(), " received fin with offset: ", offset,
+ ", which reduces current highest offset: ", highest_offset_));
+ return false;
+ }
+ }
+
close_offset_ = offset;
MaybeCloseStream();
diff --git a/quic/core/quic_stream_sequencer.h b/quic/core/quic_stream_sequencer.h
index a735a1e..8f8a6a0 100644
--- a/quic/core/quic_stream_sequencer.h
+++ b/quic/core/quic_stream_sequencer.h
@@ -11,6 +11,7 @@
#include "net/third_party/quiche/src/quic/core/quic_packets.h"
#include "net/third_party/quiche/src/quic/core/quic_stream_sequencer_buffer.h"
+#include "net/third_party/quiche/src/quic/core/quic_types.h"
#include "net/third_party/quiche/src/quic/platform/api/quic_export.h"
namespace quic {
@@ -182,6 +183,9 @@
// Stores received data in offset order.
QuicStreamSequencerBuffer buffered_frames_;
+ // The highest offset that is received so far.
+ QuicStreamOffset highest_offset_;
+
// The offset, if any, we got a stream termination for. When this many bytes
// have been processed, the sequencer will be closed.
QuicStreamOffset close_offset_;
diff --git a/quic/core/quic_stream_sequencer_test.cc b/quic/core/quic_stream_sequencer_test.cc
index 8ad845b..15484d8 100644
--- a/quic/core/quic_stream_sequencer_test.cc
+++ b/quic/core/quic_stream_sequencer_test.cc
@@ -15,6 +15,7 @@
#include "net/third_party/quiche/src/quic/core/quic_utils.h"
#include "net/third_party/quiche/src/quic/platform/api/quic_arraysize.h"
#include "net/third_party/quiche/src/quic/platform/api/quic_expect_bug.h"
+#include "net/third_party/quiche/src/quic/platform/api/quic_flags.h"
#include "net/third_party/quiche/src/quic/platform/api/quic_logging.h"
#include "net/third_party/quiche/src/quic/platform/api/quic_string_piece.h"
#include "net/third_party/quiche/src/quic/platform/api/quic_test.h"
@@ -374,12 +375,15 @@
OnFinFrame(3, "");
EXPECT_EQ(3u, QuicStreamSequencerPeer::GetCloseOffset(sequencer_.get()));
- EXPECT_CALL(stream_, Reset(QUIC_MULTIPLE_TERMINATION_OFFSETS));
+ if (!GetQuicReloadableFlag(quic_no_decrease_in_final_offset)) {
+ EXPECT_CALL(stream_, Reset(QUIC_MULTIPLE_TERMINATION_OFFSETS));
+ } else {
+ EXPECT_CALL(stream_, CloseConnectionWithDetails(
+ QUIC_STREAM_SEQUENCER_INVALID_STATE,
+ "Stream 1 received new final offset: 1, which is "
+ "different from close offset: 3"));
+ }
OnFinFrame(1, "");
- EXPECT_EQ(3u, QuicStreamSequencerPeer::GetCloseOffset(sequencer_.get()));
-
- OnFinFrame(3, "");
- EXPECT_EQ(3u, QuicStreamSequencerPeer::GetCloseOffset(sequencer_.get()));
}
class QuicSequencerRandomTest : public QuicStreamSequencerTest {
@@ -757,13 +761,32 @@
// Regression test for https://crbug.com/992486.
TEST_F(QuicStreamSequencerTest, CorruptFinFrames) {
SetQuicReloadableFlag(quic_no_stream_data_after_reset, true);
- EXPECT_CALL(stream_, Reset(QUIC_MULTIPLE_TERMINATION_OFFSETS));
+ if (!GetQuicReloadableFlag(quic_no_decrease_in_final_offset)) {
+ EXPECT_CALL(stream_, Reset(QUIC_MULTIPLE_TERMINATION_OFFSETS));
+ } else {
+ EXPECT_CALL(stream_, CloseConnectionWithDetails(
+ QUIC_STREAM_SEQUENCER_INVALID_STATE,
+ "Stream 1 received new final offset: 1, which is "
+ "different from close offset: 2"));
+ }
OnFinFrame(2u, "");
OnFinFrame(0u, "a");
EXPECT_FALSE(sequencer_->HasBytesToRead());
}
+// Regression test for crbug.com/1015693
+TEST_F(QuicStreamSequencerTest, ReceiveFinLessThanHighestOffset) {
+ SetQuicReloadableFlag(quic_no_decrease_in_final_offset, true);
+ EXPECT_CALL(stream_, OnDataAvailable()).Times(1);
+ EXPECT_CALL(stream_, CloseConnectionWithDetails(
+ QUIC_STREAM_SEQUENCER_INVALID_STATE,
+ "Stream 1 received fin with offset: 0, which "
+ "reduces current highest offset: 3"));
+ OnFrame(0u, "abc");
+ OnFinFrame(0u, "");
+}
+
} // namespace
} // namespace test
} // namespace quic
