fix: cap maximum disconnect timeout (#3810)

* chore: avoid overflow when calculating maxCapacity

* Changelog update

* Add test

* chore: handle the case of the disconnect timeout being 0

* add tests for disconnectTimout set to 0

* changelog update

---------

Co-authored-by: Emma <[email protected]>
Co-authored-by: Noel Stephens <[email protected]>

Author: 3 people

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -42,6 +42,7 @@ Additional documentation and release notes are available at [Multiplayer Documen
 ### Changed
 
 - Improve performance of `ParentSyncMessage`. (#3814)
+- If the Unity Transport Disconnect Timeout is set to 0 in the Editor, the timeout will be entirely disabeled. (#3810)
 - Improve performance of `DestroyObjectMessage`. (#3801)
 - Improve performance of `CreateObjectMessage`. (#3800)
 - First pass of CoreCLR engine API changes. (#3799)
@@ -53,6 +54,7 @@ Additional documentation and release notes are available at [Multiplayer Documen
 
 - Ensure `NetworkBehaviour.IsSessionOwner` is correctly set when a new session owner is promoted. (#3817)
 - Reset extended ownership flags on `NetworkObject` despawn. (#3817)
+- Fixed issue where maxCapacity calculation overflows if a developer sets a very, very high (large) m_DisconnectTimeoutMS in the Editor for Unity Transport. (#3810)
 - Fixed issues with the "Client-server quickstart for Netcode for GameObjects" script having static methods and properties. (#3787)
 - Fixed issue where a warning message was being logged upon a client disconnecting from a server when the log level is set to developer. (#3786)
 - Fixed issue where the server or host would no longer have access to the transport id to client id table when processing a transport level client disconnect event. (#3786)

com.unity.netcode.gameobjects/Runtime/Transports/UTP/BatchedSendQueue.cs

@@ -31,6 +31,10 @@ internal struct BatchedSendQueue : IDisposable
         public const int PerMessageOverhead = sizeof(int);
 
         internal const int MinimumMinimumCapacity = 4096;
+        // int.MaxValue is odd and maximum must be even.
+        // This is safe to be so large as the queue is dynamically allocated.
+        // It's very unlikely that the actual send queue will ever grow this large
+        internal const int MaximumMaximumCapacity = int.MaxValue - 1;
 
         // Indices into m_HeadTailIndicies.
         private const int k_HeadInternalIndex = 0;

com.unity.netcode.gameobjects/Runtime/Transports/UTP/UnityTransport.cs

@@ -1515,7 +1515,22 @@ public override void Send(ulong clientId, ArraySegment<byte> payload, NetworkDel
                 // the only case where a full send queue causes a connection loss. Full unreliable
                 // send queues are dealt with by flushing it out to the network or simply dropping
                 // new messages if that fails.
-                var maxCapacity = m_MaxSendQueueSize > 0 ? m_MaxSendQueueSize : m_DisconnectTimeoutMS * k_MaxReliableThroughput;
+                var maxCapacity = m_MaxSendQueueSize;
+                if (maxCapacity <= 0)
+                {
+                    // Setting m_DisconnectTimeoutMS to zero will disable the timeout entirely
+                    // Set the capacity as if the disconnect timeout is the largest possible value
+                    if (m_DisconnectTimeoutMS == 0)
+                    {
+                        maxCapacity = BatchedSendQueue.MaximumMaximumCapacity;
+                    }
+                    else
+                    {
+                        // Avoids overflow when m_DisconnectTimeoutMS is set to a very high value
+                        var fullCalculation = Math.BigMul(m_DisconnectTimeoutMS, k_MaxReliableThroughput);
+                        maxCapacity = (int)Math.Min(fullCalculation, BatchedSendQueue.MaximumMaximumCapacity);
+                    }
+                }
 
                 queue = new BatchedSendQueue(Math.Max(maxCapacity, m_MaxPayloadSize));
                 m_SendQueue.Add(sendTarget, queue);

com.unity.netcode.gameobjects/Tests/Runtime/Transports/UnityTransportTestHelpers.cs

@@ -96,16 +96,16 @@ public static IEnumerator EnsureNoNetworkEvent(List<TransportEvent> events, floa
         }
 
         // Common code to initialize a UnityTransport that logs its events.
-        public static void InitializeTransport(out UnityTransport transport, out List<TransportEvent> events, int maxPayloadSize = UnityTransport.InitialMaxPayloadSize, int maxSendQueueSize = 0, NetworkFamily family = NetworkFamily.Ipv4)
+        public static void InitializeTransport(out UnityTransport transport, out List<TransportEvent> events, int maxPayloadSize = UnityTransport.InitialMaxPayloadSize, int maxSendQueueSize = 0, NetworkFamily family = NetworkFamily.Ipv4, int disconnectTimeout = NetworkParameterConstants.DisconnectTimeoutMS)
         {
-            InitializeTransport(out transport, out events, string.Empty, maxPayloadSize, maxSendQueueSize, family);
+            InitializeTransport(out transport, out events, string.Empty, maxPayloadSize, maxSendQueueSize, family, disconnectTimeout);
         }
 
         /// <summary>
-        /// Interanl version with identifier parameter
+        /// Internal version with identifier parameter
         /// </summary>
         internal static void InitializeTransport(out UnityTransport transport, out List<TransportEvent> events, string identifier,
-            int maxPayloadSize = UnityTransport.InitialMaxPayloadSize, int maxSendQueueSize = 0, NetworkFamily family = NetworkFamily.Ipv4)
+            int maxPayloadSize = UnityTransport.InitialMaxPayloadSize, int maxSendQueueSize = 0, NetworkFamily family = NetworkFamily.Ipv4, int disconnectTimeout = NetworkParameterConstants.DisconnectTimeoutMS)
         {
             var logger = new TransportEventLogger()
             {
@@ -118,6 +118,7 @@ internal static void InitializeTransport(out UnityTransport transport, out List<
             transport.OnTransportEvent += logger.HandleEvent;
             transport.MaxPayloadSize = maxPayloadSize;
             transport.MaxSendQueueSize = maxSendQueueSize;
+            transport.DisconnectTimeoutMS = disconnectTimeout;
 
             if (family == NetworkFamily.Ipv6)
             {

com.unity.netcode.gameobjects/Tests/Runtime/Transports/UnityTransportTests.cs

@@ -189,6 +189,86 @@ public IEnumerator SendMaximumPayloadSize(
             yield return null;
         }
 
+        [UnityTest]
+        public IEnumerator VeryLargeDisconnectTimeout()
+        {
+            // The calculation will never be lower than the UnityTransport.InitialMaxPayloadSize
+            // We want to send a message larger than that size to ensure the maximum is high enough
+            var payloadSize = UnityTransport.InitialMaxPayloadSize * 2;
+
+            var disconnectTimeout = int.MaxValue;
+
+            InitializeTransport(out m_Server, out m_ServerEvents, payloadSize, disconnectTimeout: disconnectTimeout);
+            InitializeTransport(out m_Client1, out m_Client1Events, payloadSize, disconnectTimeout: disconnectTimeout);
+            Assert.That(m_Server.DisconnectTimeoutMS, Is.EqualTo(disconnectTimeout));
+            Assert.That(m_Client1.DisconnectTimeoutMS, Is.EqualTo(disconnectTimeout));
+
+            m_Server.StartServer();
+            m_Client1.StartClient();
+
+            yield return WaitForNetworkEvent(NetworkEvent.Connect, m_Client1Events);
+
+            var payloadData = new byte[payloadSize];
+            for (int i = 0; i < payloadData.Length; i++)
+            {
+                payloadData[i] = (byte)i;
+            }
+
+            var payload = new ArraySegment<byte>(payloadData);
+            m_Client1.Send(m_Client1.ServerClientId, payload, NetworkDelivery.Reliable);
+
+            yield return WaitForNetworkEvent(NetworkEvent.Data, m_ServerEvents, MaxNetworkEventWaitTime * 4);
+
+            Assert.AreEqual(payloadSize, m_ServerEvents[1].Data.Count);
+
+            var receivedArray = m_ServerEvents[1].Data.Array;
+            var receivedArrayOffset = m_ServerEvents[1].Data.Offset;
+            for (int i = 0; i < payloadSize; i++)
+            {
+                Assert.AreEqual(payloadData[i], receivedArray[receivedArrayOffset + i]);
+            }
+        }
+
+        [UnityTest]
+        public IEnumerator ZeroDisconnectTimeoutSetToZero()
+        {
+            // The calculation will never be lower than the UnityTransport.InitialMaxPayloadSize
+            // We want to send a message larger than that size to ensure the maximum is high enough
+            var payloadSize = UnityTransport.InitialMaxPayloadSize * 2;
+
+            var disconnectTimeout = 0;
+
+            InitializeTransport(out m_Server, out m_ServerEvents, payloadSize, disconnectTimeout: disconnectTimeout);
+            InitializeTransport(out m_Client1, out m_Client1Events, payloadSize, disconnectTimeout: disconnectTimeout);
+            Assert.That(m_Server.DisconnectTimeoutMS, Is.EqualTo(disconnectTimeout));
+            Assert.That(m_Client1.DisconnectTimeoutMS, Is.EqualTo(disconnectTimeout));
+
+            m_Server.StartServer();
+            m_Client1.StartClient();
+
+            yield return WaitForNetworkEvent(NetworkEvent.Connect, m_Client1Events);
+
+            var payloadData = new byte[payloadSize];
+            for (int i = 0; i < payloadData.Length; i++)
+            {
+                payloadData[i] = (byte)i;
+            }
+
+            var payload = new ArraySegment<byte>(payloadData);
+            m_Client1.Send(m_Client1.ServerClientId, payload, NetworkDelivery.Reliable);
+
+            yield return WaitForNetworkEvent(NetworkEvent.Data, m_ServerEvents, MaxNetworkEventWaitTime * 4);
+
+            Assert.AreEqual(payloadSize, m_ServerEvents[1].Data.Count);
+
+            var receivedArray = m_ServerEvents[1].Data.Array;
+            var receivedArrayOffset = m_ServerEvents[1].Data.Offset;
+            for (int i = 0; i < payloadSize; i++)
+            {
+                Assert.AreEqual(payloadData[i], receivedArray[receivedArrayOffset + i]);
+            }
+        }
+
         // Check making multiple sends to a client in a single frame.
         [UnityTest]
         public IEnumerator MultipleSendsSingleFrame(