From 4dd61343aa9791441c7515645355604171ff7a7b Mon Sep 17 00:00:00 2001 From: gamer147 Date: Mon, 1 Jun 2026 11:13:24 -0400 Subject: [PATCH] fix(battle-node): clip SIO ack arg instead of checked-cast throwing on overflow --- SVSim.BattleNode/Sessions/BattleSession.cs | 29 +++++++++++- .../BattleNode/Sessions/ClipAckArgTests.cs | 44 +++++++++++++++++++ 2 files changed, 71 insertions(+), 2 deletions(-) create mode 100644 SVSim.UnitTests/BattleNode/Sessions/ClipAckArgTests.cs diff --git a/SVSim.BattleNode/Sessions/BattleSession.cs b/SVSim.BattleNode/Sessions/BattleSession.cs index 6428599..dc7ad39 100644 --- a/SVSim.BattleNode/Sessions/BattleSession.cs +++ b/SVSim.BattleNode/Sessions/BattleSession.cs @@ -342,10 +342,35 @@ public sealed class BattleSession } } + /// + /// Clip a long ack arg into the int range Socket.IO v2's typed AckResponse API accepts. + /// Logs a warning on clip; the implausibly-large pubSeq case is observationally + /// indistinguishable at the client (BestHTTP.SocketIO discards the echoed value), so + /// clipping is safer than the prior checked((int)arg) that threw and killed the + /// session on overflow. + /// + internal static int ClipAckArg(long arg, ILogger log, string battleId) + { + if (arg > int.MaxValue) + { + log.LogWarning( + "BattleSession {Bid}: pubSeq {Seq} exceeds int.MaxValue; clipping ack arg.", + battleId, arg); + return int.MaxValue; + } + if (arg < int.MinValue) + { + log.LogWarning( + "BattleSession {Bid}: pubSeq {Seq} below int.MinValue; clipping ack arg.", + battleId, arg); + return int.MinValue; + } + return (int)arg; + } + private async Task SendSioAckAsync(int ackId, long arg) { - // checked() ensures we'd notice if pubSeq ever exceeded int.MaxValue (not realistic but defensive). - var ack = SocketIoFrame.AckResponse(ackId, checked((int)arg)); + var ack = SocketIoFrame.AckResponse(ackId, ClipAckArg(arg, _log, BattleId)); var (text, _) = ack.Encode(); var eioText = $"{(int)EngineIoPacketType.Message}{text}"; await SendTextAsync(eioText, _sessionCt); diff --git a/SVSim.UnitTests/BattleNode/Sessions/ClipAckArgTests.cs b/SVSim.UnitTests/BattleNode/Sessions/ClipAckArgTests.cs new file mode 100644 index 0000000..2e1d964 --- /dev/null +++ b/SVSim.UnitTests/BattleNode/Sessions/ClipAckArgTests.cs @@ -0,0 +1,44 @@ +using Microsoft.Extensions.Logging.Abstractions; +using NUnit.Framework; +using SVSim.BattleNode.Sessions; + +namespace SVSim.UnitTests.BattleNode.Sessions; + +[TestFixture] +public class ClipAckArgTests +{ + [Test] + public void InRange_ReturnsArgUnchanged() + { + var result = BattleSession.ClipAckArg(42L, NullLogger.Instance, battleId: "b"); + Assert.That(result, Is.EqualTo(42)); + } + + [Test] + public void AboveIntMax_ClipsToIntMaxValue() + { + var result = BattleSession.ClipAckArg((long)int.MaxValue + 1L, NullLogger.Instance, battleId: "b"); + Assert.That(result, Is.EqualTo(int.MaxValue)); + } + + [Test] + public void BelowIntMin_ClipsToIntMinValue() + { + var result = BattleSession.ClipAckArg((long)int.MinValue - 1L, NullLogger.Instance, battleId: "b"); + Assert.That(result, Is.EqualTo(int.MinValue)); + } + + [Test] + public void AtIntMaxBoundary_ReturnsIntMaxValue() + { + var result = BattleSession.ClipAckArg((long)int.MaxValue, NullLogger.Instance, battleId: "b"); + Assert.That(result, Is.EqualTo(int.MaxValue)); + } + + [Test] + public void AtIntMinBoundary_ReturnsIntMinValue() + { + var result = BattleSession.ClipAckArg((long)int.MinValue, NullLogger.Instance, battleId: "b"); + Assert.That(result, Is.EqualTo(int.MinValue)); + } +}