fix: Fix handling of large json data when writing to file via --json-…

…file-output [CLI-46] (#5007) * chore: use external npm package for streaming large JSON to file * chore: add test to validate file is saved correctly for very large JSON objects --------- Co-authored-by: Luke Watts <luke@snyk.io>
snyk · Jan 29, 2024 · 33485f1 · 33485f1
1 parent b65ffe1
commit 33485f1
Show file tree

Hide file tree

Showing 27 changed files with 816 additions and 49 deletions.
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -84,6 +84,7 @@
     "glob": "^7.1.7",
     "global-agent": "^2.1.12",
     "jest-json-schema": "^6.1.0",
+    "json-stream-stringify": "^3.1.1",
     "lodash.assign": "^4.2.0",
     "lodash.camelcase": "^4.3.0",
     "lodash.capitalize": "^4.2.1",

diff --git a/src/cli/commands/test/index.ts b/src/cli/commands/test/index.ts
@@ -207,13 +207,16 @@ export default async function test(
     stringifiedSarifData,
   } = extractDataToSendFromResults(results, mappedResults, options);
 
+  const jsonPayload = stringifiedJsonData.length === 0 ? dataToSend : null;
+
   if (options.json || options.sarif) {
     // if all results are ok (.ok == true)
     if (mappedResults.every((res) => res.ok)) {
       return TestCommandResult.createJsonTestCommandResult(
         stringifiedData,
         stringifiedJsonData,
         stringifiedSarifData,
+        jsonPayload,
       );
     }
 
@@ -228,6 +231,7 @@ export default async function test(
             stringifiedData,
             stringifiedJsonData,
             stringifiedSarifData,
+            jsonPayload,
           );
         }
       }
@@ -310,6 +314,7 @@ export default async function test(
           response,
           stringifiedJsonData,
           stringifiedSarifData,
+          jsonPayload,
         );
       }
     }
@@ -332,6 +337,10 @@ export default async function test(
     error.userMessage = vulnerableResults[0].userMessage;
     error.jsonStringifiedResults = stringifiedJsonData;
     error.sarifStringifiedResults = stringifiedSarifData;
+    // conditionally set jsonPayload for now, to determine whether to stream data to destination
+    if (stringifiedJsonData.length === 0) {
+      error.jsonPayload = dataToSend;
+    }
     throw error;
   }
 
@@ -345,6 +354,7 @@ export default async function test(
     response,
     stringifiedJsonData,
     stringifiedSarifData,
+    jsonPayload,
   );
 }
 

