fix: strip ansi color control chars from json (#5708)

* fix: strip ansi color control chars from json @W-16375638@ * chore: remove strip-ansi * chore: add tests * chore: fix call chain * test: new unit tests for utils.test.ts * chore: better handle command errors * chore: fix comments for failures --------- Co-authored-by: Daphne Yang <[email protected]> Co-authored-by: Daphne Yang <[email protected]>
forcedotcom · Aug 5, 2024 · 837f574 · 837f574
1 parent 0b2997c
commit 837f574
Show file tree

Hide file tree

Showing 10 changed files with 217 additions and 36 deletions.
diff --git a/package-lock.json b/package-lock.json
diff --git a/packages/salesforcedx-utils-vscode/package.json b/packages/salesforcedx-utils-vscode/package.json
@@ -16,7 +16,6 @@
     "applicationinsights": "1.0.7",
     "cross-spawn": "7.0.3",
     "rxjs": "^5.4.1",
-    "strip-ansi": "^5.2.0",
     "tree-kill": "^1.1.0"
   },
   "devDependencies": {

diff --git a/packages/salesforcedx-utils-vscode/src/cli/commandOutput.ts b/packages/salesforcedx-utils-vscode/src/cli/commandOutput.ts
@@ -5,13 +5,15 @@
  * For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
  */
 
+import { stripAnsiInJson } from '../helpers/utils';
 import { CommandExecution } from './commandExecutor';
 
 export class CommandOutput {
   private stdoutBuffer = '';
   private stderrBuffer = '';
 
   public async getCmdResult(execution: CommandExecution): Promise<string> {
+    const hasJsonEnabled = execution.command.args?.some(arg => arg === '--json');
     execution.stdoutSubject.subscribe(realData => {
       this.stdoutBuffer += realData.toString();
     });
@@ -23,9 +25,19 @@ export class CommandOutput {
       (resolve: (result: string) => void, reject: (reason: string) => void) => {
         execution.processExitSubject.subscribe(data => {
           if (data !== undefined && String(data) === '0') {
-            return resolve(this.stdoutBuffer);
+            return resolve(stripAnsiInJson(this.stdoutBuffer, hasJsonEnabled));
           } else {
-            return reject(this.stderrBuffer || this.stdoutBuffer);
+            // Is the command is sf cli - if so, just use stdoutBuffer before stderrBuffer
+            if (execution.command.command === 'sf') {
+              return reject(
+                stripAnsiInJson(this.stdoutBuffer, hasJsonEnabled) ||
+                stripAnsiInJson(this.stderrBuffer, hasJsonEnabled)
+              );
+            }
+            return reject(
+              stripAnsiInJson(this.stderrBuffer, hasJsonEnabled) ||
+                stripAnsiInJson(this.stdoutBuffer, hasJsonEnabled)
+            );
           }
         });
       }

diff --git a/packages/salesforcedx-utils-vscode/src/commands/channelService.ts b/packages/salesforcedx-utils-vscode/src/commands/channelService.ts
@@ -5,9 +5,9 @@
  * For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
  */
 
-import stripAnsi from 'strip-ansi';
 import { OutputChannel, window } from 'vscode';
 import { CommandExecution } from '../cli';
+import { stripAnsi } from '../helpers/utils';
 import { nls } from '../messages';
 import { SettingsService } from '../settings';
 

diff --git a/packages/salesforcedx-utils-vscode/src/helpers/utils.ts b/packages/salesforcedx-utils-vscode/src/helpers/utils.ts
@@ -85,3 +85,40 @@ export const fileUtils = {
   flushFilePath,
   extractJsonObject
 };
+
+export const stripAnsiInJson = (str: string, hasJson: boolean): string => {
+  return str && hasJson ? stripAnsi(str) : str;
+};
+
+export const stripAnsi = (str: string): string => {
+  return str ? str.replaceAll(ansiRegex(), '') : str;
+};
+
+/*
+Copied from https://github.com/sindresorhus/strip-ansi/blob/master/index.js
+
+MIT License
+Copyright (c) Sindre Sorhus <[email protected]> (https://sindresorhus.com)
+
+Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated
+documentation files (the "Software"), to deal in the Software without restriction, including without
+limitation the rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies
+of the Software, and to permit persons to whom the Software is furnished to do so, subject to the following conditions:
+
+The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software.
+
+THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED
+TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.
+IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE
+OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+*/
+
+export const ansiRegex = ({ onlyFirst = false } = {}): RegExp => {
+  const pattern = [
+    '[\\u001B\\u009B][[\\]()#;?]*(?:(?:(?:(?:;[-a-zA-Z\\d\\/#&.:=?%@~_]+)*|[a-zA-Z\\d]+(?:;[-a-zA-Z\\d\\/#&.:=?%@~_]*)*)?\\u0007)',
+    '(?:(?:\\d{1,4}(?:;\\d{0,4})*)?[\\dA-PR-TZcf-nq-uy=><~]))'
+  ].join('|');
+
+  return new RegExp(pattern, onlyFirst ? undefined : 'g');
+};
diff --git a/packages/salesforcedx-utils-vscode/test/jest/helpers/utils.test.ts b/packages/salesforcedx-utils-vscode/test/jest/helpers/utils.test.ts
@@ -4,33 +4,79 @@
  * Licensed under the BSD 3-Clause license.
  * For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
  */
-import { extractJsonObject } from '../../../src/helpers/utils';
-
-describe('extractJsonObject unit tests', () => {
-  const initialValue = {
-    how: 'does',
-    it: true,
-    get: 5,
-    handled: false
-  };
-  const jsonString = JSON.stringify(initialValue);
-
-  it('Should be able to parse a json string.', () => {
-    const result = extractJsonObject(jsonString);
-    expect(result).toStrictEqual(initialValue);
-  });
+import { extractJsonObject, stripAnsiInJson } from '../../../src/helpers/utils';
+
+describe('utils tests', () => {
+  describe('extractJsonObject unit tests', () => {
+    const initialValue = {
+      how: 'does',
+      it: true,
+      get: 5,
+      handled: false
+    };
+    const jsonString = JSON.stringify(initialValue);
+
+    it('Should be able to parse a json string.', () => {
+      const result = extractJsonObject(jsonString);
+      expect(result).toStrictEqual(initialValue);
+    });
 
-  it('Should throw error if argument is a simple text', () => {
-    const invalidJson = initialValue.how;
-    expect(() => extractJsonObject(invalidJson)).toThrow(
-      'The string "does" is not a valid JSON string.'
-    );
+    it('Should throw error if argument is a simple text', () => {
+      const invalidJson = initialValue.how;
+      expect(() => extractJsonObject(invalidJson)).toThrow(
+        'The string "does" is not a valid JSON string.'
+      );
+    });
+
+    it('Should throw error if argument is invalid JSON string', () => {
+      const invalidJson = jsonString.substring(10);
+      expect(() => extractJsonObject(invalidJson)).toThrow(
+        `The string "${invalidJson}" is not a valid JSON string.`
+      );
+    });
   });
+  describe('stripAnsiInJson', () => {
+    it('should return the original string if hasJson is false', () => {
+      const input = 'some string';
+      const result = stripAnsiInJson(input, false);
+      expect(result).toBe(input);
+    });
+
+    it('should return the stripped string if hasJson is true', () => {
+      const input = '\u001b[4msome string\u001b[0m';
+      const result = stripAnsiInJson(input, true);
+      expect(result).toBe('some string');
+    });
+
+    it('should return the original string even when it contains ANSI if hasJson is false', () => {
+      const input = '\u001b[4msome string\u001b[0m';
+      const result = stripAnsiInJson(input, false);
+      expect(result).toBe('\u001b[4msome string\u001b[0m');
+    });
+
+    it('should return the original string if hasJson is true and the string does not contain ANSI', () => {
+      const input = 'some string';
+      const result = stripAnsiInJson(input, true);
+      expect(result).toBe(input);
+    });
+
+    it('should return the original JSON string if hasJson is false', () => {
+      const input = '{"key": "value"}';
+      const result = stripAnsiInJson(input, false);
+      expect(result).toBe(input);
+    });
+
+    it('should return the stripped JSON string if hasJson is true', () => {
+      const input = '{"key": "\u001b[4mvalue\u001b[0m"}';
+      const result = stripAnsiInJson(input, true);
+      expect(result).toBe('{"key": "value"}');
+    });
 
-  it('Should throw error if argument is invalid JSON string', () => {
-    const invalidJson = jsonString.substring(10);
-    expect(() => extractJsonObject(invalidJson)).toThrow(
-      `The string "${invalidJson}" is not a valid JSON string.`
-    );
+    it('should handle complex JSON with ANSI codes', () => {
+      const input =
+        '{"key1": "\u001b[31mvalue1\u001b[0m", "key2": "\u001b[32mvalue2\u001b[0m"}';
+      const result = stripAnsiInJson(input, true);
+      expect(result).toBe('{"key1": "value1", "key2": "value2"}');
+    });
   });
 });
diff --git a/packages/salesforcedx-utils/src/cli/commandOutput.ts b/packages/salesforcedx-utils/src/cli/commandOutput.ts
@@ -5,13 +5,18 @@
  * For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
  */
 
+import { stripAnsiInJson } from '../helpers';
 import { CommandExecution } from '../types';
+import { JSON_FLAG } from './commandBuilder';
 
 export class CommandOutput {
   private stdoutBuffer = '';
   private stderrBuffer = '';
 
   public async getCmdResult(execution: CommandExecution): Promise<string> {
+    const hasJsonEnabled = execution.command?.args?.some(
+      arg => arg === JSON_FLAG
+    );
     execution.stdoutSubject.subscribe(realData => {
       this.stdoutBuffer += realData.toString();
     });
@@ -23,9 +28,19 @@ export class CommandOutput {
       (resolve: (result: string) => void, reject: (reason: string) => void) => {
         execution.processExitSubject.subscribe(data => {
           if (data !== undefined && String(data) === '0') {
-            return resolve(this.stdoutBuffer);
+            return resolve(stripAnsiInJson(this.stdoutBuffer, hasJsonEnabled));
           } else {
-            return reject(this.stderrBuffer || this.stdoutBuffer);
+            // Is the command is sf cli - if so, just use stdoutBuffer before stderrBuffer
+            if (execution.command.command === 'sf') {
+              return reject(
+                stripAnsiInJson(this.stdoutBuffer, hasJsonEnabled) ||
+                stripAnsiInJson(this.stderrBuffer, hasJsonEnabled)
+              );
+            }
+            return reject(
+              stripAnsiInJson(this.stderrBuffer, hasJsonEnabled) ||
+              stripAnsiInJson(this.stdoutBuffer, hasJsonEnabled)
+            );
           }
         });
       }

diff --git a/packages/salesforcedx-utils/src/helpers/index.ts b/packages/salesforcedx-utils/src/helpers/index.ts
@@ -6,3 +6,4 @@
  */
 export { extractJsonObject } from './extractJsonObject';
 export { ensureCurrentWorkingDirIsProjectPath } from './ensureCurrentWorkingDirIsProjectPath';
+export { stripAnsiInJson } from './utils';
diff --git a/packages/salesforcedx-utils/src/helpers/utils.ts b/packages/salesforcedx-utils/src/helpers/utils.ts
@@ -0,0 +1,39 @@
+/*
+ * Copyright (c) 2024, salesforce.com, inc.
+ * All rights reserved.
+ * Licensed under the BSD 3-Clause license.
+ * For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
+ */
+
+export const stripAnsiInJson = (str: string, hasJson: boolean): string => {
+  return hasJson ? str.replaceAll(ansiRegex(), '') : str;
+};
+
+/*
+Copied from https://github.com/sindresorhus/strip-ansi/blob/master/index.js
+
+MIT License
+Copyright (c) Sindre Sorhus <[email protected]> (https://sindresorhus.com)
+
+Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated
+documentation files (the "Software"), to deal in the Software without restriction, including without
+limitation the rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies
+of the Software, and to permit persons to whom the Software is furnished to do so, subject to the following conditions:
+
+The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software.
+
+THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED
+TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.
+IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE
+OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+*/
+
+const ansiRegex = ({ onlyFirst = false } = {}): RegExp => {
+  const pattern = [
+    '[\\u001B\\u009B][[\\]()#;?]*(?:(?:(?:(?:;[-a-zA-Z\\d\\/#&.:=?%@~_]+)*|[a-zA-Z\\d]+(?:;[-a-zA-Z\\d\\/#&.:=?%@~_]*)*)?\\u0007)',
+    '(?:(?:\\d{1,4}(?:;\\d{0,4})*)?[\\dA-PR-TZcf-nq-uy=><~]))'
+  ].join('|');
+
+  return new RegExp(pattern, onlyFirst ? undefined : 'g');
+};
diff --git a/packages/salesforcedx-utils/test/jest/cli/commandOutput.test.ts b/packages/salesforcedx-utils/test/jest/cli/commandOutput.test.ts
@@ -28,6 +28,9 @@ describe('CommandOutput Unit Tests.', () => {
       },
       processExitSubject: {
         subscribe: jest.fn()
+      },
+      command: {
+        command: 'sf'
       }
     };
     commandOutput = new CommandOutput();
@@ -77,7 +80,8 @@ describe('CommandOutput Unit Tests.', () => {
     });
   });
 
-  it('Should have data from stderr on failure.', async () => {
+  it('Should have data from stderr on failure for not command sf.', async () => {
+    fakeExecution.command.command = 'notsf';
     const stdoutCallback =
       fakeExecution.stdoutSubject.subscribe.mock.calls[0][0];
     stdoutCallback(goodOutput);
@@ -86,23 +90,52 @@ describe('CommandOutput Unit Tests.', () => {
     stderrCallback(badOutput);
     const exitCallback =
       fakeExecution.processExitSubject.subscribe.mock.calls[0][0];
-    // Call the exit callback with a 0 response to indicate success
+    // Call the exit callback with a 1 response to indicate failure
     exitCallback(failCode);
     result.catch(outValue => {
       expect(outValue).toEqual(badOutput);
     });
   });
 
-  it('Should have data from stdout on failure if there is no stderr.', async () => {
+  it('Should have data from stdout on failure if there is no stderr when command not sf.', async () => {
+    fakeExecution.command.command = 'notsf';
     const stdoutCallback =
       fakeExecution.stdoutSubject.subscribe.mock.calls[0][0];
     stdoutCallback(goodOutput);
     const exitCallback =
       fakeExecution.processExitSubject.subscribe.mock.calls[0][0];
-    // Call the exit callback with a 0 response to indicate success
+    // Call the exit callback with a 1 response to indicate failure
     exitCallback(failCode);
     result.catch(outValue => {
       expect(outValue).toEqual(goodOutput);
     });
   });
+  it('Should have data from stdout on failure for sf command.', async () => {
+    const stdoutCallback =
+      fakeExecution.stdoutSubject.subscribe.mock.calls[0][0];
+    stdoutCallback(badOutput);
+    const stderrCallback =
+      fakeExecution.stderrSubject.subscribe.mock.calls[0][0];
+    stderrCallback(goodOutput);
+    const exitCallback =
+      fakeExecution.processExitSubject.subscribe.mock.calls[0][0];
+    // Call the exit callback with a 1 response to indicate failure
+    exitCallback(failCode);
+    result.catch(outValue => {
+      expect(outValue).toEqual(badOutput);
+    });
+  });
+
+  it('Should have data from stderr on failure if there is no stdout when sf command.', async () => {
+    const stderrCallback =
+      fakeExecution.stderrSubject.subscribe.mock.calls[0][0];
+    stderrCallback(badOutput);
+    const exitCallback =
+      fakeExecution.processExitSubject.subscribe.mock.calls[0][0];
+    // Call the exit callback with a 1 response to indicate failure
+    exitCallback(failCode);
+    result.catch(outValue => {
+      expect(outValue).toEqual(badOutput);
+    });
+  });
 });