fix: strip comments from JSON request bodies in CLI (#5769)

Fixes collections with JSON comments failing in the CLI with `SerializationException` while working fine in the app, where comments are stripped before sending requests, but the CLI was sending them as-is, breaking APIs like AWS Cognito that expect valid JSON.
This commit is contained in:
James George 2026-01-14 17:06:30 +05:30 committed by GitHub
parent 254eb3c958
commit d3144f99fb
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
9 changed files with 414 additions and 29 deletions

View file

@ -48,6 +48,7 @@
"commander": "14.0.2", "commander": "14.0.2",
"isolated-vm": "6.0.2", "isolated-vm": "6.0.2",
"js-md5": "0.8.3", "js-md5": "0.8.3",
"jsonc-parser": "3.3.1",
"lodash-es": "4.17.22", "lodash-es": "4.17.22",
"papaparse": "5.5.3", "papaparse": "5.5.3",
"qs": "6.14.0", "qs": "6.14.0",

View file

@ -180,6 +180,16 @@ describe("hopp test [options] <file_path_or_id>", { timeout: 100000 }, () => {
expect(result.error).toBeNull(); expect(result.error).toBeNull();
}); });
test("Strips comments from JSONC request bodies", async () => {
const args = `test ${getTestJsonFilePath(
"jsonc-body-coll.json",
"collection"
)}`;
const result = await runCLIWithNetworkRetry(args);
if (result === null) return;
expect(result.error).toBeNull();
});
describe("OAuth 2 Authorization type with Authorization Code Grant Type", () => { describe("OAuth 2 Authorization type with Authorization Code Grant Type", () => {
test("Successfully translates the authorization information to headers/query params and sends it along with the request", async () => { test("Successfully translates the authorization information to headers/query params and sends it along with the request", async () => {
const args = `test ${getTestJsonFilePath( const args = `test ${getTestJsonFilePath(

View file

@ -0,0 +1,74 @@
{
"v": 11,
"name": "JSONC Body Test Collection",
"folders": [],
"requests": [
{
"v": "17",
"auth": {
"authType": "inherit",
"authActive": true
},
"body": {
"body": "{\n \"key1\": \"value1\", // inline comment\n \"key2\": \"value2\" // another comment\n}",
"contentType": "application/json"
},
"name": "Echo with inline comments",
"method": "POST",
"params": [],
"headers": [],
"endpoint": "https://echo.hoppscotch.io",
"testScript": "hopp.test('Should successfully parse JSONC with comments', () => {\n hopp.expect(hopp.response.statusCode).toBe(200);\n const data = JSON.parse(hopp.response.body.asJSON().data);\n hopp.expect(data.key1).toBe('value1');\n hopp.expect(data.key2).toBe('value2');\n});",
"preRequestScript": "",
"requestVariables": [],
"responses": {}
},
{
"v": "17",
"auth": {
"authType": "inherit",
"authActive": true
},
"body": {
"body": "{\n /* Multi-line comment\n should also work */\n \"message\": \"test\",\n \"nested\": {\n \"field\": \"value\" // another comment\n }\n}",
"contentType": "application/json"
},
"name": "Echo with multiline comments",
"method": "POST",
"params": [],
"headers": [],
"endpoint": "https://echo.hoppscotch.io",
"testScript": "hopp.test('Should successfully parse JSONC with multiline comments', () => {\n hopp.expect(hopp.response.statusCode).toBe(200);\n const data = JSON.parse(hopp.response.body.asJSON().data);\n hopp.expect(data.message).toBe('test');\n hopp.expect(data.nested.field).toBe('value');\n});",
"preRequestScript": "",
"requestVariables": [],
"responses": {}
},
{
"v": "17",
"auth": {
"authType": "inherit",
"authActive": true
},
"body": {
"body": "{\n \"key\": \"value\",\n \"count\": 42,\n}",
"contentType": "application/json"
},
"name": "Echo with trailing commas",
"method": "POST",
"params": [],
"headers": [],
"endpoint": "https://echo.hoppscotch.io",
"testScript": "hopp.test('Should successfully parse JSONC with trailing commas', () => {\n hopp.expect(hopp.response.statusCode).toBe(200);\n const data = JSON.parse(hopp.response.body.asJSON().data);\n hopp.expect(data.key).toBe('value');\n hopp.expect(data.count).toBe(42);\n});",
"preRequestScript": "",
"requestVariables": [],
"responses": {}
}
],
"auth": {
"authType": "inherit",
"authActive": true
},
"headers": [],
"variables": [],
"description": ""
}

View file

@ -0,0 +1,151 @@
import { describe, expect, test } from "vitest";
import { stripComments } from "../../utils/jsonc";
describe("stripComments", () => {
describe("handles inline comments", () => {
test("removes single inline comment", () => {
const input = '{"key": "value" // comment\n}';
const result = stripComments(input);
const parsed = JSON.parse(result);
expect(parsed).toEqual({ key: "value" });
});
test("removes multiple inline comments", () => {
const input = '{\n "key1": "value1", // comment1\n "key2": "value2" // comment2\n}';
const result = stripComments(input);
const parsed = JSON.parse(result);
expect(parsed).toEqual({ key1: "value1", key2: "value2" });
});
});
describe("handles multiline comments", () => {
test("removes single multiline comment", () => {
const input = '{\n /* This is a comment */\n "key": "value"\n}';
const result = stripComments(input);
const parsed = JSON.parse(result);
expect(parsed).toEqual({ key: "value" });
});
test("removes multiline comment spanning multiple lines", () => {
const input = '{\n /* This is\n a multiline\n comment */\n "key": "value"\n}';
const result = stripComments(input);
const parsed = JSON.parse(result);
expect(parsed).toEqual({ key: "value" });
});
});
describe("handles trailing commas", () => {
test("removes trailing comma in object", () => {
const input = '{"key": "value",}';
const result = stripComments(input);
const parsed = JSON.parse(result);
expect(parsed).toEqual({ key: "value" });
});
test("removes trailing comma in array", () => {
const input = '["item1", "item2",]';
const result = stripComments(input);
const parsed = JSON.parse(result);
expect(parsed).toEqual(["item1", "item2"]);
});
test("removes multiple trailing commas in nested structures", () => {
const input = '{"arr": ["a", "b",], "obj": {"key": "value",},}';
const result = stripComments(input);
const parsed = JSON.parse(result);
expect(parsed).toEqual({ arr: ["a", "b"], obj: { key: "value" } });
});
});
describe("handles combined cases", () => {
test("removes both comments and trailing commas", () => {
const input = '{\n "key1": "value1", // inline comment\n /* block comment */\n "key2": "value2",\n}';
const result = stripComments(input);
const parsed = JSON.parse(result);
expect(parsed).toEqual({ key1: "value1", key2: "value2" });
});
test("handles nested objects with comments and trailing commas", () => {
const input = '{\n "outer": { // comment\n "inner": "value",\n },\n}';
const result = stripComments(input);
const parsed = JSON.parse(result);
expect(parsed).toEqual({ outer: { inner: "value" } });
});
});
describe("handles edge cases", () => {
test("returns empty string unchanged", () => {
const input = "";
const result = stripComments(input);
expect(result).toBe("");
});
test("returns whitespace-only string unchanged", () => {
const input = " \n \t ";
const result = stripComments(input);
expect(result).toBe(input);
});
test("handles valid JSON without comments", () => {
const input = '{"key": "value"}';
const result = stripComments(input);
const parsed = JSON.parse(result);
expect(parsed).toEqual({ key: "value" });
});
test("preserves JSON strings containing comment-like sequences", () => {
const input = '{"url": "https://example.com//path"}';
const result = stripComments(input);
const parsed = JSON.parse(result);
expect(parsed.url).toBe("https://example.com//path");
});
test("handles deeply nested structures", () => {
const input = '{\n "a": {\n "b": {\n "c": {\n "d": "value", // nested comment\n },\n },\n },\n}';
const result = stripComments(input);
const parsed = JSON.parse(result);
expect(parsed).toEqual({ a: { b: { c: { d: "value" } } } });
});
test("handles arrays with mixed content", () => {
const input = '[\n "string",\n 123, // number\n true, // boolean\n null, // null\n {"nested": "object",}, // object\n]';
const result = stripComments(input);
const parsed = JSON.parse(result);
expect(parsed).toEqual(["string", 123, true, null, { nested: "object" }]);
});
});
describe("handles null return from stripComments_", () => {
test("gracefully handles potential null from jsonc-parser", () => {
const input = '{"key": "value"}';
const result = stripComments(input);
expect(result).toBeTruthy();
const parsed = JSON.parse(result);
expect(parsed).toEqual({ key: "value" });
});
});
describe("handles malformed JSON", () => {
test("attempts to parse malformed JSON and returns result", () => {
// jsonc-parser is lenient and tries to repair malformed JSON
const input = '{"key": "value"'; // missing closing brace
const result = stripComments(input);
// The parser will attempt to close the brace
expect(result).toBe('{"key":"value"}');
});
test("gracefully handles completely invalid JSON", () => {
const input = 'this is not json at all {]}{]';
const result = stripComments(input);
// jsonc-parser extracts what it can and returns an object (even if mostly empty)
expect(result).toBe('{}');
});
test("handles JSON with syntax errors", () => {
const input = '{"key": undefined}'; // undefined is not valid JSON
const result = stripComments(input);
// Parser will handle this - exact behavior depends on jsonc-parser
expect(typeof result).toBe('string');
});
});
});

View file

@ -0,0 +1,89 @@
import { Node, parseTree, stripComments as stripComments_ } from "jsonc-parser";
/**
* An internal error that is thrown when an invalid JSONC node configuration
* is encountered
*/
class InvalidJSONCNodeError extends Error {
constructor() {
super();
this.message = "Invalid JSONC node";
}
}
// NOTE: If we choose to export this function, do refactor it to return a result discriminated union instead of throwing
/**
* @throws {InvalidJSONCNodeError} if the node is in an invalid configuration
* @returns The JSON string without comments and trailing commas
*/
function convertNodeToJSON(node: Node): string {
switch (node.type) {
case "string":
return JSON.stringify(node.value);
case "null":
return "null";
case "array":
if (!node.children) {
throw new InvalidJSONCNodeError();
}
return `[${node.children
.map((child) => convertNodeToJSON(child))
.join(",")}]`;
case "number":
return JSON.stringify(node.value);
case "boolean":
return JSON.stringify(node.value);
case "object":
if (!node.children) {
throw new InvalidJSONCNodeError();
}
return `{${node.children
.map((child) => convertNodeToJSON(child))
.join(",")}}`;
case "property":
if (!node.children || node.children.length !== 2) {
throw new InvalidJSONCNodeError();
}
const [keyNode, valueNode] = node.children;
// Use keyNode.value instead of keyNode to avoid circular references.
// Attempting to JSON.stringify(keyNode) directly would throw
// "Converting circular structure to JSON" error.
// If the valueNode configuration is wrong, this will return an error, which will propagate up
return `${JSON.stringify(keyNode.value)}:${convertNodeToJSON(valueNode)}`;
}
}
function stripCommentsAndCommas(text: string): string {
const tree = parseTree(text, undefined, {
allowEmptyContent: true,
allowTrailingComma: true,
});
// If we couldn't parse the tree, return the original text
if (!tree) {
return text;
}
// convertNodeToJSON can throw an error if the tree is invalid
try {
return convertNodeToJSON(tree);
} catch (_) {
return text;
}
}
/**
* Removes comments and trailing commas from a JSONC string.
* This is needed because APIs like AWS Cognito expect valid JSON without comments,
* but Hoppscotch allows users to add comments to their request bodies.
*
* @param jsoncString The JSONC string with comments and/or trailing commas.
* @returns The clean JSON string without comments or trailing commas.
*/
export function stripComments(jsoncString: string): string {
return stripCommentsAndCommas(stripComments_(jsoncString) ?? jsoncString);
}

View file

@ -8,9 +8,9 @@ import {
parseTemplateStringE, parseTemplateStringE,
generateJWTToken, generateJWTToken,
HoppCollectionVariable, HoppCollectionVariable,
calculateHawkHeader calculateHawkHeader,
} from "@hoppscotch/data"; } from "@hoppscotch/data";
import { runPreRequestScript } from "@hoppscotch/js-sandbox/node" import { runPreRequestScript } from "@hoppscotch/js-sandbox/node";
import { createHoppFetchHook } from "./hopp-fetch"; import { createHoppFetchHook } from "./hopp-fetch";
import * as A from "fp-ts/Array"; import * as A from "fp-ts/Array";
import * as E from "fp-ts/Either"; import * as E from "fp-ts/Either";
@ -35,6 +35,7 @@ import {
fetchInitialDigestAuthInfo, fetchInitialDigestAuthInfo,
generateDigestAuthHeader, generateDigestAuthHeader,
} from "./auth/digest"; } from "./auth/digest";
import { stripComments } from "./jsonc";
/** /**
* Runs pre-request-script runner over given request which extracts set ENVs and * Runs pre-request-script runner over given request which extracts set ENVs and
@ -81,29 +82,31 @@ export const preRequestScriptRunner = (
updatedRequest: updatedRequest ?? {}, updatedRequest: updatedRequest ?? {},
}; };
}), }),
TE.chainW(({ preRequestUpdatedEnvs, envForEffectiveRequest, updatedRequest }) => { TE.chainW(
const finalRequest = { ...request, ...updatedRequest }; ({ preRequestUpdatedEnvs, envForEffectiveRequest, updatedRequest }) => {
const finalRequest = { ...request, ...updatedRequest };
return TE.tryCatch( return TE.tryCatch(
async () => { async () => {
const result = await getEffectiveRESTRequest( const result = await getEffectiveRESTRequest(
finalRequest, finalRequest,
envForEffectiveRequest, envForEffectiveRequest,
collectionVariables collectionVariables
); );
// Replace the updatedEnvs from getEffectiveRESTRequest with the one from pre-request script // Replace the updatedEnvs from getEffectiveRESTRequest with the one from pre-request script
// This preserves the global/selected separation // This preserves the global/selected separation
if (E.isRight(result)) { if (E.isRight(result)) {
return E.right({ return E.right({
...result.right, ...result.right,
updatedEnvs: preRequestUpdatedEnvs, updatedEnvs: preRequestUpdatedEnvs,
}); });
} }
return result; return result;
}, },
(reason) => error({ code: "PRE_REQUEST_SCRIPT_ERROR", data: reason }) (reason) => error({ code: "PRE_REQUEST_SCRIPT_ERROR", data: reason })
); );
}), }
),
TE.chainEitherKW((effectiveRequest) => effectiveRequest), TE.chainEitherKW((effectiveRequest) => effectiveRequest),
TE.mapLeft((reason) => TE.mapLeft((reason) =>
isHoppCLIError(reason) isHoppCLIError(reason)
@ -578,6 +581,58 @@ function getFinalBodyFromRequest(
return E.right(body); return E.right(body);
} }
// For JSON content types, parse the string body into a JavaScript object
// so axios can properly serialize it. This includes standard application/json
// and vendor-specific JSON media types (for example those with a +json suffix
// or subtypes whose names end with "json" or "-json").
if (request.body.contentType) {
const mimeType = request.body.contentType.split(";")[0].trim().toLowerCase();
if (
mimeType === "application/json" ||
mimeType.endsWith("+json") ||
mimeType.endsWith("/json") ||
mimeType.endsWith("-json")
) {
const envResult = parseBodyEnvVariablesE(request.body.body, resolvedVariables);
if (E.isLeft(envResult)) {
return E.left(
error({
code: "PARSING_ERROR",
data: `${request.body.body} (${envResult.left})`,
})
);
}
const bodyString = envResult.right;
// If the body string is empty or null, return null
if (!bodyString || S.isEmpty(bodyString.trim())) {
return E.right(null);
}
// Strip comments and trailing commas from JSONC
// This ensures collections with comments work the same in CLI as in desktop app
const cleanedBody = stripComments(bodyString);
// Try to parse the JSON body
try {
const parsedBody = JSON.parse(cleanedBody);
return E.right(JSON.stringify(parsedBody));
} catch (err) {
// If parsing fails after stripping comments, return error to provide
// immediate feedback instead of sending invalid JSON to the API.
// Use original template string to avoid leaking secrets from env vars.
return E.left(
error({
code: "PARSING_ERROR",
data: `${request.body.body} (Invalid JSON in request body: ${err instanceof Error ? err.message : String(err)})`,
})
);
}
}
}
return pipe( return pipe(
parseBodyEnvVariablesE(request.body.body, resolvedVariables), parseBodyEnvVariablesE(request.body.body, resolvedVariables),
E.mapLeft((e) => E.mapLeft((e) =>

View file

@ -188,7 +188,7 @@ describe("Digest Auth", () => {
algorithm: "MD5", algorithm: "MD5",
qop: "auth-int", qop: "auth-int",
opaque: "", opaque: "",
reqBody: '{"name": "test", "value": 123}', reqBody: '{"name":"test","value":123}',
}) })
expect(headers[0].value).toBe(mockDigestHeader) expect(headers[0].value).toBe(mockDigestHeader)

View file

@ -33,8 +33,7 @@ class InvalidJSONCNodeError extends Error {
// NOTE: If we choose to export this function, do refactor it to return a result discriminated union instead of throwing // NOTE: If we choose to export this function, do refactor it to return a result discriminated union instead of throwing
/** /**
* @throws {InvalidJSONCNodeError} if the node is in an invalid configuration * @throws {InvalidJSONCNodeError} if the node is in an invalid configuration
* @returns The JSON string without comments and trailing commas or null * @returns The JSON string without comments and trailing commas
* if the conversion failed
*/ */
function convertNodeToJSON(node: Node): string { function convertNodeToJSON(node: Node): string {
switch (node.type) { switch (node.type) {
@ -69,8 +68,11 @@ function convertNodeToJSON(node: Node): string {
const [keyNode, valueNode] = node.children const [keyNode, valueNode] = node.children
// Use keyNode.value instead of keyNode to avoid circular references.
// Attempting to JSON.stringify(keyNode) directly would throw
// "Converting circular structure to JSON" error.
// If the valueNode configuration is wrong, this will return an error, which will propagate up // If the valueNode configuration is wrong, this will return an error, which will propagate up
return `${JSON.stringify(keyNode)}:${convertNodeToJSON(valueNode)}` return `${JSON.stringify(keyNode.value)}:${convertNodeToJSON(valueNode)}`
} }
} }
@ -100,7 +102,7 @@ function stripCommentsAndCommas(text: string): string {
*/ */
export function stripComments(jsonString: string) { export function stripComments(jsonString: string) {
return stripCommentsAndCommas(stripComments_(jsonString)) return stripCommentsAndCommas(stripComments_(jsonString) ?? jsonString)
} }
export default linter export default linter

View file

@ -415,6 +415,9 @@ importers:
js-md5: js-md5:
specifier: 0.8.3 specifier: 0.8.3
version: 0.8.3 version: 0.8.3
jsonc-parser:
specifier: 3.3.1
version: 3.3.1
lodash-es: lodash-es:
specifier: 4.17.22 specifier: 4.17.22
version: 4.17.22 version: 4.17.22