From c94084b48f5e9385ba2decd44dd682350ee1f70b Mon Sep 17 00:00:00 2001 From: Kamron Batman <3953314+kamronbatman@users.noreply.github.com> Date: Mon, 22 Jun 2026 18:53:00 -0700 Subject: [PATCH] fix(opl): cap per-property length + chunk OplTextBlock to avoid legacy 2D-client crash The legacy 2D (EA) client copies each OPL property's argument into a fixed ~512-char (1024-byte) buffer; a single property that exceeds it overruns the heap and crashes the client. OplTextBlock previously emitted one entry regardless of length. - ObjectPropertyList.MaxArgumentLength (504 chars, under the ~510 ceiling). - ClampArgument backstop in both InternalAdd paths (span + interpolation): truncates an oversized argument and logs the offending entity/cliloc. - AddChunked(ReadOnlySpan): splits newline-joined text across multiple cycling passthrough properties, breaking only at '\n', so none exceeds the cap. - OplTextBlock.Dispose emits via AddChunked; AddChunked added to IPropertyList. --- .../ObjectPropertyListSpanAddTests.cs | 40 +++++++++++ .../Tests/PropertyList/OplTextBlockTests.cs | 20 ++++++ Projects/Server/PropertyList/IPropertyList.cs | 3 + .../Server/PropertyList/ObjectPropertyList.cs | 72 ++++++++++++++++++- Projects/Server/PropertyList/OplTextBlock.cs | 8 ++- 5 files changed, 140 insertions(+), 3 deletions(-) diff --git a/Projects/Server.Tests/Tests/PropertyList/ObjectPropertyListSpanAddTests.cs b/Projects/Server.Tests/Tests/PropertyList/ObjectPropertyListSpanAddTests.cs index e7b2007d8..408c53386 100644 --- a/Projects/Server.Tests/Tests/PropertyList/ObjectPropertyListSpanAddTests.cs +++ b/Projects/Server.Tests/Tests/PropertyList/ObjectPropertyListSpanAddTests.cs @@ -61,4 +61,44 @@ public class ObjectPropertyListSpanAddTests Assert.Single(entries); Assert.Equal((1070722, "Custom"), entries[0]); } + + [Fact] + public void Add_TruncatesArgumentOverMaxLength() + { + var oversized = new string('x', ObjectPropertyList.MaxArgumentLength + 50); + + var opl = new ObjectPropertyList(null); + opl.Add(1070722, oversized.AsSpan()); + + var entries = Decode(opl); + Assert.Single(entries); + Assert.Equal(ObjectPropertyList.MaxArgumentLength, entries[0].arg.Length); + } + + [Fact] + public void AddChunked_SplitsAtNewlinesSoNoEntryExceedsCap() + { + // 10 lines x 100 chars joined by '\n' (~1009 chars), well over the cap. + var lines = new string[10]; + for (var i = 0; i < lines.Length; i++) + { + lines[i] = new string((char)('a' + i), 100); + } + + var text = string.Join("\n", lines); + + var opl = new ObjectPropertyList(null); + opl.AddChunked(text.AsSpan()); + + var entries = Decode(opl); + Assert.True(entries.Length > 1, "expected multiple chunks"); + foreach (var (_, arg) in entries) + { + Assert.True(arg.Length <= ObjectPropertyList.MaxArgumentLength); + } + + // AddChunked breaks only at '\n' (dropping that '\n'), so rejoining with '\n' is lossless. + var rejoined = string.Join("\n", Array.ConvertAll(entries, e => e.arg)); + Assert.Equal(text, rejoined); + } } diff --git a/Projects/Server.Tests/Tests/PropertyList/OplTextBlockTests.cs b/Projects/Server.Tests/Tests/PropertyList/OplTextBlockTests.cs index c159c0771..71dd163d8 100644 --- a/Projects/Server.Tests/Tests/PropertyList/OplTextBlockTests.cs +++ b/Projects/Server.Tests/Tests/PropertyList/OplTextBlockTests.cs @@ -85,4 +85,24 @@ public class OplTextBlockTests Assert.Equal((1042971, "Luck Bonus: +5%"), Decode(opl)[0]); } + + [Fact] + public void LongContent_SplitsIntoMultipleEntriesUnderCap() + { + var opl = new ObjectPropertyList(null); + using (var block = opl.TextBlock()) + { + for (var i = 0; i < 10; i++) + { + block.Add(new string((char)('a' + i), 100).AsSpan()); + } + } + + var entries = Decode(opl); + Assert.True(entries.Length > 1, "expected the long block to chunk into multiple entries"); + foreach (var (_, arg) in entries) + { + Assert.True(arg.Length <= ObjectPropertyList.MaxArgumentLength); + } + } } diff --git a/Projects/Server/PropertyList/IPropertyList.cs b/Projects/Server/PropertyList/IPropertyList.cs index f66c8cd61..6a873f995 100644 --- a/Projects/Server/PropertyList/IPropertyList.cs +++ b/Projects/Server/PropertyList/IPropertyList.cs @@ -35,6 +35,9 @@ public interface IPropertyList : ISelfInterpolatedStringHandler /** Convenience method for span-based text without allocating a string. */ public void Add(int number, ReadOnlySpan argument); + /** Emits newline-joined text across multiple properties so none exceeds the legacy client's per-property buffer. */ + public void AddChunked(ReadOnlySpan text); + /** Convenience method for $"{value}". */ public void Add(int number, int value); diff --git a/Projects/Server/PropertyList/ObjectPropertyList.cs b/Projects/Server/PropertyList/ObjectPropertyList.cs index ffe6a7315..73288f5e1 100644 --- a/Projects/Server/PropertyList/ObjectPropertyList.cs +++ b/Projects/Server/PropertyList/ObjectPropertyList.cs @@ -20,6 +20,7 @@ using System.Diagnostics; using System.IO; using System.Runtime.CompilerServices; using Server.Buffers; +using Server.Logging; using Server.Network; using Server.Text; @@ -37,6 +38,14 @@ public sealed class ObjectPropertyList : IPropertyList, IDisposable 1114779 // ~1_val~ }; + private static readonly ILogger logger = LogFactory.GetLogger(typeof(ObjectPropertyList)); + + // Max characters for a SINGLE OPL property argument. The legacy 2D client copies each + // property's text into a fixed ~512-char (1024-byte) buffer; exceeding it corrupts the heap + // (smashes an adjacent world object's vtable -> client crash). 504 = multiple of 8, safely + // under the empirically confirmed ~510-char ceiling. For multi-line content use AddChunked(). + public const int MaxArgumentLength = 504; + private int _hash; private int _stringNumbersIndex; private byte[] _buffer; @@ -156,6 +165,65 @@ public sealed class ObjectPropertyList : IPropertyList, IDisposable public void Add(int number, ReadOnlySpan argument) => InternalAdd(number, argument); public OplTextBlock TextBlock() => new(this); + // Emits newline-joined text across as many OPL properties as needed, breaking ONLY at '\n', + // so no single property exceeds MaxArgumentLength characters. Each chunk goes through the + // passthrough-cliloc rotation. Use for variable-length multi-line content instead of one + // Add(joined) call, which would overflow the legacy 2D-client per-property tooltip buffer. + public void AddChunked(ReadOnlySpan text) + { + if (text.IsEmpty) + { + return; + } + + var chunkStart = 0; + var searchFrom = 0; + + while (true) + { + var nl = text[searchFrom..].IndexOf('\n'); + var lineEnd = nl < 0 ? text.Length : searchFrom + nl; + + // If appending this line would push the current chunk past the cap, flush the chunk + // up to the end of the previous line (excluding its '\n') first. + if (lineEnd - chunkStart > MaxArgumentLength && searchFrom > chunkStart) + { + Add(text[chunkStart..(searchFrom - 1)]); + chunkStart = searchFrom; + continue; + } + + if (nl < 0) + { + Add(text[chunkStart..]); // remainder (a single over-cap line is clamped by Add) + return; + } + + searchFrom = lineEnd + 1; + } + } + + // Hard backstop: never let a single property exceed the legacy client's per-property buffer. + // Callers with multi-line content should use AddChunked(); this truncates anything that slips + // through and surfaces the offending entity/cliloc. + private ReadOnlySpan ClampArgument(int number, ReadOnlySpan chars) + { + if (chars.Length <= MaxArgumentLength) + { + return chars; + } + + logger.Warning( + "OPL property on {Entity} (cliloc {Cliloc}) is {Length} chars; truncating to {Max} to avoid legacy 2D-client tooltip-buffer overflow. Use AddChunked for multi-line text.", + Entity, + number, + chars.Length, + MaxArgumentLength + ); + + return chars[..MaxArgumentLength]; + } + private void InternalAdd(int number, ReadOnlySpan chars) { if (number == 0) @@ -163,6 +231,8 @@ public sealed class ObjectPropertyList : IPropertyList, IDisposable return; } + chars = ClampArgument(number, chars); + if (Header == 0) { Header = number; @@ -211,7 +281,7 @@ public sealed class ObjectPropertyList : IPropertyList, IDisposable return; } - var chars = _arrayToReturnToPool.AsSpan(0, _pos); + var chars = ClampArgument(number, _arrayToReturnToPool.AsSpan(0, _pos)); if (Header == 0) { diff --git a/Projects/Server/PropertyList/OplTextBlock.cs b/Projects/Server/PropertyList/OplTextBlock.cs index e420c99fc..296094a21 100644 --- a/Projects/Server/PropertyList/OplTextBlock.cs +++ b/Projects/Server/PropertyList/OplTextBlock.cs @@ -19,7 +19,9 @@ using Server.Text; namespace Server; -// Accumulates '\n'-joined free-text lines and emits ONE cycling passthrough entry on dispose. +// Accumulates '\n'-joined free-text lines and emits them on dispose via AddChunked, which splits +// into as many cycling passthrough entries as needed so no single OPL property exceeds the legacy +// client's per-property buffer (ObjectPropertyList.MaxArgumentLength). // Use with `using var block = list.TextBlock();`. ref struct: single-threaded OPL build only. public ref struct OplTextBlock { @@ -63,7 +65,9 @@ public ref struct OplTextBlock if (_any) { // Strip the trailing '\n' (Length >= 2 whenever _any: content + separator). - _list.Add(_builder.AsSpan(0, _builder.Length - 1)); + // AddChunked splits across multiple properties so a long block never overflows the + // legacy client's per-property tooltip buffer. + _list.AddChunked(_builder.AsSpan(0, _builder.Length - 1)); } _builder.Dispose();