[core] Fixed seqno validity check by group receiver (#2142)

srtcore/common.h

@@ -634,12 +634,16 @@ class CSeqNo
    /// WITH A PRECONDITION that certainly @a seq1 is earlier than @a seq2.
    /// This can also include an enormously large distance between them,
    /// that is, exceeding the m_iSeqNoTH value (can be also used to test
-   /// if this distance is larger). Prior to calling this function the
-   /// caller must be certain that @a seq2 is a sequence coming from a
-   /// later time than @a seq1, and still, of course, this distance didn't
-   /// exceed m_iMaxSeqNo.
+   /// if this distance is larger).
+   /// Prior to calling this function the caller must be certain that
+   /// @a seq2 is a sequence coming from a later time than @a seq1,
+   /// and that the distance does not exceed m_iMaxSeqNo.
    inline static int seqlen(int32_t seq1, int32_t seq2)
-   {return (seq1 <= seq2) ? (seq2 - seq1 + 1) : (seq2 - seq1 + m_iMaxSeqNo + 2);}
+   {
+       SRT_ASSERT(seq1 >= 0 && seq1 <= m_iMaxSeqNo);
+       SRT_ASSERT(seq2 >= 0 && seq2 <= m_iMaxSeqNo);
+       return (seq1 <= seq2) ? (seq2 - seq1 + 1) : (seq2 - seq1 + m_iMaxSeqNo + 2);
+   }
 
    /// This behaves like seq2 - seq1, with the precondition that the true
    /// distance between two sequence numbers never exceeds m_iSeqNoTH.

srtcore/group.cpp

@@ -2148,6 +2148,43 @@ void CUDTGroup::updateWriteState()
     m_pGlobal->m_EPoll.update_events(id(), m_sPollID, SRT_EPOLL_OUT, true);
 }
 
+/// Validate iPktSeqno is in range
+/// (iBaseSeqno - m_iSeqNoTH/2; iBaseSeqno + m_iSeqNoTH).
+///
+/// EXPECT_EQ(isValidSeqno(125, 124), true); // behind
+/// EXPECT_EQ(isValidSeqno(125, 125), true); // behind
+/// EXPECT_EQ(isValidSeqno(125, 126), true); // the next in order
+///
+/// EXPECT_EQ(isValidSeqno(0, 0x3FFFFFFF - 2), true);  // ahead, but ok.
+/// EXPECT_EQ(isValidSeqno(0, 0x3FFFFFFF - 1), false); // too far ahead.
+/// EXPECT_EQ(isValidSeqno(0x3FFFFFFF + 2, 0x7FFFFFFF), false); // too far ahead.
+/// EXPECT_EQ(isValidSeqno(0x3FFFFFFF + 3, 0x7FFFFFFF), true); // ahead, but ok.
+/// EXPECT_EQ(isValidSeqno(0x3FFFFFFF, 0x1FFFFFFF + 2), false); // too far (behind)
+/// EXPECT_EQ(isValidSeqno(0x3FFFFFFF, 0x1FFFFFFF + 3), true); // behind, but ok
+/// EXPECT_EQ(isValidSeqno(0x70000000, 0x0FFFFFFF), true); // ahead, but ok
+/// EXPECT_EQ(isValidSeqno(0x70000000, 0x30000000 - 2), false); // too far ahead.
+/// EXPECT_EQ(isValidSeqno(0x70000000, 0x30000000 - 3), true); // ahead, but ok
+/// EXPECT_EQ(isValidSeqno(0x0FFFFFFF, 0), true);
+/// EXPECT_EQ(isValidSeqno(0x0FFFFFFF, 0x7FFFFFFF), true);
+/// EXPECT_EQ(isValidSeqno(0x0FFFFFFF, 0x70000000), false);
+/// EXPECT_EQ(isValidSeqno(0x0FFFFFFF, 0x70000001), false);
+/// EXPECT_EQ(isValidSeqno(0x0FFFFFFF, 0x70000002), true);  // behind by 536870910
+/// EXPECT_EQ(isValidSeqno(0x0FFFFFFF, 0x70000003), true);
+///
+/// @return false if @a iPktSeqno is not inside the valid range; otherwise true.
+static bool isValidSeqno(int32_t iBaseSeqno, int32_t iPktSeqno)
+{
+    const int32_t iLenAhead = CSeqNo::seqlen(iBaseSeqno, iPktSeqno);
+    if (iLenAhead >= 0 && iLenAhead < CSeqNo::m_iSeqNoTH)
+        return true;
+
+    const int32_t iLenBehind = CSeqNo::seqlen(iPktSeqno, iBaseSeqno);
+    if (iLenBehind >= 0 && iLenBehind < CSeqNo::m_iSeqNoTH / 2)
+        return true;
+
+    return false;
+}
+
 // The "app reader" version of the reading function.
 // This reads the packets from every socket treating them as independent
 // and prepared to work with the application. Then packets are sorted out
