fix: NetworkIdentity component bitmask shifting overflows (#3941)

* Failing test for netid bitmask shifting

* Fix netid bitmask shifting typing

Otherwise it uses ints and overflows when shifting more than 32

Assets/Mirror/Core/NetworkIdentity.cs

@@ -863,7 +863,7 @@ internal void OnStopLocalPlayer()
             for (int i = 0; i < components.Length; ++i)
             {
                 NetworkBehaviour component = components[i];
-                ulong nthBit = (1u << i);
+                ulong nthBit = 1ul << i;
 
                 bool dirty = component.IsDirty();
 
@@ -910,7 +910,7 @@ ulong ClientDirtyMask()
 
                 // on client, only consider owned components with SyncDirection to server
                 NetworkBehaviour component = components[i];
-                ulong nthBit = (1u << i);
+                ulong nthBit = 1ul << i;
 
                 if (isOwned && component.syncDirection == SyncDirection.ClientToServer)
                 {
@@ -928,7 +928,7 @@ ulong ClientDirtyMask()
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         internal static bool IsDirty(ulong mask, int index)
         {
-            ulong nthBit = (ulong)(1 << index);
+            ulong nthBit = 1ul << index;
             return (mask & nthBit) != 0;
         }
 

Assets/Mirror/Tests/Common/MirrorTest.cs

@@ -1,4 +1,5 @@
 // base class for networking tests to make things easier.
+using System;
 using System.Collections.Generic;
 using System.Linq;
 using NUnit.Framework;
@@ -81,6 +82,18 @@ protected void CreateNetworked(out GameObject go, out NetworkIdentity identity)
             instantiated.Add(go);
         }
 
+        protected void CreateNetworked(out GameObject go, out NetworkIdentity identity, Action<NetworkIdentity> preAwake)
+        {
+            go = new GameObject();
+            identity = go.AddComponent<NetworkIdentity>();
+            preAwake(identity);
+            // Awake is only called in play mode.
+            // call manually for initialization.
+            identity.Awake();
+            // track
+            instantiated.Add(go);
+        }
+
         // create GameObject + NetworkIdentity + NetworkBehaviour<T>
         // add to tracker list if needed (useful for cleanups afterwards)
         protected void CreateNetworked<T>(out GameObject go, out NetworkIdentity identity, out T component)
@@ -269,6 +282,44 @@ protected void CreateNetworkedAndSpawn(
             Assert.That(NetworkClient.spawned.ContainsKey(serverIdentity.netId));
         }
 
+        // create GameObject + NetworkIdentity + NetworkBehaviour & SPAWN
+        // => preAwake callbacks can be used to add network behaviours to the NI
+        // => ownerConnection can be NetworkServer.localConnection if needed.
+        // => returns objects from client and from server.
+        //    will be same in host mode.
+        protected void CreateNetworkedAndSpawn(
+            out GameObject serverGO, out NetworkIdentity serverIdentity, Action<NetworkIdentity> serverPreAwake,
+            out GameObject clientGO, out NetworkIdentity clientIdentity, Action<NetworkIdentity> clientPreAwake,
+            NetworkConnectionToClient ownerConnection = null)
+        {
+            // server & client need to be active before spawning
+            Debug.Assert(NetworkClient.active, "NetworkClient needs to be active before spawning.");
+            Debug.Assert(NetworkServer.active, "NetworkServer needs to be active before spawning.");
+
+            // create one on server, one on client
+            // (spawning has to find it on client, it doesn't create it)
+            CreateNetworked(out serverGO, out serverIdentity, serverPreAwake);
+            CreateNetworked(out clientGO, out clientIdentity, clientPreAwake);
+
+            // give both a scene id and register it on client for spawnables
+            clientIdentity.sceneId = serverIdentity.sceneId = (ulong)serverGO.GetHashCode();
+            NetworkClient.spawnableObjects[clientIdentity.sceneId] = clientIdentity;
+
+            // spawn
+            NetworkServer.Spawn(serverGO, ownerConnection);
+            ProcessMessages();
+
+            // double check isServer/isClient. avoids debugging headaches.
+            Assert.That(serverIdentity.isServer, Is.True);
+            Assert.That(clientIdentity.isClient, Is.True);
+
+            // double check that we have authority if we passed an owner connection
+            if (ownerConnection != null)
+                Debug.Assert(clientIdentity.isOwned == true, $"Behaviour Had Wrong Authority when spawned, This means that the test is broken and will give the wrong results");
+
+            // make sure the client really spawned it.
+            Assert.That(NetworkClient.spawned.ContainsKey(serverIdentity.netId));
+        }
         // create GameObject + NetworkIdentity + NetworkBehaviour & SPAWN
         // => ownerConnection can be NetworkServer.localConnection if needed.
         protected void CreateNetworkedAndSpawn<T>(out GameObject go, out NetworkIdentity identity, out T component, NetworkConnectionToClient ownerConnection = null)

Assets/Mirror/Tests/Editor/NetworkIdentity/NetworkIdentitySerializationTests.cs

@@ -1,4 +1,5 @@
 // OnDe/SerializeSafely tests.
+using System.Collections.Generic;
 using System.Text.RegularExpressions;
 using Mirror.Tests.EditorBehaviours.NetworkIdentities;
 using NUnit.Framework;
@@ -42,7 +43,7 @@ public void SerializeAndDeserializeAll()
             );
 
             // set sync modes
-            serverOwnerComp.syncMode     = clientOwnerComp.syncMode     = SyncMode.Owner;
+            serverOwnerComp.syncMode = clientOwnerComp.syncMode = SyncMode.Owner;
             serverObserversComp.syncMode = clientObserversComp.syncMode = SyncMode.Observers;
 
             // set unique values on server components
@@ -65,10 +66,127 @@ public void SerializeAndDeserializeAll()
             // deserialize client object with OBSERVERS payload
             reader = new NetworkReader(observersWriter.ToArray());
             clientIdentity.DeserializeClient(reader, true);
-            Assert.That(clientOwnerComp.value, Is.EqualTo(null)); // owner mode shouldn't be in data
+            Assert.That(clientOwnerComp.value, Is.EqualTo(null));   // owner mode shouldn't be in data
             Assert.That(clientObserversComp.value, Is.EqualTo(42)); // observers mode should be in data
         }
 
