From f85348259b2cf19fc47238283b36093c8bc889fc Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Sat, 22 Aug 2026 13:20:41 +1000 Subject: [PATCH] Delete the three-space branch from the ps parser `ps -o pid,command` has exactly one separator, so everything after the first space is the command. The branch looking for a run of three spaces is left over from a format that also carried TIME, and it was wrong in two ways at once. It sliced timeAndCommandString by firstSpace - the PID's digit count, which is not an index into that string at all - and then applied the index it found to the unsliced span. So a command containing three consecutive spaces was truncated to whatever followed them, and a seven digit PID with a short command indexed past the end and threw ArgumentOutOfRangeException. That throw comes out of ProcessCleanup's static constructor, so it is not one bad line skipped: it is every launch and every kill in the process, permanently. Two tests, both failing before this: a command with a run of spaces in it, and a long PID with a short command. --- src/DiffEngine.Tests/LinuxOsxProcessTests.cs | 30 ++++++++++++++++++++ src/DiffEngine/Process/LinuxOsxProcess.cs | 23 ++++++--------- 2 files changed, 39 insertions(+), 14 deletions(-) diff --git a/src/DiffEngine.Tests/LinuxOsxProcessTests.cs b/src/DiffEngine.Tests/LinuxOsxProcessTests.cs index 59e6eb18..641d7b3a 100644 --- a/src/DiffEngine.Tests/LinuxOsxProcessTests.cs +++ b/src/DiffEngine.Tests/LinuxOsxProcessTests.cs @@ -39,4 +39,34 @@ public async Task TryParse_singleDigit() await Assert.That(processCommand.Process).IsEqualTo(309); await Assert.That(processCommand.Command).IsEqualTo("System/Library/coreauthd -foo"); } + + /// + /// A command with a run of three spaces in it. The removed branch went looking for exactly + /// that and truncated the command to whatever followed it. + /// + [Test] + public async Task TryParse_commandContainingRunsOfSpaces() + { + var parse = LinuxOsxProcess.TryParse("123 /usr/bin/tool file.txt", out var command); + await Assert.That(parse).IsTrue(); + var processCommand = command!.Value; + await Assert.That(processCommand.Process).IsEqualTo(123); + await Assert.That(processCommand.Command).IsEqualTo("/usr/bin/tool file.txt"); + } + + /// + /// A PID with more digits than the command has characters. The removed branch sliced by the + /// PID's digit count, which is not an index into this string at all, so this threw + /// ArgumentOutOfRangeException - and did so out of ProcessCleanup's static constructor, which + /// makes it permanent for the process. + /// + [Test] + public async Task TryParse_longPidShortCommand() + { + var parse = LinuxOsxProcess.TryParse("1234567 /x y", out var command); + await Assert.That(parse).IsTrue(); + var processCommand = command!.Value; + await Assert.That(processCommand.Process).IsEqualTo(1234567); + await Assert.That(processCommand.Command).IsEqualTo("/x y"); + } } diff --git a/src/DiffEngine/Process/LinuxOsxProcess.cs b/src/DiffEngine/Process/LinuxOsxProcess.cs index ba0a3c4f..b953b88c 100644 --- a/src/DiffEngine/Process/LinuxOsxProcess.cs +++ b/src/DiffEngine/Process/LinuxOsxProcess.cs @@ -66,20 +66,15 @@ public static bool TryParse(string line, out ProcessCommand? processCommand) var pidString = trim[..firstSpace]; var pid = int.Parse(pidString.ToString()); - var timeAndCommandString = trim[(firstSpace + 1)..]; - var multiSpaceIndex = 0; - CharSpan command; - - var spaces = new CharSpan([' ',' ',' ']); - if (timeAndCommandString.IndexOf(spaces, StringComparison.InvariantCulture) > 0) - { - multiSpaceIndex = timeAndCommandString[firstSpace..].IndexOf(spaces, StringComparison.InvariantCulture); - command = timeAndCommandString[(multiSpaceIndex + 1)..].Trim(); - } - else - { - command = timeAndCommandString[multiSpaceIndex..].Trim(); - } + // `ps -o pid,command` has exactly one separator, so everything after the first space + // is the command. There used to be a second branch here looking for a run of three + // spaces, left over from a format that also carried TIME, and it was wrong twice over: + // it sliced by firstSpace, which is the PID's digit count and means nothing in this + // string, and then applied the index it found to the unsliced span. So a command + // containing three spaces was truncated, and a seven digit PID with a short command + // threw ArgumentOutOfRangeException - out of ProcessCleanup's static constructor, + // which makes it permanent for the process + var command = trim[(firstSpace + 1)..].Trim(); processCommand = new(command.ToString(), in pid); return true;