fix(battle-node): SocketIoFrame disposal safety + escaping + empty-args encoding
- Wrap all JsonDocument.Parse calls in using blocks and Clone() each retained JsonElement to eliminate UAF hazard after GC. - Use JsonSerializer.Serialize with UnsafeRelaxedJsonEscaping so event names with " or \ produce \" / \ rather than " / plain \; avoids malformed JSON on Encode(). - Guard the [ ] block in Encode() behind EventName-or-args check so Connect/Disconnect packets round-trip as bare "0"/"1" not "0[]". - Add three regression tests: Connect no-bracket, Event round-trip, special-char event name escaping. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -1,8 +1,17 @@
|
||||
using System.Text;
|
||||
using System.Text.Encodings.Web;
|
||||
using System.Text.Json;
|
||||
|
||||
namespace SVSim.BattleNode.Wire;
|
||||
|
||||
file static class SocketIoJsonOptions
|
||||
{
|
||||
internal static readonly JsonSerializerOptions EventNameOptions = new()
|
||||
{
|
||||
Encoder = JavaScriptEncoder.UnsafeRelaxedJsonEscaping
|
||||
};
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Socket.IO v2 packet. Wire form: <c><type><N>-<ackId?>[json-args]</c> where
|
||||
/// <c><N>-</c> appears only on binary types (5/6). For binary events/acks, the JSON contains
|
||||
@@ -72,9 +81,16 @@ public sealed class SocketIoFrame
|
||||
}
|
||||
|
||||
var argsJson = cursor < raw.Length ? raw.Substring(cursor) : string.Empty;
|
||||
var allElements = string.IsNullOrEmpty(argsJson)
|
||||
? Array.Empty<JsonElement>()
|
||||
: JsonDocument.Parse(argsJson).RootElement.EnumerateArray().ToArray();
|
||||
JsonElement[] allElements;
|
||||
if (string.IsNullOrEmpty(argsJson))
|
||||
{
|
||||
allElements = Array.Empty<JsonElement>();
|
||||
}
|
||||
else
|
||||
{
|
||||
using var doc = JsonDocument.Parse(argsJson);
|
||||
allElements = doc.RootElement.EnumerateArray().Select(el => el.Clone()).ToArray();
|
||||
}
|
||||
|
||||
string? eventName = null;
|
||||
JsonElement[] rawArgs;
|
||||
@@ -119,7 +135,11 @@ public sealed class SocketIoFrame
|
||||
sb.Append("{\"_placeholder\":true,\"num\":").Append(i).Append('}');
|
||||
}
|
||||
sb.Append(']');
|
||||
var args = JsonDocument.Parse(sb.ToString()).RootElement.EnumerateArray().ToArray();
|
||||
JsonElement[] args;
|
||||
using (var doc = JsonDocument.Parse(sb.ToString()))
|
||||
{
|
||||
args = doc.RootElement.EnumerateArray().Select(el => el.Clone()).ToArray();
|
||||
}
|
||||
|
||||
return new SocketIoFrame(
|
||||
SocketIoPacketType.BinaryEvent,
|
||||
@@ -133,7 +153,11 @@ public sealed class SocketIoFrame
|
||||
/// <summary>Build an ack response with a single int argument (the spec's pubSeq echo).</summary>
|
||||
public static SocketIoFrame AckResponse(int ackId, int arg)
|
||||
{
|
||||
var args = JsonDocument.Parse($"[{arg}]").RootElement.EnumerateArray().ToArray();
|
||||
JsonElement[] args;
|
||||
using (var doc = JsonDocument.Parse($"[{arg}]"))
|
||||
{
|
||||
args = doc.RootElement.EnumerateArray().Select(el => el.Clone()).ToArray();
|
||||
}
|
||||
return new SocketIoFrame(
|
||||
SocketIoPacketType.Ack, ackId, 0, null, args, Array.Empty<byte[]>());
|
||||
}
|
||||
@@ -152,20 +176,22 @@ public sealed class SocketIoFrame
|
||||
}
|
||||
if (AckId.HasValue) sb.Append(AckId.Value);
|
||||
// Re-serialize args — for event/binary-event types, re-prepend the event name.
|
||||
sb.Append('[');
|
||||
var prependEventName = Type is SocketIoPacketType.Event or SocketIoPacketType.BinaryEvent
|
||||
&& EventName is not null;
|
||||
if (prependEventName)
|
||||
bool hasJsonPayload = EventName is not null || RawArgs.Length > 0;
|
||||
if (hasJsonPayload)
|
||||
{
|
||||
sb.Append('"').Append(EventName).Append('"');
|
||||
if (RawArgs.Length > 0) sb.Append(',');
|
||||
sb.Append('[');
|
||||
if (EventName is not null)
|
||||
{
|
||||
sb.Append(JsonSerializer.Serialize(EventName, SocketIoJsonOptions.EventNameOptions));
|
||||
if (RawArgs.Length > 0) sb.Append(',');
|
||||
}
|
||||
for (var i = 0; i < RawArgs.Length; i++)
|
||||
{
|
||||
if (i > 0) sb.Append(',');
|
||||
sb.Append(RawArgs[i].GetRawText());
|
||||
}
|
||||
sb.Append(']');
|
||||
}
|
||||
for (var i = 0; i < RawArgs.Length; i++)
|
||||
{
|
||||
if (i > 0) sb.Append(',');
|
||||
sb.Append(RawArgs[i].GetRawText());
|
||||
}
|
||||
sb.Append(']');
|
||||
return (sb.ToString(), BinaryAttachments);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -85,4 +85,34 @@ public class SocketIoFrameTests
|
||||
var header = SocketIoFrame.Parse("51-[\"msg\",{\"_placeholder\":true,\"num\":0}]");
|
||||
Assert.Throws<ArgumentException>(() => header.WithAttachments(Array.Empty<byte[]>()));
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void Encode_ConnectPacket_HasNoBracketedArgs()
|
||||
{
|
||||
// Regression for an earlier bug where Encode always emitted "[]".
|
||||
var frame = SocketIoFrame.Parse("0");
|
||||
var (text, bins) = frame.Encode();
|
||||
Assert.That(text, Is.EqualTo("0"));
|
||||
Assert.That(bins, Is.Empty);
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void ParseThenEncode_EventWithAck_RoundTripsByteForByte()
|
||||
{
|
||||
const string wire = "27[\"msg\",42]";
|
||||
var frame = SocketIoFrame.Parse(wire);
|
||||
var (text, _) = frame.Encode();
|
||||
Assert.That(text, Is.EqualTo(wire));
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void Encode_EventNameWithSpecialChars_IsJsonEscaped()
|
||||
{
|
||||
var frame = SocketIoFrame.BinaryEventWithAttachments(
|
||||
eventName: "weird \"name\" with \\ backslash",
|
||||
attachments: new[] { new byte[] { 0x00 } });
|
||||
var (text, _) = frame.Encode();
|
||||
// The event name must be JSON-escaped: each " becomes \", and the literal \ becomes \\.
|
||||
Assert.That(text, Does.Contain("\"weird \\\"name\\\" with \\\\ backslash\""));
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user