From 133346e3e88ca60a1ff8bb1f2840c7ef2fabf91c Mon Sep 17 00:00:00 2001 From: gamer147 Date: Mon, 1 Jun 2026 11:48:17 -0400 Subject: [PATCH] refactor(battle-node): SocketIoFrame throws on namespace; typed JSON construction --- SVSim.BattleNode/Wire/SocketIoFrame.cs | 51 +++++++++++-------- .../BattleNode/Wire/SocketIoFrameTests.cs | 10 ++++ 2 files changed, 41 insertions(+), 20 deletions(-) diff --git a/SVSim.BattleNode/Wire/SocketIoFrame.cs b/SVSim.BattleNode/Wire/SocketIoFrame.cs index 3a0b2d6..1cd56b1 100644 --- a/SVSim.BattleNode/Wire/SocketIoFrame.cs +++ b/SVSim.BattleNode/Wire/SocketIoFrame.cs @@ -1,6 +1,7 @@ using System.Text; using System.Text.Encodings.Web; using System.Text.Json; +using System.Text.Json.Nodes; namespace SVSim.BattleNode.Wire; @@ -65,11 +66,17 @@ public sealed class SocketIoFrame cursor = dashIdx + 1; } - // Skip namespace (only present if a '/' starts here, terminated by ','). + // Namespace prefix (only present if '/' starts here, terminated by ','). v1 only + // uses the default namespace; anything else is a protocol surprise we should + // surface rather than silently route to default. If we ever support non-default + // namespaces, capture into a property and let callers branch. if (cursor < raw.Length && raw[cursor] == '/') { var commaIdx = raw.IndexOf(',', cursor); - cursor = commaIdx >= 0 ? commaIdx + 1 : raw.Length; + var ns = commaIdx >= 0 ? raw.Substring(cursor, commaIdx - cursor) : raw.Substring(cursor); + throw new ArgumentException( + $"Socket.IO namespaces aren't supported — got '{ns}'. v1 expects default namespace only.", + nameof(raw)); } int? ackId = null; @@ -126,19 +133,15 @@ public sealed class SocketIoFrame /// public static SocketIoFrame BinaryEventWithAttachments(string eventName, IReadOnlyList attachments) { - // Build the placeholder-only portion (without the event name) — event name is stored separately. - var sb = new StringBuilder(); - sb.Append('['); + // Build placeholders via the typed Nodes API; event name is stored separately. + var placeholders = new JsonArray(); for (var i = 0; i < attachments.Count; i++) { - if (i > 0) sb.Append(','); - sb.Append("{\"_placeholder\":true,\"num\":").Append(i).Append('}'); - } - sb.Append(']'); - JsonElement[] args; - using (var doc = JsonDocument.Parse(sb.ToString())) - { - args = doc.RootElement.EnumerateArray().Select(el => el.Clone()).ToArray(); + placeholders.Add(new JsonObject + { + ["_placeholder"] = true, + ["num"] = i, + }); } return new SocketIoFrame( @@ -146,20 +149,28 @@ public sealed class SocketIoFrame ackId: null, attachmentCount: attachments.Count, eventName: eventName, - rawArgs: args, + rawArgs: NodesToElements(placeholders), binaryAttachments: attachments); } /// Build an ack response with a single int argument (the spec's pubSeq echo). public static SocketIoFrame AckResponse(int ackId, int arg) { - JsonElement[] args; - using (var doc = JsonDocument.Parse($"[{arg}]")) - { - args = doc.RootElement.EnumerateArray().Select(el => el.Clone()).ToArray(); - } + var args = new JsonArray { arg }; return new SocketIoFrame( - SocketIoPacketType.Ack, ackId, 0, null, args, Array.Empty()); + SocketIoPacketType.Ack, ackId, 0, null, NodesToElements(args), Array.Empty()); + } + + /// + /// Convert a into the [] that + /// stores. The current storage type is + /// because produces it from ; this helper + /// keeps the typed-construction call sites without changing . + /// + private static JsonElement[] NodesToElements(JsonArray nodes) + { + using var doc = JsonDocument.Parse(nodes.ToJsonString()); + return doc.RootElement.EnumerateArray().Select(el => el.Clone()).ToArray(); } /// diff --git a/SVSim.UnitTests/BattleNode/Wire/SocketIoFrameTests.cs b/SVSim.UnitTests/BattleNode/Wire/SocketIoFrameTests.cs index 9cf43c5..f0e9d50 100644 --- a/SVSim.UnitTests/BattleNode/Wire/SocketIoFrameTests.cs +++ b/SVSim.UnitTests/BattleNode/Wire/SocketIoFrameTests.cs @@ -105,6 +105,16 @@ public class SocketIoFrameTests Assert.That(text, Is.EqualTo(wire)); } + [Test] + public void Parse_NamespacePrefix_Throws() + { + // v1 only supports the default namespace. A "/foo," prefix used to be silently + // skipped, which would route a frame meant for namespace /foo to the default + // handler. Fail loud instead so we'd notice if the client ever started using one. + var ex = Assert.Throws(() => SocketIoFrame.Parse("2/foo,[\"msg\"]")); + Assert.That(ex!.Message, Does.Contain("/foo")); + } + [Test] public void Encode_EventNameWithSpecialChars_IsJsonEscaped() {