+        // test serialize -> deserialize of any supported number of components
+        [Test]
+        public void SerializeAndDeserializeN([NUnit.Framework.Range(1, 64)] int numberOfNBs)
+        {
+            List<SerializeTest1NetworkBehaviour> serverNBs = new List<SerializeTest1NetworkBehaviour>();
+            List<SerializeTest1NetworkBehaviour> clientNBs = new List<SerializeTest1NetworkBehaviour>();
+            // need two of both versions so we can serialize -> deserialize
+            CreateNetworkedAndSpawn(
+                out _, out NetworkIdentity serverIdentity, ni =>
+                {
+                    for (int i = 0; i < numberOfNBs; i++)
+                    {
+                        SerializeTest1NetworkBehaviour nb = ni.gameObject.AddComponent<SerializeTest1NetworkBehaviour>();
+                        nb.syncInterval = 0;
+                        nb.syncMode = SyncMode.Observers;
+                        serverNBs.Add(nb);
+                    }
+                },
+                out _, out NetworkIdentity clientIdentity, ni =>
+                {
+                    for (int i = 0; i < numberOfNBs; i++)
+                    {
+                        SerializeTest1NetworkBehaviour nb = ni.gameObject.AddComponent<SerializeTest1NetworkBehaviour>();
+                        nb.syncInterval = 0;
+                        nb.syncMode = SyncMode.Observers;
+                        clientNBs.Add(nb);
+                    }
+                }
+            );
+
+            // INITIAL SYNC
+            // set unique values on server components
+            for (int i = 0; i < serverNBs.Count; i++)
+            {
+                serverNBs[i].value = (i + 1) * 3;
+                serverNBs[i].SetDirty();
+            }
+
+            // serialize server object
+            serverIdentity.SerializeServer(true, ownerWriter, observersWriter);
+
+            // deserialize client object with OBSERVERS payload
+            NetworkReader reader = new NetworkReader(observersWriter.ToArray());
+            clientIdentity.DeserializeClient(reader, true);
+            for (int i = 0; i < clientNBs.Count; i++)
+            {
+                int expected = (i + 1) * 3;
+                Assert.That(clientNBs[i].value, Is.EqualTo(expected), $"Expected the clientNBs[{i}] to have a value of {expected}");
+            }
+
+            // clear dirty bits for incremental sync
+            foreach (SerializeTest1NetworkBehaviour serverNB in serverNBs)
+                serverNB.ClearAllDirtyBits();
+
+            // INCREMENTAL SYNC ALL
+            // set unique values on server components
+            for (int i = 0; i < serverNBs.Count; i++)
+            {
+                serverNBs[i].value = (i + 1) * 11;
+                serverNBs[i].SetDirty();
+            }
+
+            ownerWriter.Reset();
+            observersWriter.Reset();
+            // serialize server object
+            serverIdentity.SerializeServer(false, ownerWriter, observersWriter);
+
+            // deserialize client object with OBSERVERS payload
+            reader = new NetworkReader(observersWriter.ToArray());
+            clientIdentity.DeserializeClient(reader, false);
+            for (int i = 0; i < clientNBs.Count; i++)
+            {
+                int expected = (i + 1) * 11;
+                Assert.That(clientNBs[i].value, Is.EqualTo(expected), $"Expected the clientNBs[{i}] to have a value of {expected}");
+            }
+
+            // clear dirty bits for incremental sync
+            foreach (SerializeTest1NetworkBehaviour serverNB in serverNBs)
+                serverNB.ClearAllDirtyBits();
+
+            // INCREMENTAL SYNC INDIVIDUAL
+            for (int i = 0; i < numberOfNBs; i++)
+            {
+                // reset all client nbs
+                foreach (SerializeTest1NetworkBehaviour clientNB in clientNBs)
+                    clientNB.value = 0;
+
+                int expected = (i + 1) * 7;
+
+                // set unique value on server components
+                serverNBs[i].value = expected;
+                serverNBs[i].SetDirty();
+
+                ownerWriter.Reset();
+                observersWriter.Reset();
+                // serialize server object
+                serverIdentity.SerializeServer(false, ownerWriter, observersWriter);
+
+                // deserialize client object with OBSERVERS payload
+                reader = new NetworkReader(observersWriter.ToArray());
+                clientIdentity.DeserializeClient(reader, false);
+                for (int index = 0; index < clientNBs.Count; index++)
+                {
+                    SerializeTest1NetworkBehaviour clientNB = clientNBs[index];
+                    if (index == i)
+                    {
+                        Assert.That(clientNB.value, Is.EqualTo(expected), $"Expected the clientNBs[{index}] to have a value of {expected}");
+                    }
+                    else
+                    {
+                        Assert.That(clientNB.value, Is.EqualTo(0), $"Expected the clientNBs[{index}] to have a value of 0 since we're not syncing that index (on sync of #{i})");
+                    }
+                }
+            }
+        }
+
+
         // serialization should work even if a component throws an exception.
         // so if first component throws, second should still be serialized fine.
         [Test]
