Skip to content

Commit f8120d3

Browse files
wjlafranceclaude
andcommitted
Improve logging for malformed packet diagnosis
The existing exception handling in SocketIOCompleted already catches all exceptions from message handlers and disconnects the client, so the server does not crash on malformed packets. However, the log output was unhelpful (e.g. "ArgumentOutOfRangeException error encountered!" with no packet context). Add try/catch in Invoke() that logs the message ID, size, and hex dump before disconnecting. Fix BinaryReader.ReadByteString() to throw a descriptive GameProtocolViolationException on missing null terminators instead of an opaque ArgumentOutOfRangeException. Fix GetNextNull() to restore stream position on failure. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent b48d77b commit f8120d3

2 files changed

Lines changed: 25 additions & 5 deletions

File tree

src/Atlasd/Battlenet/BinaryReader.cs

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
using System.IO;
1+
using Atlasd.Battlenet.Exceptions;
2+
using System.IO;
23
using System.Text;
34

45
namespace Atlasd.Battlenet
@@ -17,7 +18,7 @@ public long GetNextNull()
1718
{
1819
long lastPosition = BaseStream.Position;
1920

20-
while (BaseStream.CanRead)
21+
while (BaseStream.Position < BaseStream.Length)
2122
{
2223
if (ReadByte() == 0)
2324
{
@@ -27,6 +28,7 @@ public long GetNextNull()
2728
}
2829
}
2930

31+
BaseStream.Position = lastPosition;
3032
return -1;
3133
}
3234
}
@@ -35,7 +37,13 @@ public byte[] ReadByteString()
3537
{
3638
lock (_lock)
3739
{
38-
var size = GetNextNull() - BaseStream.Position;
40+
var nullPos = GetNextNull();
41+
if (nullPos < 0)
42+
{
43+
throw new GameProtocolViolationException(null,
44+
$"Truncated string field at stream position {BaseStream.Position}: missing null terminator");
45+
}
46+
var size = nullPos - BaseStream.Position;
3947
return ReadBytes((int)size)[..^1];
4048
}
4149
}
@@ -46,7 +54,7 @@ public override string ReadString()
4654
{
4755
string str = "";
4856
char chr;
49-
while ((int)(chr = ReadChar()) != 0)
57+
while (BaseStream.Position < BaseStream.Length && (int)(chr = ReadChar()) != 0)
5058
str += chr;
5159
return str;
5260
}

src/Atlasd/Battlenet/ClientState.cs

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,8 +139,20 @@ private void Invoke(SocketAsyncEventArgs e)
139139

140140
while (BattlenetGameFrame.Messages.TryDequeue(out var msg))
141141
{
142-
if (!msg.Invoke(context))
142+
try
143+
{
144+
if (!msg.Invoke(context))
145+
{
146+
Disconnect();
147+
return;
148+
}
149+
}
150+
catch (Exception ex)
143151
{
152+
var hexDump = msg.Buffer != null ? BitConverter.ToString(msg.Buffer) : "(null)";
153+
Logging.WriteLine(Logging.LogLevel.Error, Logging.LogType.Client, RemoteEndPoint,
154+
$"Exception processing {Message.MessageName(msg.Id)} (0x{msg.Id:X2}, {msg.Buffer?.Length ?? 0} bytes): " +
155+
$"{ex.GetType().Name}: {ex.Message}; packet dump: {hexDump}");
144156
Disconnect();
145157
return;
146158
}

0 commit comments

Comments
 (0)