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 <noreply@anthropic.com>
This commit is contained in:
BastiaanandClaude Opus 4.8 authored and GitHub committed 2026-09-13 14:53:49 +01:00
1 parent 8239a96509
commit fc73433ffb
2 files changed
+47 -4

No files matched your search

+14 -4
View File
@@ -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]);
});
}
+33
View File
@@ -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 <server>. 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", " "));
});
});
});