From f4a60345e90adf7f2aaf9bf4cdaa601c8b695bd8 Mon Sep 17 00:00:00 2001 From: Simon Lemay Date: Mon, 7 Apr 2025 11:11:18 -0400 Subject: [PATCH 1/4] fix: Don't accept invalid connections in UnityTransport.Send [1.X] --- com.unity.netcode.gameobjects/CHANGELOG.md | 1 + .../Runtime/Transports/UTP/UnityTransport.cs | 7 ++++++- .../Transports/UnityTransportTestHelpers.cs | 17 +++++++++++++++++ .../Runtime/Transports/UnityTransportTests.cs | 17 +++++++++++++++++ 4 files changed, 41 insertions(+), 1 deletion(-) diff --git a/com.unity.netcode.gameobjects/CHANGELOG.md b/com.unity.netcode.gameobjects/CHANGELOG.md index 4ea3d7cb38..a961ec4af2 100644 --- a/com.unity.netcode.gameobjects/CHANGELOG.md +++ b/com.unity.netcode.gameobjects/CHANGELOG.md @@ -16,6 +16,7 @@ Additional documentation and release notes are available at [Multiplayer Documen ### Fixed +- Fixed an issue in `UnityTransport` where the transport would accept sends on invalid connections, leading to a useless memory allocation and confusing error message. - Fixed initial `NetworkTransform` spawn, ensure it uses world space. (#3361) - Fixed issue where `AnticipatedNetworkVariable` previous value returned by `AnticipatedNetworkVariable.OnAuthoritativeValueChanged` is updated correctly on the non-authoritative side. (#3322) diff --git a/com.unity.netcode.gameobjects/Runtime/Transports/UTP/UnityTransport.cs b/com.unity.netcode.gameobjects/Runtime/Transports/UTP/UnityTransport.cs index 8a8afcdbac..c910b679f1 100644 --- a/com.unity.netcode.gameobjects/Runtime/Transports/UTP/UnityTransport.cs +++ b/com.unity.netcode.gameobjects/Runtime/Transports/UTP/UnityTransport.cs @@ -1342,8 +1342,13 @@ public override NetcodeNetworkEvent PollEvent(out ulong clientId, out ArraySegme /// The delivery type (QoS) to send data with public override void Send(ulong clientId, ArraySegment payload, NetworkDelivery networkDelivery) { - var pipeline = SelectSendPipeline(networkDelivery); + var connection = ParseClientId(clientId); + if (!m_Driver.IsCreated || m_Driver.GetConnectionState(connection) != NetworkConnection.State.Connected) + { + return; + } + var pipeline = SelectSendPipeline(networkDelivery); if (pipeline != m_ReliableSequencedPipeline && payload.Count > m_MaxPayloadSize) { Debug.LogError($"Unreliable payload of size {payload.Count} larger than configured 'Max Payload Size' ({m_MaxPayloadSize})."); diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/Transports/UnityTransportTestHelpers.cs b/com.unity.netcode.gameobjects/Tests/Runtime/Transports/UnityTransportTestHelpers.cs index 75144f66c3..aeb25434fc 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/Transports/UnityTransportTestHelpers.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/Transports/UnityTransportTestHelpers.cs @@ -36,6 +36,23 @@ public static IEnumerator WaitForNetworkEvent(NetworkEvent type, List events, float timeout = MaxNetworkEventWaitTime) + { + int initialCount = events.Count; + float startTime = Time.realtimeSinceStartup; + + while (Time.realtimeSinceStartup - startTime < timeout) + { + if (events.Count > initialCount) + { + Assert.Fail("Received unexpected network event."); + } + + yield return new WaitForSeconds(0.01f); + } + } + // Common code to initialize a UnityTransport that logs its events. public static void InitializeTransport(out UnityTransport transport, out List events, int maxPayloadSize = UnityTransport.InitialMaxPayloadSize, int maxSendQueueSize = 0, NetworkFamily family = NetworkFamily.Ipv4) diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/Transports/UnityTransportTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/Transports/UnityTransportTests.cs index 7d5b5c30f5..bb405c4a62 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/Transports/UnityTransportTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/Transports/UnityTransportTests.cs @@ -554,5 +554,22 @@ public IEnumerator DoesNotActAfterShutdown([Values] AfterShutdownAction afterShu m_Server.DisconnectLocalClient(); } } + + [UnityTest] + public IEnumerator DoesNotAttemptToSendOnInvalidConnections() + { + InitializeTransport(out m_Server, out m_ServerEvents); + InitializeTransport(out m_Client1, out m_Client1Events); + + m_Server.StartServer(); + m_Client1.StartClient(); + + yield return WaitForNetworkEvent(NetworkEvent.Connect, m_Client1Events); + + var data = new ArraySegment(new byte[42]); + m_Server.Send(m_Client1.ServerClientId, data, NetworkDelivery.Reliable); + + yield return EnsureNoNetworkEvent(m_Client1Events); + } } } From 7ce0e7fe1c51a5001dda2150fe2b2492ee418c54 Mon Sep 17 00:00:00 2001 From: Simon Lemay Date: Mon, 7 Apr 2025 11:15:41 -0400 Subject: [PATCH 2/4] Fix test case --- .../Tests/Runtime/Transports/UnityTransportTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/Transports/UnityTransportTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/Transports/UnityTransportTests.cs index bb405c4a62..c33f268b2a 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/Transports/UnityTransportTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/Transports/UnityTransportTests.cs @@ -567,7 +567,7 @@ public IEnumerator DoesNotAttemptToSendOnInvalidConnections() yield return WaitForNetworkEvent(NetworkEvent.Connect, m_Client1Events); var data = new ArraySegment(new byte[42]); - m_Server.Send(m_Client1.ServerClientId, data, NetworkDelivery.Reliable); + m_Server.Send(0, data, NetworkDelivery.Reliable); yield return EnsureNoNetworkEvent(m_Client1Events); } From 0934a5fbd39322c271e5dd59c22ab32f5158975b Mon Sep 17 00:00:00 2001 From: Simon Lemay Date: Mon, 7 Apr 2025 11:18:13 -0400 Subject: [PATCH 3/4] Add PR number to CHANGELOG entry --- com.unity.netcode.gameobjects/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/com.unity.netcode.gameobjects/CHANGELOG.md b/com.unity.netcode.gameobjects/CHANGELOG.md index a961ec4af2..a88b6daea1 100644 --- a/com.unity.netcode.gameobjects/CHANGELOG.md +++ b/com.unity.netcode.gameobjects/CHANGELOG.md @@ -16,7 +16,7 @@ Additional documentation and release notes are available at [Multiplayer Documen ### Fixed -- Fixed an issue in `UnityTransport` where the transport would accept sends on invalid connections, leading to a useless memory allocation and confusing error message. +- Fixed an issue in `UnityTransport` where the transport would accept sends on invalid connections, leading to a useless memory allocation and confusing error message. (#3383) - Fixed initial `NetworkTransform` spawn, ensure it uses world space. (#3361) - Fixed issue where `AnticipatedNetworkVariable` previous value returned by `AnticipatedNetworkVariable.OnAuthoritativeValueChanged` is updated correctly on the non-authoritative side. (#3322) From 8686bb666ce1764943eda137c1b40836c9a03a40 Mon Sep 17 00:00:00 2001 From: michalChrobot Date: Tue, 8 Apr 2025 09:58:36 +0200 Subject: [PATCH 4/4] Added new test APIs to pvpExceptions.json --- pvpExceptions.json | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pvpExceptions.json b/pvpExceptions.json index 0a338d445a..d506649f88 100644 --- a/pvpExceptions.json +++ b/pvpExceptions.json @@ -2591,6 +2591,7 @@ "Unity.Netcode.RuntimeTests.UnityTransportTestHelpers: undocumented", "Unity.Netcode.RuntimeTests.UnityTransportTestHelpers: MaxNetworkEventWaitTime: undocumented", "Unity.Netcode.RuntimeTests.UnityTransportTestHelpers: IEnumerator WaitForNetworkEvent(NetworkEvent, List, float): undocumented", + "Unity.Netcode.RuntimeTests.UnityTransportTestHelpers: IEnumerator EnsureNoNetworkEvent(List, float): undocumented", "Unity.Netcode.RuntimeTests.UnityTransportTestHelpers: void InitializeTransport(out UnityTransport, out List, int, int, NetworkFamily): undocumented", "Unity.Netcode.RuntimeTests.UnityTransportTestHelpers.TransportEvent: undocumented", "Unity.Netcode.RuntimeTests.UnityTransportTestHelpers.TransportEvent: Type: undocumented", @@ -2617,6 +2618,7 @@ "Unity.Netcode.RuntimeTests.UnityTransportTests: IEnumerator SendQueuesFlushedOnRemoteClientDisconnect(NetworkDelivery): undocumented", "Unity.Netcode.RuntimeTests.UnityTransportTests: IEnumerator ReliablePayloadsCanBeLargerThanMaximum(): undocumented", "Unity.Netcode.RuntimeTests.UnityTransportTests: IEnumerator DoesNotActAfterShutdown(AfterShutdownAction): undocumented", + "Unity.Netcode.RuntimeTests.UnityTransportTests: IEnumerator DoesNotAttemptToSendOnInvalidConnections(): undocumented", "Unity.Netcode.RuntimeTests.UnityTransportTests.AfterShutdownAction: undocumented", "Unity.Netcode.RuntimeTests.UnityTransportTests.AfterShutdownAction: Send: undocumented", "Unity.Netcode.RuntimeTests.UnityTransportTests.AfterShutdownAction: DisconnectRemoteClient: undocumented",