diff --git a/src/NosCore.Packets/Attributes/PacketIndexAttribute.cs b/src/NosCore.Packets/Attributes/PacketIndexAttribute.cs
index 75d28b0b..5d375259 100644
--- a/src/NosCore.Packets/Attributes/PacketIndexAttribute.cs
+++ b/src/NosCore.Packets/Attributes/PacketIndexAttribute.cs
@@ -31,5 +31,12 @@ public PacketIndexAttribute(int index, string specialSeparator)
public bool RemoveHeader { get; set; }
public bool RemoveHash { get; set; }
+
+ ///
+ /// Encode spaces as "^" even when nothing would break without it. The last field of a
+ /// packet keeps its spaces by default, which is what chat lines need; a few fields
+ /// carry text the client expects "^"-encoded regardless of where they sit.
+ ///
+ public bool EscapeSpaces { get; set; }
}
}
\ No newline at end of file
diff --git a/src/NosCore.Packets/NosCore.Packets.csproj b/src/NosCore.Packets/NosCore.Packets.csproj
index c62d0442..2c30ec12 100644
--- a/src/NosCore.Packets/NosCore.Packets.csproj
+++ b/src/NosCore.Packets/NosCore.Packets.csproj
@@ -12,7 +12,7 @@
https://github.com/NosCoreIO/NosCore.Packets.git
nostale, noscore, chickenapi, nostale private server source, nostale emulator
- 21.0.0
+ 21.1.0
false
NosCore's Packets (Nostale packets) defined over classes
MIT
diff --git a/src/NosCore.Packets/Serializer.cs b/src/NosCore.Packets/Serializer.cs
index e53ce85c..1c443d6e 100644
--- a/src/NosCore.Packets/Serializer.cs
+++ b/src/NosCore.Packets/Serializer.cs
@@ -80,17 +80,33 @@ private Expression DefaultSerializer(Expression specificTypeExpression, Expressi
return ConcatExpression(splitter, specificTypeExpression);
}
- private Expression StringSerializer(Expression exp, bool isLastIndex, bool isOptional, Expression splitter)
+ // escapeSeparator is the separator this field is joined to the packet with, which is
+ // what a value has to avoid containing. It is not the same expression as splitter: an
+ // ordinary property declares no special separator, so escaping against that one left
+ // every plain space-separated string free to split its own field in two.
+ private Expression StringSerializer(Expression exp, bool isLastIndex, bool isOptional, Expression splitter,
+ Expression escapeSeparator, bool isNested, bool escapeSpaces)
{
- var replace = Expression.Call(exp,
- typeof(string).GetMethod("Replace", new[] { typeof(string), typeof(string) })!,
- Expression.Convert(splitter, typeof(string)),
- Expression.Constant("^", typeof(string))
- );
+ var replaceMethod = typeof(string).GetMethod("Replace", new[] { typeof(string), typeof(string) })!;
+
+ // A nested value sits inside a field of the packet above it, so its own separator is
+ // not the only one it can break: a space in a sub-packet splits the outer field.
+ Expression escaped = Expression.Call(exp, replaceMethod,
+ Expression.Convert(escapeSeparator, typeof(string)),
+ Expression.Constant("^", typeof(string)));
+
+ if (isNested || escapeSpaces)
+ {
+ escaped = Expression.Call(escaped, replaceMethod,
+ Expression.Constant(" ", typeof(string)),
+ Expression.Constant("^", typeof(string)));
+ }
+
+ var replace = escaped;
var nullOrEmpty = Expression.Call(null,
typeof(string).GetMethod("IsNullOrEmpty", new[] { typeof(string) })!,
- Expression.Convert(splitter, typeof(string))
+ Expression.Convert(escapeSeparator, typeof(string))
);
return Expression.Condition(
@@ -99,7 +115,7 @@ private Expression StringSerializer(Expression exp, bool isLastIndex, bool isOpt
ConcatExpression(splitter,
Expression.Condition(nullOrEmpty,
Expression.Convert(exp, typeof(object)),
- Expression.Convert(isLastIndex ? exp : replace, typeof(object))))
+ Expression.Convert(isLastIndex && !escapeSpaces ? exp : replace, typeof(object))))
);
}
@@ -195,7 +211,7 @@ private Expression ListSerializer(Expression injectedPacket, Expression specific
Expression.Convert(isPacketList
? PacketSerializer(injectedPacket, indexAttr, param, subtype!, 0, propertySplitter, "", true)
: PropertySerializer(injectedPacket, indexAttr, subtype!, param, 0,
- Expression.Constant("")), typeof(string)), param)
+ Expression.Constant(""), Expression.Constant("")), typeof(string)), param)
);
var listJoin = Expression.Convert(Expression.Call(
@@ -240,6 +256,12 @@ private Expression PacketSerializer(Expression injectedPacket, PacketIndexAttrib
isOptionalSerie = false;
}
+ var splitter = Expression.Condition(injectedPacket,
+ Expression.Constant("^", typeof(object)),
+ index.SpecialSeparator != null
+ ? Expression.Constant(indexAttr.SpecialSeparator ?? string.Empty, typeof(object))
+ : (Expression)Expression.Convert(propertySplitter, typeof(object)));
+
var exp = Expression.Convert(
PropertySerializer(injectedPacket,
index,
@@ -247,15 +269,13 @@ private Expression PacketSerializer(Expression injectedPacket, PacketIndexAttrib
Expression.Property(specificTypeExpression, property.Name),
maxIndex,
Expression.Condition(injectedPacket, Expression.Constant(string.Empty, typeof(object)),
- Expression.Constant(index.SpecialSeparator, typeof(object))))
+ Expression.Constant(index.SpecialSeparator, typeof(object))),
+ index.SpecialSeparator != null
+ ? Expression.Constant(index.SpecialSeparator, typeof(object))
+ : splitter,
+ isFromList)
, typeof(object));
- var splitter = Expression.Condition(injectedPacket,
- Expression.Constant("^", typeof(object)),
- index.SpecialSeparator != null
- ? Expression.Constant(indexAttr.SpecialSeparator ?? string.Empty, typeof(object))
- : (Expression)Expression.Convert(propertySplitter, typeof(object)));
-
var trimfirst = Expression.Condition(
Expression.Equal(incrementExpr, Expression.Constant(false)),
exp,
@@ -301,7 +321,8 @@ private Expression PacketSerializer(Expression injectedPacket, PacketIndexAttrib
}
private Expression PropertySerializer(Expression injectedPacket, PacketIndexAttribute indexAttr, Type type,
- Expression specificTypeExpression, int maxIndex, Expression propertySplitter)
+ Expression specificTypeExpression, int maxIndex, Expression propertySplitter, Expression escapeSeparator,
+ bool isNested = false)
{
var useCustomSerializer = true;
switch (type)
@@ -328,7 +349,8 @@ private Expression PropertySerializer(Expression injectedPacket, PacketIndexAttr
//handle string
case var t when t == typeof(string):
specificTypeExpression = StringSerializer(specificTypeExpression, indexAttr.Index == maxIndex,
- indexAttr.IsOptional, propertySplitter);
+ indexAttr.IsOptional, propertySplitter, escapeSeparator, isNested,
+ indexAttr.EscapeSpaces);
break;
//IPacket declared type
case var t when typeof(IPacket).IsAssignableFrom(t) && (t != typeof(IPacket)):
diff --git a/src/NosCore.Packets/ServerPackets/Families/GInfoPacket.cs b/src/NosCore.Packets/ServerPackets/Families/GInfoPacket.cs
index cdeea661..bf4e4672 100644
--- a/src/NosCore.Packets/ServerPackets/Families/GInfoPacket.cs
+++ b/src/NosCore.Packets/ServerPackets/Families/GInfoPacket.cs
@@ -66,7 +66,7 @@ public class GInfoPacket : PacketBase
///
/// Should replace ' ' by '^'
///
- [PacketIndex(16)]
+ [PacketIndex(16, EscapeSpaces = true)]
public string? FamilyMessage { get; set; }
}
}
\ No newline at end of file
diff --git a/src/NosCore.Packets/ServerPackets/Miniland/MlInfoBrPacket.cs b/src/NosCore.Packets/ServerPackets/Miniland/MlInfoBrPacket.cs
index 32cf1f35..2090f305 100644
--- a/src/NosCore.Packets/ServerPackets/Miniland/MlInfoBrPacket.cs
+++ b/src/NosCore.Packets/ServerPackets/Miniland/MlInfoBrPacket.cs
@@ -27,7 +27,7 @@ public class MlInfoBrPacket : PacketBase
[PacketIndex(4)]
public byte Unknown2 { get; set; }
- [PacketIndex(5)]
+ [PacketIndex(5, EscapeSpaces = true)]
public string? MinilandMessage { get; set; }
}
}
\ No newline at end of file
diff --git a/src/NosCore.Packets/ServerPackets/Miniland/MlintroPacket.cs b/src/NosCore.Packets/ServerPackets/Miniland/MlintroPacket.cs
index b9df9436..41ac58b3 100644
--- a/src/NosCore.Packets/ServerPackets/Miniland/MlintroPacket.cs
+++ b/src/NosCore.Packets/ServerPackets/Miniland/MlintroPacket.cs
@@ -12,7 +12,7 @@ namespace NosCore.Packets.ServerPackets.Miniland
[PacketHeader("mlintro", Scope.InGame)]
public class MlintroPacket : PacketBase
{
- [PacketIndex(0)]
+ [PacketIndex(0, EscapeSpaces = true)]
public string? Intro { get; set; }
}
}
\ No newline at end of file
diff --git a/test/NosCore.Packets.Tests/StringFieldSeparatorTests.cs b/test/NosCore.Packets.Tests/StringFieldSeparatorTests.cs
new file mode 100644
index 00000000..6cc59bf6
--- /dev/null
+++ b/test/NosCore.Packets.Tests/StringFieldSeparatorTests.cs
@@ -0,0 +1,133 @@
+// __ _ __ __ ___ __ ___ ___
+// | \| |/__\ /' _/ / _//__\| _ \ __|
+// | | ' | \/ |`._`.| \_| \/ | v / _|
+// |_|\__|\__/ |___/ \__/\__/|_|_\___|
+// -----------------------------------
+
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+using NosCore.Packets.Attributes;
+using NosCore.Packets.Enumerations;
+using NosCore.Packets.Interfaces;
+using NosCore.Packets.ServerPackets.Mates;
+using NosCore.Packets.ServerPackets.Groups;
+using NosCore.Packets.ServerPackets.Miniland;
+using NosCore.Packets.ServerPackets.Visibility;
+using System.Collections.Generic;
+using NosCore.Shared.Enumerations;
+
+namespace NosCore.Packets.Tests
+{
+ [PacketHeader("tstsep", Scope.InGame)]
+ public class SeparatorProbePacket : PacketBase
+ {
+ [PacketIndex(0)] public int Before { get; set; }
+ [PacketIndex(1)] public string? Middle { get; set; }
+ [PacketIndex(2)] public int After { get; set; }
+ [PacketIndex(3)] public string? Last { get; set; }
+ }
+
+ [PacketHeader("tstdot", Scope.InGame)]
+ public class DottedProbePacket : PacketBase
+ {
+ [PacketIndex(0)] public int Before { get; set; }
+ [PacketIndex(1, SpecialSeparator = ".")] public string? Dotted { get; set; }
+ [PacketIndex(2)] public int After { get; set; }
+ }
+
+ // A value that contains the field separator used to split its own field in two and shift
+ // every field after it. The escaping existed but was aimed at SpecialSeparator, which an
+ // ordinary property never declares.
+ [TestClass]
+ public class StringFieldSeparatorTests
+ {
+ private static readonly ISerializer Serializer =
+ new Serializer(new[]
+ {
+ typeof(SeparatorProbePacket), typeof(DottedProbePacket),
+ typeof(ScpPacket), typeof(ScnPacket), typeof(InPacket),
+ typeof(PinitPacket), typeof(MlintroPacket), typeof(MlInfoBrPacket)
+ });
+
+ [TestMethod]
+ public void ASpaceInAMiddleFieldIsEscaped()
+ {
+ Assert.AreEqual("tstsep 1 Joyeux^Mouton 2 -",
+ Serializer.Serialize(new SeparatorProbePacket { Before = 1, Middle = "Joyeux Mouton", After = 2 }));
+ }
+
+ [TestMethod]
+ public void TheLastFieldKeepsItsSpaces()
+ {
+ // Nothing follows it, so nothing can shift - and chat lines rely on this.
+ Assert.AreEqual("tstsep 1 - 2 hello there",
+ Serializer.Serialize(new SeparatorProbePacket { Before = 1, After = 2, Last = "hello there" }));
+ }
+
+ [TestMethod]
+ public void AFieldWithItsOwnSeparatorEscapesThatSeparatorAndNotTheSpace()
+ {
+ Assert.AreEqual("tstdot 1.a^b 2",
+ Serializer.Serialize(new DottedProbePacket { Before = 1, Dotted = "a.b", After = 2 }));
+ }
+
+ [TestMethod]
+ public void ASpacedNameNoLongerSplitsTheMatePacket()
+ {
+ var wire = Serializer.Serialize(new ScpPacket
+ {
+ PetId = 1, NpcMonsterVNum = 333, Level = 15, Name = "Joyeux Mouton", IsSummonable = true
+ });
+
+ StringAssert.Contains(wire, "Joyeux^Mouton");
+ Assert.IsFalse(wire.Contains("Joyeux Mouton"));
+ }
+
+ [TestMethod]
+ public void TheSameHoldsForTheOtherPacketsThatCarryAName()
+ {
+ // ScnPacket.Name is index 36 of more, InPacket.Name is index 1 of many - neither is
+ // last, so both are escaped now. ScnPacket has said "Spaces should be replaced by ^"
+ // in a doc comment the whole time without anything enforcing it.
+ var scn = Serializer.Serialize(new ScnPacket
+ {
+ PetId = 1, NpcMonsterVNum = 333, Level = 15, Name = "Joyeux Mouton"
+ });
+ StringAssert.Contains(scn, "Joyeux^Mouton");
+ Assert.IsFalse(scn.Contains("Joyeux Mouton"));
+
+ var inp = Serializer.Serialize(new InPacket
+ {
+ VisualType = VisualType.Npc, Name = "Joyeux Mouton", VNum = "333", VisualId = 1
+ });
+ StringAssert.Contains(inp, "Joyeux^Mouton");
+ Assert.IsFalse(inp.Contains("Joyeux Mouton"));
+ }
+
+ [TestMethod]
+ public void AStringInsideASubPacketEscapesTheOuterSeparatorToo()
+ {
+ // Its own separator is "|", but the sub-packet sits inside a space-separated field,
+ // so an unescaped space here splits the packet above it.
+ Assert.AreEqual("pinit 0 0|0|1|10|Joyeux^Mouton|0|0|0|0|0|0",
+ Serializer.Serialize(new PinitPacket
+ {
+ PinitSubPackets = new List
+ {
+ new() { GroupPosition = 1, Name = "Joyeux Mouton", Level = 10 }
+ }
+ }));
+ }
+
+ [TestMethod]
+ public void AFieldThatOptsInIsEscapedEvenThoughItIsLast()
+ {
+ // Nothing would break without it - the client simply expects these encoded.
+ Assert.AreEqual("mlintro Bienvenue^chez^moi",
+ Serializer.Serialize(new MlintroPacket { Intro = "Bienvenue chez moi" }));
+
+ StringAssert.Contains(
+ Serializer.Serialize(new MlInfoBrPacket { Name = "Bob", MinilandMessage = "hello there" }),
+ "hello^there");
+ }
+ }
+}