Add fault tolerance to GameData messages

src/Empostor.Server/Net/State/Game.Data.cs

@@ -76,6 +76,18 @@ namespace Empostor.Server.Net.State
             return default;
         }
 
+        private enum GameDataResult
+        {
+            /// <summary>Keep the current message in the packet and continue with the next one.</summary>
+            Continue,
+
+            /// <summary>Remove the current message from the packet and continue with the next one.</summary>
+            Remove,
+
+            /// <summary>Drop the whole packet (a cheat was detected and the connection should not relay it).</summary>
+            Abort,
+        }
+
         public async ValueTask<bool> HandleGameDataAsync(IMessageReader parent, ClientPlayer sender, bool toPlayer)
         {
             // Find target player.
@@ -93,11 +105,52 @@ namespace Empostor.Server.Net.State
                 _logger.LogTrace("Received GameData for target {0}.", targetId);
             }
 
-            // Parse GameData messages.
+            var startPosition = parent.Position;
+
             while (parent.Position < parent.Length)
             {
                 using var reader = parent.ReadMessage();
 
+                if (sender.Client.Player == null)
+                {
+                    return false;
+                }
+
+                if (toPlayer && (target == null || !_players.ContainsKey(target.Client.Id)))
+                {
+                    return false;
+                }
+
+                GameDataResult result;
+
+                try
+                {
+                    result = await HandleGameDataInnerAsync(reader, sender, target);
+                }
+                catch (Exception ex)
+                {
+                    _logger.LogError(ex, "{Code} - Error while handling a GameData message, skipping it", Code);
+                    parent.RemoveMessage(reader);
+                    continue;
+                }
+
+                if (result == GameDataResult.Abort)
+                {
+                    // A cheat was detected, drop the whole packet.
+                    return false;
+                }
+
+                if (result == GameDataResult.Remove)
+                {
+                    parent.RemoveMessage(reader);
+                }
+            }
+
+            return parent.Length > startPosition;
+        }
+
+        private async ValueTask<GameDataResult> HandleGameDataInnerAsync(IMessageReader reader, ClientPlayer sender, ClientPlayer? target)
+        {
             switch (reader.Tag)
             {
                 case GameDataTag.DataFlag:
@@ -112,7 +165,7 @@ namespace Empostor.Server.Net.State
                         _logger.LogWarning("{Code} - Received DataFlag for unregistered NetId {NetId}", Code, netId);
                     }
 
-                        break;
+                    return GameDataResult.Continue;
                 }
 
                 case GameDataTag.RpcFlag:
@@ -122,8 +175,7 @@ namespace Empostor.Server.Net.State
                     {
                         if (!await obj.HandleRpcAsync(sender, target, (RpcCalls)reader.ReadByte(), reader))
                         {
-                                parent.RemoveMessage(reader);
-                                continue;
+                            return GameDataResult.Remove;
                         }
                     }
                     else
@@ -131,7 +183,7 @@ namespace Empostor.Server.Net.State
                         _logger.LogWarning("{Code} - Received RpcFlag for unregistered NetId {NetId}", Code, netId);
                     }
 
-                        break;
+                    return GameDataResult.Continue;
                 }
 
                 case GameDataTag.SpawnFlag:
@@ -141,7 +193,7 @@ namespace Empostor.Server.Net.State
                     {
                         if (await sender.Client.ReportCheatAsync(new CheatContext(nameof(GameDataTag.SpawnFlag)), CheatCategory.MustBeHost, "Tried to send SpawnFlag as non-host."))
                         {
-                                return false;
+                            return GameDataResult.Abort;
                         }
                     }
 
@@ -164,7 +216,7 @@ namespace Empostor.Server.Net.State
                                 innerNetObject.GetType().Name,
                                 componentsCount,
                                 components.Count);
-                                continue;
+                            return GameDataResult.Continue;
                         }
 
                         _logger.LogDebug(
@@ -202,11 +254,11 @@ namespace Empostor.Server.Net.State
                             await OnSpawnAsync(sender, obj);
                         }
 
-                            continue;
+                        return GameDataResult.Continue;
                     }
 
                     _logger.LogWarning("Couldn't find spawnable object {ObjectId}", objectId);
-                        break;
+                    return GameDataResult.Continue;
                 }
 
                 // Only the host is allowed to despawn objects.
@@ -222,7 +274,7 @@ namespace Empostor.Server.Net.State
                                 sender.Client.Name,
                                 sender.Client.Id,
                                 netId);
-                                return false;
+                            return GameDataResult.Abort;
                         }
 
                         RemoveNetObject(obj);
@@ -238,7 +290,7 @@ namespace Empostor.Server.Net.State
                             netId);
                     }
 
-                        break;
+                    return GameDataResult.Continue;
                 }
 
                 case GameDataTag.SceneChangeFlag:
@@ -253,7 +305,7 @@ namespace Empostor.Server.Net.State
                             "Player {0} ({1}) tried to send SceneChangeFlag for another player.",
                             sender.Client.Name,
                             sender.Client.Id);
-                            return false;
+                        return GameDataResult.Abort;
                     }
 
                     // According to game assembly, sender is only allowed to send OnlineGame.
@@ -264,7 +316,7 @@ namespace Empostor.Server.Net.State
                             sender.Client.Name,
                             sender.Client.Id,
                             scene);
-                            return false;
+                        return GameDataResult.Abort;
                     }
 
                     sender.Scene = scene;
@@ -274,7 +326,7 @@ namespace Empostor.Server.Net.State
                     await SyncServerObjectsAsync(sender);
                     await SpawnPlayerInfoAsync(sender);
 
-                        break;
+                    return GameDataResult.Continue;
                 }
 
                 case GameDataTag.ReadyFlag:
@@ -287,11 +339,11 @@ namespace Empostor.Server.Net.State
                             "Player {0} ({1}) tried to send ReadyFlag for another player.",
                             sender.Client.Name,
                             sender.Client.Id);
-                            return false;
+                        return GameDataResult.Abort;
                     }
 
                     _logger.LogTrace("> IsReady {0}", clientId);
-                        break;
+                    return GameDataResult.Continue;
                 }
 
                 case GameDataTag.ConsoleDeclareClientPlatformFlag:
@@ -303,30 +355,21 @@ namespace Empostor.Server.Net.State
                     {
                         if (await sender.Client.ReportCheatAsync(new CheatContext(nameof(GameDataTag.ConsoleDeclareClientPlatformFlag)), CheatCategory.Ownership, "Client sent info with wrong client id"))
                         {
-                                return false;
+                            return GameDataResult.Abort;
                         }
                     }
 
                     sender.Platform = platform;
 
-                        break;
+                    return GameDataResult.Continue;
                 }
 
                 default:
                 {
                     _logger.LogWarning("{Code} - Bad GameData tag {Tag}", Code, reader.Tag);
-                        break;
+                    return GameDataResult.Continue;
                 }
             }
-
-                if (sender.Client.Player == null)
-                {
-                    // Disconnect handler was probably invoked, cancel the rest.
-                    return false;
-                }
-            }
-
-            return true;
         }
 
         private async ValueTask OnSpawnAsync(ClientPlayer sender, InnerNetObject netObj)