fix: Warn when sending empty gumps (#2563)

### Summary

Generates a console warning when users receive an empty gump. This will help prevent client side leaks.
This commit is contained in:
Guflly 2026-08-08 00:55:12 -07:00 committed by GitHub
parent b2c59191bd
commit 64e6fe5da8
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
12 changed files with 114 additions and 0 deletions

View file

@ -6,6 +6,8 @@ public class DynamicTestGump : DynamicGump
{ {
private readonly string _petName; private readonly string _petName;
public bool HasVisualElementsForTest => HasVisualElements;
public DynamicTestGump(string petName) : base(50, 50) public DynamicTestGump(string petName) : base(50, 50)
{ {
_petName = petName; _petName = petName;

View file

@ -0,0 +1,40 @@
using Server.Gumps;
namespace Server.Tests.Gumps;
public sealed class EmptyLegacyTestGump : Gump
{
public bool HasVisualElementsForTest => HasVisualElements;
public EmptyLegacyTestGump() : base(0, 0)
{
}
}
public sealed class EmptyDynamicTestGump : DynamicGump
{
public bool HasVisualElementsForTest => HasVisualElements;
public EmptyDynamicTestGump() : base(0, 0)
{
}
protected override void BuildLayout(ref DynamicGumpBuilder builder)
{
builder.AddPage();
}
}
public sealed class EmptyStaticTestGump : StaticGump<EmptyStaticTestGump>
{
public bool HasVisualElementsForTest => HasVisualElements;
public EmptyStaticTestGump() : base(0, 0)
{
}
protected override void BuildLayout(ref StaticGumpBuilder builder)
{
builder.SetNoClose();
}
}

View file

@ -4,6 +4,8 @@ namespace Server.Tests.Gumps;
public sealed class LegacyTestGump : Gump public sealed class LegacyTestGump : Gump
{ {
public bool HasVisualElementsForTest => HasVisualElements;
public LegacyTestGump(string petName) : base(50, 50) public LegacyTestGump(string petName) : base(50, 50)
{ {
Serial = (Serial)0x123; Serial = (Serial)0x123;

View file

@ -4,6 +4,8 @@ namespace Server.Tests.Gumps;
public class StaticTestGump : StaticGump<StaticTestGump> public class StaticTestGump : StaticGump<StaticTestGump>
{ {
public bool HasVisualElementsForTest => HasVisualElements;
public StaticTestGump() : base(50, 50) public StaticTestGump() : base(50, 50)
{ {
Serial = (Serial)0x123; Serial = (Serial)0x123;

View file

@ -73,6 +73,32 @@ public class TestLayoutGumps
AssertThat.Equal(writer.Span, packet); AssertThat.Equal(writer.Span, packet);
} }
[Fact]
public void TestEmptyGumpsHaveNoVisualElements()
{
Assert.False(Compile(new EmptyLegacyTestGump()).HasVisualElementsForTest);
Assert.False(Compile(new EmptyDynamicTestGump()).HasVisualElementsForTest);
Assert.False(Compile(new EmptyStaticTestGump()).HasVisualElementsForTest);
Assert.False(Compile(new EmptyStaticTestGump()).HasVisualElementsForTest);
}
[Fact]
public void TestVisibleGumpsHaveVisualElements()
{
Assert.True(Compile(new LegacyTestGump("Test")).HasVisualElementsForTest);
Assert.True(Compile(new DynamicTestGump("Test")).HasVisualElementsForTest);
Assert.True(Compile(new StaticTestGump()).HasVisualElementsForTest);
Assert.True(Compile(new StaticTestGump()).HasVisualElementsForTest);
}
private static T Compile<T>(T gump) where T : BaseGump
{
var buffer = GC.AllocateUninitializedArray<byte>(512);
var writer = new SpanWriter(buffer);
gump.Compile(ref writer);
return gump;
}
private static void InternalTestStaticGump<T>(ReadOnlySpan<byte> expectedLayout, StaticGump<T> staticGump, string[] strings) private static void InternalTestStaticGump<T>(ReadOnlySpan<byte> expectedLayout, StaticGump<T> staticGump, string[] strings)
where T : StaticGump<T> where T : StaticGump<T>
{ {

View file

@ -13,6 +13,7 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>. * * along with this program. If not, see <http://www.gnu.org/licenses/>. *
*************************************************************************/ *************************************************************************/
using Server.Logging;
using Server.Network; using Server.Network;
using System; using System;
using System.Buffers; using System.Buffers;
@ -23,10 +24,12 @@ namespace Server.Gumps;
public abstract class BaseGump public abstract class BaseGump
{ {
private static readonly byte[] _packetBuffer = GC.AllocateUninitializedArray<byte>(0x10000); private static readonly byte[] _packetBuffer = GC.AllocateUninitializedArray<byte>(0x10000);
private static readonly ILogger _logger = LogFactory.GetLogger(typeof(BaseGump));
private static Serial nextSerial = (Serial)1; private static Serial nextSerial = (Serial)1;
public int TypeID { get; protected set; } public int TypeID { get; protected set; }
public Serial Serial { get; protected set; } public Serial Serial { get; protected set; }
protected bool HasVisualElements { get; set; }
public abstract int Switches { get; } public abstract int Switches { get; }
public abstract int TextEntries { get; } public abstract int TextEntries { get; }
@ -56,6 +59,11 @@ public abstract class BaseGump
var writer = new SpanWriter(_packetBuffer); var writer = new SpanWriter(_packetBuffer);
Compile(ref writer); Compile(ref writer);
if (!HasVisualElements)
{
_logger.Warning("Sending empty gump {GumpType}", GetType().FullName);
}
ns.Send(writer.Span); ns.Send(writer.Span);
writer.Dispose(); writer.Dispose();

View file

@ -47,6 +47,7 @@ public abstract class DynamicGump : BaseGump
BuildLayout(ref gumpBuilder); BuildLayout(ref gumpBuilder);
gumpBuilder.FinalizeLayout(); gumpBuilder.FinalizeLayout();
HasVisualElements = gumpBuilder.HasVisualElements;
_switches = gumpBuilder.Switches; _switches = gumpBuilder.Switches;
_textEntries = gumpBuilder.TextEntries; _textEntries = gumpBuilder.TextEntries;

View file

@ -36,6 +36,7 @@ public ref struct DynamicGumpBuilder
public int Switches => _gumpBuilder._switches; public int Switches => _gumpBuilder._switches;
public int TextEntries => _gumpBuilder._textEntries; public int TextEntries => _gumpBuilder._textEntries;
internal bool HasVisualElements => _gumpBuilder._hasVisualElements;
[MethodImpl(MethodImplOptions.AggressiveInlining)] [MethodImpl(MethodImplOptions.AggressiveInlining)]
public DynamicGumpBuilder() public DynamicGumpBuilder()

View file

@ -30,6 +30,7 @@ public ref struct GumpLayoutBuilder
internal GumpFlags _flags; internal GumpFlags _flags;
internal int _switches; internal int _switches;
internal int _textEntries; internal int _textEntries;
internal bool _hasVisualElements;
internal Span<byte> LayoutData => _layoutBuffer.AsSpan(0, _bytesWritten); internal Span<byte> LayoutData => _layoutBuffer.AsSpan(0, _bytesWritten);
@ -95,6 +96,7 @@ public ref struct GumpLayoutBuilder
public void AddBackground(int x, int y, int width, int height, int gumpId) public void AddBackground(int x, int y, int width, int height, int gumpId)
{ {
_hasVisualElements = true;
GrowIfNeeded(9 + 9 + 45); GrowIfNeeded(9 + 9 + 45);
WriteStart("resizepic"u8); WriteStart("resizepic"u8);
WriteValue(x); WriteValue(x);
@ -109,6 +111,7 @@ public ref struct GumpLayoutBuilder
int x, int y, int normalId, int pressedId, int buttonId, GumpButtonType type = GumpButtonType.Reply, int param = 0 int x, int y, int normalId, int pressedId, int buttonId, GumpButtonType type = GumpButtonType.Reply, int param = 0
) )
{ {
_hasVisualElements = true;
GrowIfNeeded(11 + 6 + 54 + 2); GrowIfNeeded(11 + 6 + 54 + 2);
WriteStart("button"u8); WriteStart("button"u8);
WriteValue(x); WriteValue(x);
@ -123,6 +126,7 @@ public ref struct GumpLayoutBuilder
public void AddCheckbox(int x, int y, int inactiveId, int activeId, bool selected, int switchId) public void AddCheckbox(int x, int y, int inactiveId, int activeId, bool selected, int switchId)
{ {
_hasVisualElements = true;
GrowIfNeeded(10 + 8 + 45 + 2); GrowIfNeeded(10 + 8 + 45 + 2);
WriteStart("checkbox"u8); WriteStart("checkbox"u8);
WriteValue(x); WriteValue(x);
@ -154,6 +158,7 @@ public ref struct GumpLayoutBuilder
public int AddHtmlPlaceholder(int x, int y, int width, int height, public int AddHtmlPlaceholder(int x, int y, int width, int height,
bool background = false, bool scrollbar = false) bool background = false, bool scrollbar = false)
{ {
_hasVisualElements = true;
GrowIfNeeded(11 + 8 + 36 + 10); GrowIfNeeded(11 + 8 + 36 + 10);
WriteStart("htmlgump"u8); WriteStart("htmlgump"u8);
WriteValue(x); WriteValue(x);
@ -172,6 +177,7 @@ public ref struct GumpLayoutBuilder
public void AddHtml(int x, int y, int width, int height, int text, public void AddHtml(int x, int y, int width, int height, int text,
bool background = false, bool scrollbar = false) bool background = false, bool scrollbar = false)
{ {
_hasVisualElements = true;
GrowIfNeeded(11 + 8 + 45 + 4); GrowIfNeeded(11 + 8 + 45 + 4);
WriteStart("htmlgump"u8); WriteStart("htmlgump"u8);
WriteValue(x); WriteValue(x);
@ -188,6 +194,7 @@ public ref struct GumpLayoutBuilder
int x, int y, int width, int height, int number, bool background = false, bool scrollbar = false int x, int y, int width, int height, int number, bool background = false, bool scrollbar = false
) )
{ {
_hasVisualElements = true;
GrowIfNeeded(11 + 11 + 45 + 4); GrowIfNeeded(11 + 11 + 45 + 4);
WriteStart("xmfhtmlgump"u8); WriteStart("xmfhtmlgump"u8);
WriteValue(x); WriteValue(x);
@ -204,6 +211,7 @@ public ref struct GumpLayoutBuilder
int x, int y, int width, int height, int number, int color, bool background = false, bool scrollbar = false int x, int y, int width, int height, int number, int color, bool background = false, bool scrollbar = false
) )
{ {
_hasVisualElements = true;
GrowIfNeeded(12 + 16 + 45 + 5 + 4); GrowIfNeeded(12 + 16 + 45 + 5 + 4);
WriteStart("xmfhtmlgumpcolor"u8); WriteStart("xmfhtmlgumpcolor"u8);
WriteValue(x); WriteValue(x);
@ -220,6 +228,7 @@ public ref struct GumpLayoutBuilder
public void AddHtmlLocalized(int x, int y, int width, int height, int number, ReadOnlySpan<char> args, int color, public void AddHtmlLocalized(int x, int y, int width, int height, int number, ReadOnlySpan<char> args, int color,
bool background = false, bool scrollbar = false) bool background = false, bool scrollbar = false)
{ {
_hasVisualElements = true;
GrowIfNeeded(12 + 10 + 45 + 5 + 4 + (args.Length > 0 ? 3 + args.Length : 0)); GrowIfNeeded(12 + 10 + 45 + 5 + 4 + (args.Length > 0 ? 3 + args.Length : 0));
WriteStart("xmfhtmltok"u8); WriteStart("xmfhtmltok"u8);
WriteValue(x); WriteValue(x);
@ -254,6 +263,7 @@ public ref struct GumpLayoutBuilder
public void AddImage(int x, int y, int gumpId, int hue = 0, ReadOnlySpan<char> cls = default) public void AddImage(int x, int y, int gumpId, int hue = 0, ReadOnlySpan<char> cls = default)
{ {
_hasVisualElements = true;
GrowIfNeeded(7 + 7 + 36 + (hue != 0 ? 14 : 0) + (cls.Length > 0 ? 7 + cls.Length : 0)); GrowIfNeeded(7 + 7 + 36 + (hue != 0 ? 14 : 0) + (cls.Length > 0 ? 7 + cls.Length : 0));
WriteStart("gumppic"u8); WriteStart("gumppic"u8);
WriteValue(x); WriteValue(x);
@ -294,6 +304,7 @@ public ref struct GumpLayoutBuilder
public void AddImageTiledButton(int x, int y, int normalId, int pressedId, int buttonId, GumpButtonType type, int param, public void AddImageTiledButton(int x, int y, int normalId, int pressedId, int buttonId, GumpButtonType type, int param,
int itemId, int hue, int width, int height, int localizedTooltip = -1) int itemId, int hue, int width, int height, int localizedTooltip = -1)
{ {
_hasVisualElements = true;
GrowIfNeeded(15 + 13 + 90 + 2); GrowIfNeeded(15 + 13 + 90 + 2);
WriteStart("buttontileart"u8); WriteStart("buttontileart"u8);
WriteValue(x); WriteValue(x);
@ -319,6 +330,7 @@ public ref struct GumpLayoutBuilder
public void AddImageTiled(int x, int y, int width, int height, int gumpId) public void AddImageTiled(int x, int y, int width, int height, int gumpId)
{ {
_hasVisualElements = true;
GrowIfNeeded(9 + 12 + 45); GrowIfNeeded(9 + 12 + 45);
WriteStart("gumppictiled"u8); WriteStart("gumppictiled"u8);
WriteValue(x); WriteValue(x);
@ -331,6 +343,7 @@ public ref struct GumpLayoutBuilder
public void AddItem(int x, int y, int itemId, int hue = 0) public void AddItem(int x, int y, int itemId, int hue = 0)
{ {
_hasVisualElements = true;
GrowIfNeeded(7 + 36 + (hue != 0 ? 20 : 7)); GrowIfNeeded(7 + 36 + (hue != 0 ? 20 : 7));
WriteStart(hue == 0 ? "tilepic"u8 : "tilepichue"u8); WriteStart(hue == 0 ? "tilepic"u8 : "tilepichue"u8);
WriteValue(x); WriteValue(x);
@ -355,6 +368,7 @@ public ref struct GumpLayoutBuilder
public int AddLabelPlaceholder(int x, int y, int hue) public int AddLabelPlaceholder(int x, int y, int hue)
{ {
_hasVisualElements = true;
GrowIfNeeded(8 + 4 + 27 + 6); GrowIfNeeded(8 + 4 + 27 + 6);
WriteStart("text"u8); WriteStart("text"u8);
WriteValue(x); WriteValue(x);
@ -368,6 +382,7 @@ public ref struct GumpLayoutBuilder
public void AddLabel(int x, int y, int hue, int text) public void AddLabel(int x, int y, int hue, int text)
{ {
_hasVisualElements = true;
GrowIfNeeded(8 + 4 + 36 + 6); GrowIfNeeded(8 + 4 + 36 + 6);
WriteStart("text"u8); WriteStart("text"u8);
WriteValue(x); WriteValue(x);
@ -379,6 +394,7 @@ public ref struct GumpLayoutBuilder
public int AddLabelCroppedPlaceholder(int x, int y, int width, int height, int hue) public int AddLabelCroppedPlaceholder(int x, int y, int width, int height, int hue)
{ {
_hasVisualElements = true;
GrowIfNeeded(10 + 11 + 45 + 6); GrowIfNeeded(10 + 11 + 45 + 6);
WriteStart("croppedtext"u8); WriteStart("croppedtext"u8);
WriteValue(x); WriteValue(x);
@ -394,6 +410,7 @@ public ref struct GumpLayoutBuilder
public void AddLabelCropped(int x, int y, int width, int height, int hue, int text) public void AddLabelCropped(int x, int y, int width, int height, int hue, int text)
{ {
_hasVisualElements = true;
GrowIfNeeded(10 + 11 + 54 + 6); GrowIfNeeded(10 + 11 + 54 + 6);
WriteStart("croppedtext"u8); WriteStart("croppedtext"u8);
WriteValue(x); WriteValue(x);
@ -430,6 +447,7 @@ public ref struct GumpLayoutBuilder
public void AddRadio(int x, int y, int inactiveId, int activeId, bool selected, int switchId) public void AddRadio(int x, int y, int inactiveId, int activeId, bool selected, int switchId)
{ {
_hasVisualElements = true;
GrowIfNeeded(10 + 5 + 45 + 2); GrowIfNeeded(10 + 5 + 45 + 2);
WriteStart("radio"u8); WriteStart("radio"u8);
WriteValue(x); WriteValue(x);
@ -445,6 +463,7 @@ public ref struct GumpLayoutBuilder
public void AddSpriteImage(int x, int y, int gumpId, int width, int height, int sx, int sy) public void AddSpriteImage(int x, int y, int gumpId, int width, int height, int sx, int sy)
{ {
_hasVisualElements = true;
GrowIfNeeded(11 + 8 + 63); GrowIfNeeded(11 + 8 + 63);
WriteStart("picinpic"u8); WriteStart("picinpic"u8);
WriteValue(x); WriteValue(x);
@ -461,6 +480,7 @@ public ref struct GumpLayoutBuilder
int x, int y, int width, int height, int hue, int entryId int x, int y, int width, int height, int hue, int entryId
) )
{ {
_hasVisualElements = true;
GrowIfNeeded(11 + 9 + 54 + 6); GrowIfNeeded(11 + 9 + 54 + 6);
WriteStart("textentry"u8); WriteStart("textentry"u8);
WriteValue(x); WriteValue(x);
@ -481,6 +501,7 @@ public ref struct GumpLayoutBuilder
int x, int y, int width, int height, int hue, int entryId, int initialText int x, int y, int width, int height, int hue, int entryId, int initialText
) )
{ {
_hasVisualElements = true;
GrowIfNeeded(11 + 9 + 63 + 6); GrowIfNeeded(11 + 9 + 63 + 6);
WriteStart("textentry"u8); WriteStart("textentry"u8);
WriteValue(x); WriteValue(x);
@ -499,6 +520,7 @@ public ref struct GumpLayoutBuilder
int x, int y, int width, int height, int hue, int entryId, int size = 0 int x, int y, int width, int height, int hue, int entryId, int size = 0
) )
{ {
_hasVisualElements = true;
GrowIfNeeded(12 + 16 + 63 + 6); GrowIfNeeded(12 + 16 + 63 + 6);
WriteStart("textentrylimited"u8); WriteStart("textentrylimited"u8);
WriteValue(x); WriteValue(x);
@ -520,6 +542,7 @@ public ref struct GumpLayoutBuilder
int x, int y, int width, int height, int hue, int entryId, int initialText, int size = 0 int x, int y, int width, int height, int hue, int entryId, int initialText, int size = 0
) )
{ {
_hasVisualElements = true;
GrowIfNeeded(12 + 16 + 72); GrowIfNeeded(12 + 16 + 72);
WriteStart("textentrylimited"u8); WriteStart("textentrylimited"u8);
WriteValue(x); WriteValue(x);

View file

@ -249,6 +249,7 @@ public class Gump : BaseGump
{ {
_textEntries = 0; _textEntries = 0;
_switches = 0; _switches = 0;
HasVisualElements = false;
var layoutWriter = new SpanWriter(_layoutBuffer); var layoutWriter = new SpanWriter(_layoutBuffer);
@ -274,6 +275,7 @@ public class Gump : BaseGump
foreach (var entry in Entries) foreach (var entry in Entries)
{ {
HasVisualElements |= IsVisualEntry(entry);
entry.AppendTo(ref layoutWriter, _stringsList, ref _textEntries, ref _switches); entry.AppendTo(ref layoutWriter, _stringsList, ref _textEntries, ref _switches);
} }
@ -311,6 +313,9 @@ public class Gump : BaseGump
} }
} }
private static bool IsVisualEntry(GumpEntry entry) =>
entry is not (GumpAlphaRegion or GumpECHandleInput or GumpGroup or GumpItemProperty or GumpMasterGump or GumpPage or GumpTooltip);
protected void Reset() protected void Reset()
{ {
_switches = 0; _switches = 0;

View file

@ -30,6 +30,7 @@ public abstract class StaticGump<TSelf> : BaseGump where TSelf : StaticGump<TSel
private static byte[] _compressedStringsData; private static byte[] _compressedStringsData;
private static bool _hasDynamicStrings; private static bool _hasDynamicStrings;
private static bool _cachedHasVisualElements;
private static int _staticStringsCount; private static int _staticStringsCount;
private static byte[] _staticStrings; private static byte[] _staticStrings;
@ -72,6 +73,7 @@ public abstract class StaticGump<TSelf> : BaseGump where TSelf : StaticGump<TSel
if (Cached && _compressedLayoutData != null) if (Cached && _compressedLayoutData != null)
{ {
HasVisualElements = _cachedHasVisualElements;
writer.Write(_compressedLayoutData); writer.Write(_compressedLayoutData);
if (_compressedStringsData != null) if (_compressedStringsData != null)
@ -116,6 +118,7 @@ public abstract class StaticGump<TSelf> : BaseGump where TSelf : StaticGump<TSel
BuildLayout(ref gumpBuilder); BuildLayout(ref gumpBuilder);
gumpBuilder.FinalizeLayout(); gumpBuilder.FinalizeLayout();
HasVisualElements = _cachedHasVisualElements = gumpBuilder.HasVisualElements;
_switches = gumpBuilder.Switches; _switches = gumpBuilder.Switches;
_textEntries = gumpBuilder.TextEntries; _textEntries = gumpBuilder.TextEntries;
_staticStringsCount = gumpBuilder._stringsCount; _staticStringsCount = gumpBuilder._stringsCount;

View file

@ -43,6 +43,7 @@ public ref struct StaticGumpBuilder
public int Switches => _gumpBuilder._switches; public int Switches => _gumpBuilder._switches;
public int TextEntries => _gumpBuilder._textEntries; public int TextEntries => _gumpBuilder._textEntries;
internal bool HasVisualElements => _gumpBuilder._hasVisualElements;
[MethodImpl(MethodImplOptions.AggressiveInlining)] [MethodImpl(MethodImplOptions.AggressiveInlining)]
public StaticGumpBuilder() public StaticGumpBuilder()