From 05ea0db806c50a02f45ac8e9a2cbb8ad55dc72d9 Mon Sep 17 00:00:00 2001 From: simoleo89 Date: Tue, 19 May 2026 21:05:36 +0200 Subject: [PATCH] fix(parser): drop unsafe borderId read inside the RoomUnitParser per-user loop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Infostand Borders merge (origin/Dev 4b7d04d, upstream commit) added user.borderId = (wrapper.bytesAvailable ? wrapper.readInt() : 0); inside the per-user loop in RoomUnitParser (the parser for the RoomUsersComposer packet — header 3920 — which ships the full roster on room enter). The guard is unsafe inside a loop: `bytesAvailable` is a boolean meaning "any bytes left in the WHOLE packet?", not "any bytes left in THIS user record". For every user except the last one, `bytesAvailable === true` because the NEXT user's bytes still follow, so the parser reads an int and steals 4 bytes from the next user — cascade corruption of the entire roster. Symptom in production: users don't see each other on first room sight. The roster arrives, the parser sfasa, RoomEngine drops the malformed records. Fix: stop reading borderId inside the loop. The per-user border id is shipped separately via RoomUnitInfoParser (single-user packet, no loop), where the bytesAvailable guard is safe. The roster packet's last-tail extension story stays clean for any future trailing block the same way other parsers do — but only when the guard is the LAST read in the packet, not a per-record one. This also makes the renderer wire-compatible with both old emulators (no borderId at all) and the new Arcturus version that ships borderId in RoomUsersComposer — the latter just has 4 extra trailing bytes per user that the parser ignores. A follow-up change on Arcturus' RoomUsersComposer can drop the borderId append, or keep it and the client simply doesn't read it from the roster (which is fine — the infostand re-fetch via RoomUnitInfoParser gives the authoritative border). mvn-equivalent: yarn compile:fast clean, vitest 138/138. --- .../src/messages/parser/room/unit/RoomUnitParser.ts | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/packages/communication/src/messages/parser/room/unit/RoomUnitParser.ts b/packages/communication/src/messages/parser/room/unit/RoomUnitParser.ts index de703ae..8ac25a6 100644 --- a/packages/communication/src/messages/parser/room/unit/RoomUnitParser.ts +++ b/packages/communication/src/messages/parser/room/unit/RoomUnitParser.ts @@ -146,7 +146,16 @@ export class RoomUnitParser implements IMessageParser user.roomEntryMethod = wrapper.readString(); user.roomEntryTeleportId = wrapper.readInt(); - user.borderId = (wrapper.bytesAvailable ? wrapper.readInt() : 0); + // NOTE: do NOT read borderId here. `wrapper.bytesAvailable` + // is a boolean meaning "any bytes left in the whole packet?", + // not "any bytes left in THIS user". For users that aren't + // the last in the roster, bytesAvailable === true because + // the NEXT user's bytes follow — reading an int would steal + // 4 bytes from the next user and cascade-corrupt the entire + // roster (users not seeing each other on first sight). The + // border id for an individual user arrives via + // RoomUnitInfoParser (single-user packet), where the + // bytesAvailable guard is safe because there is no loop. i++; }