Skip to content

Commit f853482

Browse files
committed
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.
1 parent ae73cbb commit f853482

2 files changed

Lines changed: 39 additions & 14 deletions

File tree

src/DiffEngine.Tests/LinuxOsxProcessTests.cs

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,4 +39,34 @@ public async Task TryParse_singleDigit()
3939
await Assert.That(processCommand.Process).IsEqualTo(309);
4040
await Assert.That(processCommand.Command).IsEqualTo("System/Library/coreauthd -foo");
4141
}
42+
43+
/// <summary>
44+
/// A command with a run of three spaces in it. The removed branch went looking for exactly
45+
/// that and truncated the command to whatever followed it.
46+
/// </summary>
47+
[Test]
48+
public async Task TryParse_commandContainingRunsOfSpaces()
49+
{
50+
var parse = LinuxOsxProcess.TryParse("123 /usr/bin/tool file.txt", out var command);
51+
await Assert.That(parse).IsTrue();
52+
var processCommand = command!.Value;
53+
await Assert.That(processCommand.Process).IsEqualTo(123);
54+
await Assert.That(processCommand.Command).IsEqualTo("/usr/bin/tool file.txt");
55+
}
56+
57+
/// <summary>
58+
/// A PID with more digits than the command has characters. The removed branch sliced by the
59+
/// PID's digit count, which is not an index into this string at all, so this threw
60+
/// ArgumentOutOfRangeException - and did so out of ProcessCleanup's static constructor, which
61+
/// makes it permanent for the process.
62+
/// </summary>
63+
[Test]
64+
public async Task TryParse_longPidShortCommand()
65+
{
66+
var parse = LinuxOsxProcess.TryParse("1234567 /x y", out var command);
67+
await Assert.That(parse).IsTrue();
68+
var processCommand = command!.Value;
69+
await Assert.That(processCommand.Process).IsEqualTo(1234567);
70+
await Assert.That(processCommand.Command).IsEqualTo("/x y");
71+
}
4272
}

src/DiffEngine/Process/LinuxOsxProcess.cs

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -66,20 +66,15 @@ public static bool TryParse(string line, out ProcessCommand? processCommand)
6666
var pidString = trim[..firstSpace];
6767
var pid = int.Parse(pidString.ToString());
6868

69-
var timeAndCommandString = trim[(firstSpace + 1)..];
70-
var multiSpaceIndex = 0;
71-
CharSpan command;
72-
73-
var spaces = new CharSpan([' ',' ',' ']);
74-
if (timeAndCommandString.IndexOf(spaces, StringComparison.InvariantCulture) > 0)
75-
{
76-
multiSpaceIndex = timeAndCommandString[firstSpace..].IndexOf(spaces, StringComparison.InvariantCulture);
77-
command = timeAndCommandString[(multiSpaceIndex + 1)..].Trim();
78-
}
79-
else
80-
{
81-
command = timeAndCommandString[multiSpaceIndex..].Trim();
82-
}
69+
// `ps -o pid,command` has exactly one separator, so everything after the first space
70+
// is the command. There used to be a second branch here looking for a run of three
71+
// spaces, left over from a format that also carried TIME, and it was wrong twice over:
72+
// it sliced by firstSpace, which is the PID's digit count and means nothing in this
73+
// string, and then applied the index it found to the unsliced span. So a command
74+
// containing three spaces was truncated, and a seven digit PID with a short command
75+
// threw ArgumentOutOfRangeException - out of ProcessCleanup's static constructor,
76+
// which makes it permanent for the process
77+
var command = trim[(firstSpace + 1)..].Trim();
8378

8479
processCommand = new(command.ToString(), in pid);
8580
return true;

0 commit comments

Comments
 (0)