@@ -2407,7 +2444,7 @@ int CUDTGroup::recv(char* buf, int len, SRT_MSGCTRL& w_mc)
                 // embrace everything below.
 
                 // We need to first qualify the sequence, just for a case
-                if (m_RcvBaseSeqNo != SRT_SEQNO_NONE && abs(m_RcvBaseSeqNo - mctrl.pktseq) > CSeqNo::m_iSeqNoTH)
+                if (m_RcvBaseSeqNo != SRT_SEQNO_NONE && !isValidSeqno(m_RcvBaseSeqNo, mctrl.pktseq))
                 {
                     // This error should be returned if the link turns out
                     // to be the only one, or set to the group data.

test/test_seqno.cpp

@@ -16,7 +16,6 @@ TEST(CSeqNo, constants)
     EXPECT_EQ(CSeqNo::m_iSeqNoTH,  0x3FFFFFFF);
 }
 
-
 TEST(CSeqNo, seqcmp)
 {
     // Compare two seq#, considering the wraping.
@@ -31,7 +30,6 @@ TEST(CSeqNo, seqcmp)
     EXPECT_EQ(CSeqNo::seqcmp(1, 0x7FFFFFFF), 0x7FFFFFFE);   //  2147483646
 }
 
-
 TEST(CSeqNo, seqoff)
 {
     // seqoff: offset from the 2nd to the 1st seq#
@@ -48,6 +46,9 @@ TEST(CSeqNo, seqlen)
 {
     EXPECT_EQ(CSeqNo::seqlen(125, 125), 1);
     EXPECT_EQ(CSeqNo::seqlen(125, 126), 2);
+
+    EXPECT_EQ(CSeqNo::seqlen(2147483647, 0), 2);
+    EXPECT_EQ(CSeqNo::seqlen(0, 2147483647), -2147483648);
 }
 
 TEST(CUDT, getFlightSpan)
@@ -74,7 +75,6 @@ TEST(CSeqNo, incseq)
     EXPECT_EQ(CSeqNo::incseq(0x3FFFFFFF), 0x40000000);
 }
 
-
 TEST(CSeqNo, decseq)
 {
     // decseq: decrease the seq# by 1
@@ -84,7 +84,6 @@ TEST(CSeqNo, decseq)
     EXPECT_EQ(CSeqNo::decseq(0x40000000), 0x3FFFFFFF);
 }
 
-
 TEST(CSeqNo, incseqint)
 {
     // incseq: increase the seq# by 1
@@ -98,7 +97,6 @@ TEST(CSeqNo, incseqint)
     EXPECT_EQ(CSeqNo::incseq(0x3FFFFFFF, 0x40000001), 0x00000000);
 }
 
-
 TEST(CSeqNo, decseqint)
 {
     // decseq: decrease the seq# by 1
@@ -107,4 +105,3 @@ TEST(CSeqNo, decseqint)
     EXPECT_EQ(CSeqNo::decseq(0, 1),          0x7FFFFFFF);
     EXPECT_EQ(CSeqNo::decseq(0x40000000, 1), 0x3FFFFFFF);
 }
-