Merge pull request #4489 from D4N14L/user/danade/FixProcessList

[node-core-library] Fix parsing of process name on MacOS

Author: D4N14L

common/changes/@rushstack/node-core-library/user-danade-FixProcessList_2024-01-23-18-56.json

@@ -0,0 +1,10 @@
+{
+  "changes": [
+    {
+      "packageName": "@rushstack/node-core-library",
+      "comment": "Fix Executable.getProcessInfoBy* methods truncating the process name on MacOS",
+      "type": "patch"
+    }
+  ],
+  "packageName": "@rushstack/node-core-library"
+}

libraries/node-core-library/src/Executable.ts

@@ -1,8 +1,8 @@
 // Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license.
 // See LICENSE in the project root for license information.
 
-import * as child_process from 'child_process';
 import * as os from 'os';
+import * as child_process from 'child_process';
 import * as path from 'path';
 import { EnvironmentMap } from './EnvironmentMap';
 
@@ -11,6 +11,8 @@ import { PosixModeBits } from './PosixModeBits';
 import { Text } from './Text';
 import { InternalError } from './InternalError';
 
+const OS_PLATFORM: NodeJS.Platform = os.platform();
+
 /**
  * Typings for one of the streams inside IExecutableSpawnSyncOptions.stdio.
  * @public
@@ -221,29 +223,33 @@ export interface IProcessInfo {
 }
 
 export async function parseProcessListOutputAsync(
-  stream: NodeJS.ReadableStream
+  stream: NodeJS.ReadableStream,
+  platform: NodeJS.Platform = OS_PLATFORM
 ): Promise<Map<number, IProcessInfo>> {
   const processInfoById: Map<number, IProcessInfo> = new Map<number, IProcessInfo>();
   let seenHeaders: boolean = false;
   for await (const line of Text.readLinesFromIterableAsync(stream, { ignoreEmptyLines: true })) {
     if (!seenHeaders) {
       seenHeaders = true;
     } else {
-      parseProcessInfoEntry(line, processInfoById);
+      parseProcessInfoEntry(line, processInfoById, platform);
     }
   }
   return processInfoById;
 }
 
-// eslint-disable-next-line @rushstack/no-new-null
-export function parseProcessListOutput(output: Iterable<string | null>): Map<number, IProcessInfo> {
+export function parseProcessListOutput(
+  // eslint-disable-next-line @rushstack/no-new-null
+  output: Iterable<string | null>,
+  platform: NodeJS.Platform = OS_PLATFORM
+): Map<number, IProcessInfo> {
   const processInfoById: Map<number, IProcessInfo> = new Map<number, IProcessInfo>();
   let seenHeaders: boolean = false;
   for (const line of Text.readLinesFromIterable(output, { ignoreEmptyLines: true })) {
     if (!seenHeaders) {
       seenHeaders = true;
     } else {
-      parseProcessInfoEntry(line, processInfoById);
+      parseProcessInfoEntry(line, processInfoById, platform);
     }
   }
   return processInfoById;
@@ -253,18 +259,28 @@ export function parseProcessListOutput(output: Iterable<string | null>): Map<num
 // Name             ParentProcessId   ProcessId
 // process name     1234              5678
 // unix format:
-// COMMAND             PPID     PID
-// process name       51234   56784
+//  PPID     PID   COMMAND
+// 51234   56784   process name
 const NAME_GROUP: 'name' = 'name';
 const PROCESS_ID_GROUP: 'pid' = 'pid';
 const PARENT_PROCESS_ID_GROUP: 'ppid' = 'ppid';
 // eslint-disable-next-line @rushstack/security/no-unsafe-regexp
-const PROCESS_LIST_ENTRY_REGEX: RegExp = new RegExp(
+const PROCESS_LIST_ENTRY_REGEX_WIN32: RegExp = new RegExp(
   `^(?<${NAME_GROUP}>.+?)\\s+(?<${PARENT_PROCESS_ID_GROUP}>\\d+)\\s+(?<${PROCESS_ID_GROUP}>\\d+)\\s*$`
 );
+// eslint-disable-next-line @rushstack/security/no-unsafe-regexp
+const PROCESS_LIST_ENTRY_REGEX_UNIX: RegExp = new RegExp(
+  `^\\s*(?<${PARENT_PROCESS_ID_GROUP}>\\d+)\\s+(?<${PROCESS_ID_GROUP}>\\d+)\\s+(?<${NAME_GROUP}>.+?)\\s*$`
+);
 
-function parseProcessInfoEntry(line: string, existingProcessInfoById: Map<number, IProcessInfo>): void {
-  const match: RegExpMatchArray | null = line.match(PROCESS_LIST_ENTRY_REGEX);
+function parseProcessInfoEntry(
+  line: string,
+  existingProcessInfoById: Map<number, IProcessInfo>,
+  platform: NodeJS.Platform
+): void {
+  const processListEntryRegex: RegExp =
+    platform === 'win32' ? PROCESS_LIST_ENTRY_REGEX_WIN32 : PROCESS_LIST_ENTRY_REGEX_UNIX;
+  const match: RegExpMatchArray | null = line.match(processListEntryRegex);
   if (!match?.groups) {
     throw new InternalError(`Invalid process list entry: ${line}`);
   }
@@ -326,8 +342,6 @@ function convertToProcessInfoByNameMap(
   return processInfoByNameMap;
 }
 
-const OS_PLATFORM: NodeJS.Platform = os.platform();
-
 function getProcessListProcessOptions(): ICommandLineOptions {
   let command: string;
   let args: string[];
@@ -338,10 +352,12 @@ function getProcessListProcessOptions(): ICommandLineOptions {
   } else {
     command = 'ps';
     // -A: Select all processes
+    // -w: Wide format
     // -o: User-defined format
-    // Order of declared properties impacts the order of the output, so match
-    // the order of wmic.exe output
-    args = ['-Ao', 'comm,ppid,pid'];
+    // Order of declared properties impacts the order of the output. We will
+    // need to request the "comm" property last in order to ensure that the
+    // process names are not truncated on certain platforms
+    args = ['-Awo', 'ppid,pid,comm'];
   }
   return { path: command, args };
 }

libraries/node-core-library/src/test/Executable.test.ts

@@ -341,22 +341,26 @@ describe('Executable process list', () => {
   ];
 
   const UNIX_PROCESS_LIST_OUTPUT: (string | null)[] = [
-    'COMMAND                PPID   PID\n',
+    'PPID   PID   COMMAND\n',
     // Test that the parser can handle referencing a parent that doesn't exist
-    'init                      0     1\n',
-    'process2           ',
+    '   0     1   init\n',
     // Test that the parser can handle a line that is truncated in the middle of a field
     // Test that the parser can handle an entry referencing a parent that hasn't been seen yet
-    '       2     4\n',
-    'process0                  1     2\n',
+    // Test that the parser can handle whitespace at the end of the process name.
+    '   2     4',
+    '   process2           \n',
+    '   1     2   process0\n',
     // Test that the parser can handle empty strings
     '',
     // Test children handling when multiple entries reference the same parent
-    'process1                  1     3\n'
+    '   1     3   process1\n'
   ];
 
   test('parses win32 output', () => {
-    const processListMap: Map<number, IProcessInfo> = parseProcessListOutput(WIN32_PROCESS_LIST_OUTPUT);
+    const processListMap: Map<number, IProcessInfo> = parseProcessListOutput(
+      WIN32_PROCESS_LIST_OUTPUT,
+      'win32'
+    );
     const results: IProcessInfo[] = [...processListMap.values()].sort();
 
     // Expect 7 because we reference a parent that doesn't exist
@@ -379,7 +383,8 @@ describe('Executable process list', () => {
 
   test('parses win32 stream output', async () => {
     const processListMap: Map<number, IProcessInfo> = await parseProcessListOutputAsync(
-      Readable.from(WIN32_PROCESS_LIST_OUTPUT)
+      Readable.from(WIN32_PROCESS_LIST_OUTPUT),
+      'win32'
     );
     const results: IProcessInfo[] = [...processListMap.values()].sort();
 
@@ -402,7 +407,10 @@ describe('Executable process list', () => {
   });
 
   test('parses unix output', () => {
-    const processListMap: Map<number, IProcessInfo> = parseProcessListOutput(UNIX_PROCESS_LIST_OUTPUT);
+    const processListMap: Map<number, IProcessInfo> = parseProcessListOutput(
+      UNIX_PROCESS_LIST_OUTPUT,
+      'linux'
+    );
     const results: IProcessInfo[] = [...processListMap.values()].sort();
 
     // Expect 5 because we reference a parent that doesn't exist
@@ -423,7 +431,8 @@ describe('Executable process list', () => {
 
   test('parses unix stream output', async () => {
     const processListMap: Map<number, IProcessInfo> = await parseProcessListOutputAsync(
-      Readable.from(UNIX_PROCESS_LIST_OUTPUT)
+      Readable.from(UNIX_PROCESS_LIST_OUTPUT),
+      'linux'
     );
     const results: IProcessInfo[] = [...processListMap.values()].sort();