@@ -150,20 +268,20 @@ public void TooManyComponents()
         public void ErrorCorrection()
         {
             int original = 0x12345678;
-            byte safety  =       0x78; // last byte
+            byte safety = 0x78; // last byte
 
             // correct size shouldn't be corrected
-            Assert.That(NetworkBehaviour.ErrorCorrection(original + 0, safety),  Is.EqualTo(original));
+            Assert.That(NetworkBehaviour.ErrorCorrection(original + 0, safety), Is.EqualTo(original));
 
             // read a little too much
-            Assert.That(NetworkBehaviour.ErrorCorrection(original + 1, safety),   Is.EqualTo(original));
-            Assert.That(NetworkBehaviour.ErrorCorrection(original + 2, safety),   Is.EqualTo(original));
-            Assert.That(NetworkBehaviour.ErrorCorrection(original + 42, safety),  Is.EqualTo(original));
+            Assert.That(NetworkBehaviour.ErrorCorrection(original + 1, safety), Is.EqualTo(original));
+            Assert.That(NetworkBehaviour.ErrorCorrection(original + 2, safety), Is.EqualTo(original));
+            Assert.That(NetworkBehaviour.ErrorCorrection(original + 42, safety), Is.EqualTo(original));
 
             // read a little too less
-            Assert.That(NetworkBehaviour.ErrorCorrection(original - 1, safety),   Is.EqualTo(original));
-            Assert.That(NetworkBehaviour.ErrorCorrection(original - 2, safety),   Is.EqualTo(original));
-            Assert.That(NetworkBehaviour.ErrorCorrection(original - 42, safety),  Is.EqualTo(original));
+            Assert.That(NetworkBehaviour.ErrorCorrection(original - 1, safety), Is.EqualTo(original));
+            Assert.That(NetworkBehaviour.ErrorCorrection(original - 2, safety), Is.EqualTo(original));
+            Assert.That(NetworkBehaviour.ErrorCorrection(original - 42, safety), Is.EqualTo(original));
 
             // reading way too much / less is expected to fail.
             // we can only correct the last byte, not more.