Skip to content

Commit

Permalink
fix: Fix handling of large json data when writing to file via --json …
Browse files Browse the repository at this point in the history
…[CLI-73] (#5093)

* fix: Support large json data structures via --json

---------

Co-authored-by: Peter Schäfer <[email protected]>
  • Loading branch information
j-luong and PeterSchafer authored Mar 22, 2024
1 parent 9419e14 commit c0d401c
Show file tree
Hide file tree
Showing 13 changed files with 491 additions and 72 deletions.
9 changes: 8 additions & 1 deletion .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ executors:
alpine:
docker:
- image: alpine:3.17
resource_class: xlarge
generic-ubuntu:
docker:
- image: ubuntu:latest
Expand All @@ -49,6 +50,11 @@ executors:
- image: snyklabs/cli-build:20240319-123447
working_directory: /mnt/ramdisk/snyk
resource_class: large
docker-amd64-xl:
docker:
- image: bastiandoetsch209/cli-build:20240214-145818
working_directory: /mnt/ramdisk/snyk
resource_class: xlarge
docker-arm64:
docker:
- image: snyklabs/cli-build-arm64:20240319-123447
Expand Down Expand Up @@ -512,7 +518,7 @@ workflows:
ignore: main
requires:
- build linux amd64
executor: docker-amd64
executor: docker-amd64-xl
test_snyk_command: ./binary-releases/snyk-linux

- acceptance-tests:
Expand Down Expand Up @@ -1050,6 +1056,7 @@ jobs:
TEST_SNYK_COMMAND: << parameters.test_snyk_command >>
TEST_SNYK_DONT_SKIP_ANYTHING: << parameters.dont_skip_tests >>
JEST_JUNIT_OUTPUT_DIR: test/reports
NODE_OPTIONS: --max-old-space-size=4096
- store_test_results:
path: test/reports
- store_artifacts:
Expand Down
5 changes: 3 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -156,12 +156,13 @@
"depcheck": "^1.4.3",
"eslint": "6.8.0",
"eslint-config-prettier": "^6.1.0",
"jest-junit": "^16.0.0",
"eslint-plugin-anti-trojan-source": "^1.1.1",
"eslint-plugin-jest": "^24.4.0",
"express": "^4.17.1",
"fs-extra": "^9.1.0",
"jest": "^28.1.3",
"jest-junit": "^16.0.0",
"jsonparse": "^1.3.1",
"lodash": "^4.17.20",
"mock-fs": "^4.13.0",
"node-loader": "^2.0.0",
Expand Down
8 changes: 6 additions & 2 deletions src/cli/commands/test/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import * as Debug from 'debug';
import { EOL } from 'os';
const cloneDeep = require('lodash.clonedeep');
const omit = require('lodash.omit');
const assign = require('lodash.assign');
import chalk from 'chalk';
import { MissingArgError } from '../../../lib/errors';
Expand Down Expand Up @@ -236,8 +237,7 @@ export default async function test(
}
}
err.code = 'VULNS';
const dataToSendNoVulns = dataToSend;
delete dataToSendNoVulns.vulnerabilities;
const dataToSendNoVulns = omit(dataToSend, 'vulnerabilities');
err.jsonNoVulns = dataToSendNoVulns;
}

Expand All @@ -252,6 +252,10 @@ export default async function test(
err.json = stringifiedData;
err.jsonStringifiedResults = stringifiedJsonData;
err.sarifStringifiedResults = stringifiedSarifData;
// set jsonPayload if we failed to stringify it
if (jsonPayload) {
err.jsonPayload = jsonPayload;
}
throw err;
}

Expand Down
9 changes: 8 additions & 1 deletion src/cli/main.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import * as Debug from 'debug';
import * as pathLib from 'path';
import { JsonStreamStringify } from 'json-stream-stringify';

// import args as a first internal module
import { args as argsLib, Args, ArgsOptions } from './args';
Expand Down Expand Up @@ -144,7 +145,13 @@ async function handleError(args, error) {
const output = vulnsFound
? error.message
: stripAnsi(error.json || error.stack);
console.log(output);
if (error.jsonPayload) {
new JsonStreamStringify(error.jsonPayload, undefined, 2).pipe(
process.stdout,
);
} else {
console.log(output);
}
} else {
if (!args.options.quiet) {
const result = errors.message(error);
Expand Down
7 changes: 0 additions & 7 deletions src/lib/formatters/test/format-test-results.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,13 +105,6 @@ 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
Expand Down
13 changes: 3 additions & 10 deletions src/lib/json.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ const debug = require('debug')('snyk-json');

/**
* Attempt to json-stringify an object which is potentially very large and might exceed the string limit.
* If it does exceed the string limit, try again without pretty-print to hopefully come out below the string limit.
* If it does exceed the string limit, return empty string.
* @param obj the object from which you want to get a JSON string
*/
export function jsonStringifyLargeObject(obj: any): string {
Expand All @@ -11,14 +11,7 @@ export function jsonStringifyLargeObject(obj: any): string {
res = JSON.stringify(obj, null, 2);
return res;
} catch (err) {
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;
}
debug('jsonStringifyLargeObject failed: ', err);
return res;
}
}
10 changes: 5 additions & 5 deletions src/lib/request/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { Payload } from './types';
import { getVersion } from '../version';
import * as https from 'https';
import * as http from 'http';
import { jsonStringifyLargeObject } from '../json';

const debug = debugModule('snyk:req');
const snykDebug = debugModule('snyk');
Expand Down Expand Up @@ -46,7 +47,7 @@ function setupRequest(payload: Payload) {
debug('compressing request body');
const json = JSON.stringify(body);
if (json.length < 1e4) {
debug(JSON.stringify(body, null, 2));
debug(json);
}

// always compress going upstream
Expand All @@ -58,7 +59,7 @@ function setupRequest(payload: Payload) {

let callGraphLength: number | null = null;
if (body.callGraph) {
callGraphLength = JSON.stringify(body.callGraph).length;
callGraphLength = jsonStringifyLargeObject(body.callGraph).length;
snykDebug('call graph size:', callGraphLength);
}

Expand All @@ -84,7 +85,7 @@ function setupRequest(payload: Payload) {
}

try {
debug('request payload: ', JSON.stringify(payload));
debug('request payload: ', jsonStringifyLargeObject(payload));
} catch (e) {
debug('request payload is too big to log', e);
}
Expand Down Expand Up @@ -139,15 +140,14 @@ export async function makeRequest(
payload: Payload,
): Promise<{ res: needle.NeedleResponse; body: any }> {
const { method, url, data, options } = setupRequest(payload);
debug(data);

return new Promise((resolve, reject) => {
needle.request(method, url, data, options, (err, res, respBody) => {
debug(err);
debug(
'response (%s): ',
(res || {}).statusCode,
JSON.stringify(respBody),
jsonStringifyLargeObject(respBody),
);
if (err) {
return reject(err);
Expand Down
Loading

0 comments on commit c0d401c

Please sign in to comment.