diff --git a/src/cli/commands/types.ts b/src/cli/commands/types.ts
@@ -18,6 +18,7 @@ export class CommandResult {
 export abstract class TestCommandResult extends CommandResult {
   protected jsonResult = '';
   protected sarifResult = '';
+  protected jsonData = {};
 
   public getJsonResult(): string {
     return this.jsonResult;
@@ -27,41 +28,58 @@ export abstract class TestCommandResult extends CommandResult {
     return this.sarifResult;
   }
 
+  public getJsonData(): Record<string, unknown> {
+    return this.jsonData;
+  }
+
   public static createHumanReadableTestCommandResult(
     humanReadableResult: string,
     jsonResult: string,
     sarifResult?: string,
+    jsonData?: Record<string, unknown>,
   ): HumanReadableTestCommandResult {
     return new HumanReadableTestCommandResult(
       humanReadableResult,
       jsonResult,
       sarifResult,
+      jsonData,
     );
   }
 
   public static createJsonTestCommandResult(
     stdout: string,
     jsonResult?: string,
     sarifResult?: string,
+    jsonPayload?: Record<string, unknown>,
   ): JsonTestCommandResult {
-    return new JsonTestCommandResult(stdout, jsonResult, sarifResult);
+    return new JsonTestCommandResult(
+      stdout,
+      jsonResult,
+      sarifResult,
+      jsonPayload,
+    );
   }
 }
 
 class HumanReadableTestCommandResult extends TestCommandResult {
   protected jsonResult = '';
   protected sarifResult = '';
+  protected jsonData = {};
 
   constructor(
     humanReadableResult: string,
     jsonResult: string,
     sarifResult?: string,
+    jsonData?: Record<string, unknown>,
   ) {
     super(humanReadableResult);
     this.jsonResult = jsonResult;
     if (sarifResult) {
       this.sarifResult = sarifResult;
     }
+    if (jsonData) {
+      this.jsonData = jsonData;
+    }
   }
 
   public getJsonResult(): string {
@@ -71,10 +89,19 @@ class HumanReadableTestCommandResult extends TestCommandResult {
   public getSarifResult(): string {
     return this.sarifResult;
   }
+
+  public getJsonData(): Record<string, unknown> {
+    return this.jsonData;
+  }
 }
 
 class JsonTestCommandResult extends TestCommandResult {
-  constructor(stdout: string, jsonResult?: string, sarifResult?: string) {
+  constructor(
+    stdout: string,
+    jsonResult?: string,
+    sarifResult?: string,
+    jsonData?: Record<string, unknown>,
+  ) {
     super(stdout);
     if (jsonResult) {
       this.jsonResult = jsonResult;
@@ -84,6 +111,9 @@ class JsonTestCommandResult extends TestCommandResult {
     } else {
       this.jsonResult = stdout;
     }
+    if (jsonData) {
+      this.jsonData = jsonData;
+    }
   }
 
   public getJsonResult(): string {

diff --git a/src/cli/main.ts b/src/cli/main.ts
@@ -33,7 +33,10 @@ import stripAnsi = require('strip-ansi');
 import { ExcludeFlagInvalidInputError } from '../lib/errors/exclude-flag-invalid-input';
 import { modeValidation } from './modes';
 import { JsonFileOutputBadInputError } from '../lib/errors/json-file-output-bad-input-error';
-import { saveJsonToFileCreatingDirectoryIfRequired } from '../lib/json-file-output';
+import {
+  saveObjectToFile,
+  saveJsonToFileCreatingDirectoryIfRequired,
+} from '../lib/json-file-output';
 import {
   Options,
   TestOptions,
@@ -44,6 +47,7 @@ import { SarifFileOutputEmptyError } from '../lib/errors/empty-sarif-output-erro
 import { InvalidDetectionDepthValue } from '../lib/errors/invalid-detection-depth-value';
 import { obfuscateArgs } from '../lib/utils';
 import { EXIT_CODES } from './exit-codes';
+const isEmpty = require('lodash/isEmpty');
 
 const debug = Debug('snyk');
 
@@ -74,7 +78,8 @@ async function runCommand(args: Args) {
   // also save the json (in error.json) to file if option is set
   if (args.command === 'test') {
     const jsonResults = (commandResult as TestCommandResult).getJsonResult();
-    await saveResultsToFile(args.options, 'json', jsonResults);
+    const jsonPayload = (commandResult as TestCommandResult).getJsonData();
+    await saveResultsToFile(args.options, 'json', jsonResults, jsonPayload);
     const sarifResults = (commandResult as TestCommandResult).getSarifResult();
     await saveResultsToFile(args.options, 'sarif', sarifResults);
   }
@@ -157,7 +162,13 @@ async function handleError(args, error) {
     }
   }
 
-  await saveResultsToFile(args.options, 'json', error.jsonStringifiedResults);
+  if (error.jsonPayload) {
+    // send raw jsonPayload instead of stringified payload
+    await saveResultsToFile(args.options, 'json', '', error.jsonPayload);
+  } else {
+    // fallback to original behaviour
+    await saveResultsToFile(args.options, 'json', error.jsonStringifiedResults);
+  }
   await saveResultsToFile(args.options, 'sarif', error.sarifStringifiedResults);
 
   const analyticsError = vulnsFound
@@ -208,6 +219,7 @@ function getFullPath(filepathFragment: string): string {
 async function saveJsonResultsToFile(
   stringifiedJson: string,
   jsonOutputFile: string,
+  jsonPayload?: Record<string, unknown>,
 ) {
   if (!jsonOutputFile) {
     console.error('empty jsonOutputFile');
@@ -219,10 +231,15 @@ async function saveJsonResultsToFile(
     return;
   }
 
-  await saveJsonToFileCreatingDirectoryIfRequired(
-    jsonOutputFile,
-    stringifiedJson,
-  );
+  // save to file with jsonPayload object instead of stringifiedJson
+  if (jsonPayload && !isEmpty(jsonPayload)) {
+    await saveObjectToFile(jsonOutputFile, jsonPayload);
+  } else {
+    await saveJsonToFileCreatingDirectoryIfRequired(
+      jsonOutputFile,
+      stringifiedJson,
+    );
+  }
 }
 
 function checkRuntime() {
@@ -438,13 +455,18 @@ async function saveResultsToFile(
   options: ArgsOptions,
   outputType: string,
   jsonResults: string,
+  jsonPayload?: Record<string, unknown>,
 ) {
   const flag = `${outputType}-file-output`;
   const outputFile = options[flag];
-  if (outputFile && jsonResults) {
+  if (outputFile && (jsonResults || jsonPayload)) {
     const outputFileStr = outputFile as string;
     const fullOutputFilePath = getFullPath(outputFileStr);
-    await saveJsonResultsToFile(stripAnsi(jsonResults), fullOutputFilePath);
+    await saveJsonResultsToFile(
+      stripAnsi(jsonResults),
+      fullOutputFilePath,
+      jsonPayload,
+    );
   }
 }
 

diff --git a/src/lib/formatters/test/format-test-results.ts b/src/lib/formatters/test/format-test-results.ts
@@ -105,6 +105,13 @@ export function extractDataToSendFromResults(
     stringifiedJsonData = jsonStringifyLargeObject(jsonData);
   }
 
+  // TODO: Remove this when we support streaming JSON to stdout
+  if (stringifiedJsonData.length === 0 && options['json']) {
+    console.warn(
+      "'--json' does not work for very large objects - try just using '--json-file-output=<filePath>' instead",
+    );
+  }
+
   const dataToSend = options.sarif ? sarifData : jsonData;
   const stringifiedData = options.sarif
     ? stringifiedSarifData

diff --git a/src/lib/json-file-output.ts b/src/lib/json-file-output.ts
@@ -1,6 +1,7 @@
 import { gte } from 'semver';
 import { existsSync, mkdirSync, createWriteStream } from 'fs';
 import * as path from 'path';
+import { JsonStreamStringify } from 'json-stream-stringify';
 
 export const MIN_VERSION_FOR_MKDIR_RECURSIVE = '10.12.0';
 
@@ -78,3 +79,16 @@ export async function saveJsonToFileCreatingDirectoryIfRequired(
     await writeContentsToFileSwallowingErrors(jsonOutputFile, contents);
   }
 }
+
+export async function saveObjectToFile(
+  jsonOutputFile: string,
+  jsonPayload: Record<string, unknown>,
+): Promise<void> {
+  const dirPath = path.dirname(jsonOutputFile);
+  const createDirSuccess = createDirectory(dirPath);
+  if (createDirSuccess) {
+    const writer = createWriteStream(jsonOutputFile);
+
+    await new JsonStreamStringify(jsonPayload).pipe(writer);
+  }
+}
diff --git a/src/lib/json.ts b/src/lib/json.ts
@@ -8,13 +8,17 @@ const debug = require('debug')('snyk-json');
 export function jsonStringifyLargeObject(obj: any): string {
   let res = '';
   try {
-    // first try pretty-print
     res = JSON.stringify(obj, null, 2);
     return res;
   } catch (err) {
-    // if that doesn't work, try non-pretty print
-    debug('JSON.stringify failed - trying again without pretty print', err);
-    res = JSON.stringify(obj);
-    return res;
+    try {
+      // if that doesn't work, try non-pretty print
+      debug('JSON.stringify failed - trying again without pretty print', err);
+      res = JSON.stringify(obj);
+      return res;
+    } catch (error) {
+      debug('jsonStringifyLargeObject failed: ', error);
+      return res;
+    }
   }
 }