fix: handle null or empty named message registration [MTT-7771] (#2807)

* fix

Fixes issue where registering or unregistering named message that is null or empty does not throw an exception.
Fixes issue where it was possible that NetworkTransform could try to unregister a named message before it had been assigned a value.

* update

Actually decided to make both cases log an error and raised the minimum loglevel.

* test

Adding a test to verify that trying to register or unregister a null or empty named message will not throw an exception.

* style

removing whitespace

* update

Adding change log entries

Author: NoelStephensUnity

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -6,6 +6,19 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 
 Additional documentation and release notes are available at [Multiplayer Documentation](https://docs-multiplayer.unity3d.com).
 
+## [Unreleased]
+
+### Added
+
+### Fixed
+
+- Fixed issue where NetworkTransform could potentially attempt to "unregister" a named message prior to it being registered. (#2807)
+
+### Changed
+
+- Changed `CustomMessageManager` so it no longer attempts to register or "unregister" a null or empty string and will log an error if this condition occurs. (#2807)
+
+
 ## [1.8.0] - 2023-12-12
 
 ### Added

com.unity.netcode.gameobjects/Components/NetworkTransform.cs

@@ -2763,7 +2763,7 @@ public override void OnNetworkSpawn()
         public override void OnNetworkDespawn()
         {
             // During destroy, use NetworkBehaviour.NetworkManager as opposed to m_CachedNetworkManager
-            if (!NetworkManager.ShutdownInProgress && NetworkManager.CustomMessagingManager != null)
+            if (!NetworkManager.ShutdownInProgress && NetworkManager.CustomMessagingManager != null && !string.IsNullOrEmpty(m_MessageName))
             {
                 NetworkManager.CustomMessagingManager.UnregisterNamedMessageHandler(m_MessageName);
             }

com.unity.netcode.gameobjects/Runtime/Messaging/CustomMessageManager.cs

@@ -1,6 +1,7 @@
 using System;
 using System.Collections.Generic;
 using Unity.Collections;
+using UnityEngine;
 
 namespace Unity.Netcode
 {
@@ -199,6 +200,14 @@ internal void InvokeNamedMessage(ulong hash, ulong sender, FastBufferReader read
         /// <param name="callback">The callback to run when a named message is received.</param>
         public void RegisterNamedMessageHandler(string name, HandleNamedMessageDelegate callback)
         {
+            if (string.IsNullOrEmpty(name))
+            {
+                if (m_NetworkManager.LogLevel <= LogLevel.Error)
+                {
+                    Debug.LogError($"[{nameof(CustomMessagingManager.RegisterNamedMessageHandler)}] Cannot register a named message of type null or empty!");
+                }
+                return;
+            }
             var hash32 = XXHash.Hash32(name);
             var hash64 = XXHash.Hash64(name);
 
@@ -215,6 +224,15 @@ public void RegisterNamedMessageHandler(string name, HandleNamedMessageDelegate
         /// <param name="name">The name of the message.</param>
         public void UnregisterNamedMessageHandler(string name)
         {
+            if (string.IsNullOrEmpty(name))
+            {
+                if (m_NetworkManager.LogLevel <= LogLevel.Error)
+                {
+                    Debug.LogError($"[{nameof(CustomMessagingManager.UnregisterNamedMessageHandler)}] Cannot unregister a named message of type null or empty!");
+                }
+                return;
+            }
+
             var hash32 = XXHash.Hash32(name);
             var hash64 = XXHash.Hash64(name);
 

com.unity.netcode.gameobjects/Tests/Runtime/Messaging/NamedMessageTests.cs

@@ -86,6 +86,24 @@ public void NamedMessageIsReceivedOnHostWithContent()
             Assert.AreEqual(m_ServerNetworkManager.LocalClientId, receivedMessageSender);
         }
 
+        private void MockNamedMessageCallback(ulong sender, FastBufferReader reader)
+        {
+
+        }
+
+        [Test]
+        public void NullOrEmptyNamedMessageDoesNotThrowException()
+        {
+            LogAssert.Expect(UnityEngine.LogType.Error, $"[{nameof(CustomMessagingManager.RegisterNamedMessageHandler)}] Cannot register a named message of type null or empty!");
+            m_ServerNetworkManager.CustomMessagingManager.RegisterNamedMessageHandler(string.Empty, MockNamedMessageCallback);
+            LogAssert.Expect(UnityEngine.LogType.Error, $"[{nameof(CustomMessagingManager.RegisterNamedMessageHandler)}] Cannot register a named message of type null or empty!");
+            m_ServerNetworkManager.CustomMessagingManager.RegisterNamedMessageHandler(null, MockNamedMessageCallback);
+            LogAssert.Expect(UnityEngine.LogType.Error, $"[{nameof(CustomMessagingManager.UnregisterNamedMessageHandler)}] Cannot unregister a named message of type null or empty!");
+            m_ServerNetworkManager.CustomMessagingManager.UnregisterNamedMessageHandler(string.Empty);
+            LogAssert.Expect(UnityEngine.LogType.Error, $"[{nameof(CustomMessagingManager.UnregisterNamedMessageHandler)}] Cannot unregister a named message of type null or empty!");
+            m_ServerNetworkManager.CustomMessagingManager.UnregisterNamedMessageHandler(null);
+        }
+
         [UnityTest]
         public IEnumerator NamedMessageIsReceivedOnMultipleClientsWithContent()
         {