dcsctp: Limit the size of generated SACK chunks

Today, there is no actual limit on how large a SACK chunk can be. And
having limits is good to be able to stay within the MTU.

This commit adds a limit to the number of reported duplicate TSNs as
well as the number of reported gap-ack-blocks in a SACK chunk. These
limits are never expected to be reached in a real-life situation.

Bug: webrtc:12614
Change-Id: Ib2c143714a214cd3d961e8a52dac26a04b909b80
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/219464
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34108}

3 files changed, 47 insertions, 7 deletions

diff --git a/net/dcsctp/rx/data_tracker.cc b/net/dcsctp/rx/data_tracker.cc
index 68a4895ec8..5083e0981b 100644
--- a/net/dcsctp/rx/data_tracker.cc
+++ b/net/dcsctp/rx/data_tracker.cc
@@ -26,6 +26,9 @@
 
 namespace dcsctp {
 
+constexpr size_t DataTracker::kMaxDuplicateTsnReported;
+constexpr size_t DataTracker::kMaxGapAckBlocksReported;
+
 bool DataTracker::IsTSNValid(TSN tsn) const {
   UnwrappedTSN unwrapped_tsn = tsn_unwrapper_.PeekUnwrap(tsn);
 
@@ -52,7 +55,9 @@ void DataTracker::Observe(TSN tsn,
 
   // Old chunk already seen before?
   if (unwrapped_tsn <= last_cumulative_acked_tsn_) {
-    duplicate_tsns_.insert(unwrapped_tsn.Wrap());
+    if (duplicate_tsns_.size() < kMaxDuplicateTsnReported) {
+      duplicate_tsns_.insert(unwrapped_tsn.Wrap());
+    }
     return;
   }
 
@@ -69,7 +74,9 @@ void DataTracker::Observe(TSN tsn,
     bool inserted = additional_tsns_.insert(unwrapped_tsn).second;
     if (!inserted) {
       // Already seen before.
-      duplicate_tsns_.insert(unwrapped_tsn.Wrap());
+      if (duplicate_tsns_.size() < kMaxDuplicateTsnReported) {
+        duplicate_tsns_.insert(unwrapped_tsn.Wrap());
+      }
     }
   }
 
@@ -200,12 +207,14 @@ std::vector<SackChunk::GapAckBlock> DataTracker::CreateGapAckBlocks() const {
 
   auto flush = [&]() {
     if (first_tsn_in_block.has_value()) {
-      auto start_diff = UnwrappedTSN::Difference(*first_tsn_in_block,
+      if (gap_ack_blocks.size() < kMaxGapAckBlocksReported) {
+        auto start_diff = UnwrappedTSN::Difference(*first_tsn_in_block,
+                                                   last_cumulative_acked_tsn_);
+        auto end_diff = UnwrappedTSN::Difference(*last_tsn_in_block,
                                                  last_cumulative_acked_tsn_);
-      auto end_diff = UnwrappedTSN::Difference(*last_tsn_in_block,
-                                               last_cumulative_acked_tsn_);
-      gap_ack_blocks.emplace_back(static_cast<uint16_t>(start_diff),
-                                  static_cast<uint16_t>(end_diff));
+        gap_ack_blocks.emplace_back(static_cast<uint16_t>(start_diff),
+                                    static_cast<uint16_t>(end_diff));
+      }
       first_tsn_in_block = absl::nullopt;
       last_tsn_in_block = absl::nullopt;
     }
diff --git a/net/dcsctp/rx/data_tracker.h b/net/dcsctp/rx/data_tracker.h
index f5deaf147d..63b7d8fe36 100644
--- a/net/dcsctp/rx/data_tracker.h
+++ b/net/dcsctp/rx/data_tracker.h
@@ -38,6 +38,11 @@ namespace dcsctp {
 // 200ms, whatever is smallest).
 class DataTracker {
  public:
+  // The maximum number of duplicate TSNs that will be reported in a SACK.
+  static constexpr size_t kMaxDuplicateTsnReported = 20;
+  // The maximum number of gap-ack-blocks that will be reported in a SACK.
+  static constexpr size_t kMaxGapAckBlocksReported = 20;
+
   // The maximum number of accepted in-flight DATA chunks. This indicates the
   // maximum difference from this buffer's last cumulative ack TSN, and any
   // received data. Data received beyond this limit will be dropped, which will
diff --git a/net/dcsctp/rx/data_tracker_test.cc b/net/dcsctp/rx/data_tracker_test.cc
index 7518d6dc91..a19ac8ba4c 100644
--- a/net/dcsctp/rx/data_tracker_test.cc
+++ b/net/dcsctp/rx/data_tracker_test.cc
@@ -25,6 +25,7 @@ namespace dcsctp {
 namespace {
 using ::testing::ElementsAre;
 using ::testing::IsEmpty;
+using ::testing::SizeIs;
 using ::testing::UnorderedElementsAre;
 
 constexpr size_t kArwnd = 10000;
@@ -269,5 +270,30 @@ TEST_F(DataTrackerTest, ClearsDuplicateTsnsAfterCreatingSack) {
   EXPECT_THAT(sack2.duplicate_tsns(), IsEmpty());
 }
 
+TEST_F(DataTrackerTest, LimitsNumberOfDuplicatesReported) {
+  for (size_t i = 0; i < DataTracker::kMaxDuplicateTsnReported + 10; ++i) {
+    TSN tsn(11 + i);
+    buf_.Observe(tsn, AnyDataChunk::ImmediateAckFlag(false));
+    buf_.Observe(tsn, AnyDataChunk::ImmediateAckFlag(false));
+  }
+
+  SackChunk sack = buf_.CreateSelectiveAck(kArwnd);
+  EXPECT_THAT(sack.gap_ack_blocks(), IsEmpty());
+  EXPECT_THAT(sack.duplicate_tsns(),
+              SizeIs(DataTracker::kMaxDuplicateTsnReported));
+}
+
+TEST_F(DataTrackerTest, LimitsNumberOfGapAckBlocksReported) {
+  for (size_t i = 0; i < DataTracker::kMaxGapAckBlocksReported + 10; ++i) {
+    TSN tsn(11 + i * 2);
+    buf_.Observe(tsn, AnyDataChunk::ImmediateAckFlag(false));
+  }
+
+  SackChunk sack = buf_.CreateSelectiveAck(kArwnd);
+  EXPECT_EQ(sack.cumulative_tsn_ack(), TSN(11));
+  EXPECT_THAT(sack.gap_ack_blocks(),
+              SizeIs(DataTracker::kMaxGapAckBlocksReported));
+}
+
 }  // namespace
 }  // namespace dcsctp