From fc73433ffbff4d42d6771cd4166dd1b2fae24e93 Mon Sep 17 00:00:00 2001 From: Bastiaan <17990055+bbassie@users.noreply.github.com> Date: Sun, 13 Sep 2026 15:53:49 +0200 Subject: [PATCH] Stop host crash on malformed action log lines (#1034) _watchServerLogActions dereferenced two regex results with ! and threw on null. The name regex /^([^ ]+)/ returns null when a line classified as an action starts with a space, and the BAN reason regex /\. Reason: (.+)\.$/ returns null for a [BAN] line with no ". Reason: X." tail. The output listener runs synchronously off the stdout LineSplitter, so the throw becomes an uncaughtException and takes the whole host down with every instance on it. Repro: instance with enable_save_patching off and enable_script_commands on; a control client sends /sc print("2000-01-01 00:00:00 [BAN] evil"). The raw print reaches stdout, parses as a dated BAN action, and the reason deref throws. Skip the line when the name regex misses and default the ban reason to empty when the tail is absent. Apply the same name guard to _watchPlayerPromote. The autosave ![1] in server.ts is safe because its filter regex already guarantees the capture. Co-authored-by: Claude Opus 4.8 --- packages/host/src/Instance.ts | 18 ++++++++++++++---- test/host/Instance.js | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 4 deletions(-) diff --git a/packages/host/src/Instance.ts b/packages/host/src/Instance.ts index 3f78c67b..5a9eb35b 100644 --- a/packages/host/src/Instance.ts +++ b/packages/host/src/Instance.ts @@ -306,7 +306,12 @@ export default class Instance extends lib.Link { return; } - let name = /^([^ ]+)/.exec(parsed.message)![1]; + // Guard against lines that classify as actions but lack a name + let nameMatch = /^([^ ]+)/.exec(parsed.message); + if (!nameMatch) { + return; + } + let name = nameMatch[1]; // Detect player leave and join if (parsed.action === "JOIN") { @@ -322,7 +327,9 @@ export default class Instance extends lib.Link { // Detect banned and admin state change, whitelist changes are not logged if (parsed.action === "BAN") { - const reason = /\. Reason: (.+)\.$/.exec(parsed.message)![1]; + // Ban lines without a ". Reason: X." tail default to no reason + const reasonMatch = /\. Reason: (.+)\.$/.exec(parsed.message); + const reason = reasonMatch ? reasonMatch[1] : ""; this._recordUserUpdate(parsed.action, name, reason !== "unspecified" ? reason : ""); } else if (["UNBANNED", "PROMOTE", "DEMOTE"].includes(parsed.action)) { this._recordUserUpdate(parsed.action as "UNBANNED" | "PROMOTE" | "DEMOTE", name); @@ -348,8 +355,11 @@ export default class Instance extends lib.Link { return; } - const name = /^([^ ]+)/.exec(parsed.message)![1]; - this._recordUserUpdate(parsed.action as "PROMOTE" | "DEMOTE", name); + const nameMatch = /^([^ ]+)/.exec(parsed.message); + if (!nameMatch) { + return; + } + this._recordUserUpdate(parsed.action as "PROMOTE" | "DEMOTE", nameMatch[1]); }); } diff --git a/test/host/Instance.js b/test/host/Instance.js index 73e11c1a..e42c02c2 100644 --- a/test/host/Instance.js +++ b/test/host/Instance.js @@ -328,4 +328,37 @@ describe("class Instance", function() { } }); }); + + describe("_watchServerLogActions()", function() { + beforeEach(function() { + instance.config.set("factorio.sync_banlist", "bidirectional"); + instance._watchServerLogActions(); + }); + afterEach(function() { + clearInterval(instance._playerCheckInterval); + }); + function emitAction(action, message) { + instance.server.emit("output", { type: "action", action, message }, ""); + } + it("should record a normal JOIN action", function() { + emitAction("JOIN", "player joined the game"); + assert(instance.playersOnline.has("player"), "player was not added"); + }); + it("should parse the reason of a normal BAN action", function() { + emitAction("BAN", "player was banned by . Reason: griefing."); + assert.deepEqual( + connector.sentMessages[0], + new lib.MessageEvent( + 1, src, addr("allInstances"), "InstanceBanlistUpdateEvent", + new lib.InstanceBanlistUpdateEvent("player", true, "griefing"), + ), + ); + }); + it("should not throw on a BAN action without a reason tail", function() { + assert.doesNotThrow(() => emitAction("BAN", "evil")); + }); + it("should not throw on an action whose message starts with a space", function() { + assert.doesNotThrow(() => emitAction("JOIN", " ")); + }); + }); });