From 885ce226584d8b1f6b40ea00b0c163abac1fb27e Mon Sep 17 00:00:00 2001
From: Forest <forest@innersloth.com>
Date: Mon, 22 Mar 2021 16:32:52 -0700
Subject: [PATCH] Fix an off-by-one bug when receiving out of order messages
 that wrap the last received id past ushort.MaxValue

---
 Hazel.UnitTests/UdpConnectionTestHarness.cs |  2 ++
 Hazel.UnitTests/UdpReliabilityTests.cs      | 30 +++++++++++++++++++++
 Hazel/Udp/UdpConnection.Reliable.cs         |  4 +--
 3 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/Hazel.UnitTests/UdpConnectionTestHarness.cs b/Hazel.UnitTests/UdpConnectionTestHarness.cs
index c7cde8d..8414e0c 100644
--- a/Hazel.UnitTests/UdpConnectionTestHarness.cs
+++ b/Hazel.UnitTests/UdpConnectionTestHarness.cs
@@ -10,6 +10,8 @@ namespace Hazel.UnitTests
     internal class UdpConnectionTestHarness : UdpConnection
     {
         public List<MessageReader> BytesSent = new List<MessageReader>();
+        public ushort ReliableReceiveLast => this.reliableReceiveLast;
+
 
         public override void Connect(byte[] bytes = null, int timeout = 5000)
         {
diff --git a/Hazel.UnitTests/UdpReliabilityTests.cs b/Hazel.UnitTests/UdpReliabilityTests.cs
index ef36485..38ddc9e 100644
--- a/Hazel.UnitTests/UdpReliabilityTests.cs
+++ b/Hazel.UnitTests/UdpReliabilityTests.cs
@@ -8,6 +8,36 @@ namespace Hazel.UnitTests
     [TestClass]
     public class UdpReliabilityTests
     {
+        [TestMethod]
+        public void TestReliableWrapOffByOne()
+        {
+            List<MessageReader> messagesReceived = new List<MessageReader>();
+
+            UdpConnectionTestHarness dut = new UdpConnectionTestHarness();
+            dut.DataReceived += evt =>
+            {
+                messagesReceived.Add(evt.Message);
+            };
+
+            MessageWriter data = MessageWriter.Get(SendOption.Reliable);
+
+            Assert.AreEqual(ushort.MaxValue, dut.ReliableReceiveLast);
+                        
+            SetReliableId(data, 10);
+            dut.Test_Receive(data);
+
+            // This message may not be received if there is an off-by-one error when marking missed pkts up to 10.
+            SetReliableId(data, 9); 
+            dut.Test_Receive(data);
+
+            // Both messages should be received.
+            Assert.AreEqual(2, messagesReceived.Count);
+            messagesReceived.Clear();
+
+            Assert.AreEqual(2, dut.BytesSent.Count);
+            dut.BytesSent.Clear();
+        }
+
         [TestMethod]
         public void TestThatAllMessagesAreReceived()
         {
diff --git a/Hazel/Udp/UdpConnection.Reliable.cs b/Hazel/Udp/UdpConnection.Reliable.cs
index 46ffc49..83feabd 100644
--- a/Hazel/Udp/UdpConnection.Reliable.cs
+++ b/Hazel/Udp/UdpConnection.Reliable.cs
@@ -56,7 +56,7 @@ namespace Hazel.Udp
         /// <summary>
         ///     The packet id that was received last.
         /// </summary>
-        private volatile ushort reliableReceiveLast = ushort.MaxValue;
+        protected volatile ushort reliableReceiveLast = ushort.MaxValue;
 
         private object PingLock = new object();
 
@@ -364,7 +364,7 @@ namespace Hazel.Udp
                     else
                     {
                         int cnt = (ushort.MaxValue - reliableReceiveLast) + id;
-                        for (ushort i = 1; i < cnt; ++i)
+                        for (ushort i = 1; i <= cnt; ++i)
                         {
                             reliableDataPacketsMissing.Add((ushort)(i + reliableReceiveLast));
                         }
-- 
2.39.5