From 20c4767dce97f93a2f24e18d31ba3d3697a4380e Mon Sep 17 00:00:00 2001 From: Anwarul Islam Date: Mon, 28 Oct 2024 21:34:08 +0600 Subject: [PATCH] fix: trailing comma makes json invalid (#4416) Co-authored-by: Andrew Bastin Co-authored-by: jamesgeorge007 <25279263+jamesgeorge007@users.noreply.github.com> --- .../components/environments/my/Details.vue | 4 +- .../components/environments/teams/Details.vue | 4 +- .../src/helpers/editor/linting/jsonc.ts | 83 +++++- .../src/helpers/editor/linting/jsoncPretty.ts | 262 +----------------- .../src/helpers/utils/EffectiveURL.ts | 6 +- 5 files changed, 93 insertions(+), 266 deletions(-) diff --git a/packages/hoppscotch-common/src/components/environments/my/Details.vue b/packages/hoppscotch-common/src/components/environments/my/Details.vue index eaf245a66..3f9a05899 100644 --- a/packages/hoppscotch-common/src/components/environments/my/Details.vue +++ b/packages/hoppscotch-common/src/components/environments/my/Details.vue @@ -352,7 +352,7 @@ watch( env: { key: e.key, value: e.secret - ? secretEnvironmentService.getSecretEnvironmentVariable( + ? (secretEnvironmentService.getSecretEnvironmentVariable( props.editingEnvironmentIndex === "Global" ? "Global" : workingEnvID.value, @@ -360,7 +360,7 @@ watch( )?.value ?? // @ts-expect-error `value` field can exist for secret environment variables as inferred while importing e.value ?? - "" + "") : e.value, secret: e.secret, }, diff --git a/packages/hoppscotch-common/src/components/environments/teams/Details.vue b/packages/hoppscotch-common/src/components/environments/teams/Details.vue index cde3ff194..9833cb912 100644 --- a/packages/hoppscotch-common/src/components/environments/teams/Details.vue +++ b/packages/hoppscotch-common/src/components/environments/teams/Details.vue @@ -311,13 +311,13 @@ watch( env: { key: e.key, value: e.secret - ? secretEnvironmentService.getSecretEnvironmentVariable( + ? (secretEnvironmentService.getSecretEnvironmentVariable( editingID.value ?? "", index )?.value ?? // @ts-expect-error `value` field can exist for secret environment variables as inferred while importing e.value ?? - "" + "") : e.value, secret: e.secret, }, diff --git a/packages/hoppscotch-common/src/helpers/editor/linting/jsonc.ts b/packages/hoppscotch-common/src/helpers/editor/linting/jsonc.ts index daa905ab7..712b3700d 100644 --- a/packages/hoppscotch-common/src/helpers/editor/linting/jsonc.ts +++ b/packages/hoppscotch-common/src/helpers/editor/linting/jsonc.ts @@ -1,7 +1,7 @@ +import { Node, parseTree, stripComments as stripComments_ } from "jsonc-parser" +import jsoncParse from "~/helpers/jsoncParse" import { convertIndexToLineCh } from "../utils" import { LinterDefinition, LinterResult } from "./linter" -import jsoncParse from "~/helpers/jsoncParse" -import { stripComments } from "jsonc-parser" const linter: LinterDefinition = (text) => { try { @@ -19,13 +19,88 @@ const linter: LinterDefinition = (text) => { } } +/** + * 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 or null + * if the conversion failed + */ +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 + + // If the valueNode configuration is wrong, this will return an error, which will propagate up + return `${JSON.stringify(keyNode)}:${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 from a JSON string. * @param jsonString The JSON string with comments. * @returns The JSON string without comments. */ -export function removeComments(jsonString: string): string { - return stripComments(jsonString) + +export function stripComments(jsonString: string) { + return stripCommentsAndCommas(stripComments_(jsonString)) } export default linter diff --git a/packages/hoppscotch-common/src/helpers/editor/linting/jsoncPretty.ts b/packages/hoppscotch-common/src/helpers/editor/linting/jsoncPretty.ts index 392449897..93b39c191 100644 --- a/packages/hoppscotch-common/src/helpers/editor/linting/jsoncPretty.ts +++ b/packages/hoppscotch-common/src/helpers/editor/linting/jsoncPretty.ts @@ -1,258 +1,10 @@ -import jsonParse, { - JSONArrayValue, - JSONCommentValue, - JSONObjectValue, - JSONValue, -} from "~/helpers/jsoncParse" +import { format, applyEdits } from "jsonc-parser" -type PrettifyOptions = { - indent?: string | number - maxLength?: number - commentSpace?: boolean - trailingComma?: boolean -} - -const DEFAULT_OPTIONS: Required = { - indent: 2, - maxLength: 80, - commentSpace: true, - trailingComma: true, -} - -function prettify( - ast: JSONObjectValue | JSONArrayValue, - options: PrettifyOptions = {} -): string { - const opts = { ...DEFAULT_OPTIONS, ...options } - const indent = - typeof opts.indent === "number" ? " ".repeat(opts.indent) : opts.indent - return formatValue(ast, opts, 0, indent) -} - -function formatValue( - node: JSONValue, - options: Required, - depth: number, - indent: string -): string { - switch (node.kind) { - case "Object": - return formatObject(node, options, depth, indent) - case "Array": - return formatArray(node, options, depth, indent) - case "String": - return JSON.stringify(node.value) - case "Number": - case "Boolean": - return String(node.value) - case "Null": - return "null" - default: - return "" - } -} - -function formatComments( - comments: JSONCommentValue[] | undefined, - options: Required, - indentation: string, - inline: boolean = false -): string { - if (!comments?.length) return "" - - return comments - .map((comment) => { - if (comment.kind === "SingleLineComment") { - const space = options.commentSpace ? " " : "" - return inline - ? ` //${space}${comment.value}` - : `\n${indentation}//${space}${comment.value}` - } - const space = options.commentSpace ? " " : "" - const commentLines = comment.value.split("\n") - - if (commentLines.length === 1) { - return inline - ? ` /*${space}${comment.value}${space}*/` - : `\n${indentation}/*${space}${comment.value}${space}*/` - } - - return ( - `\n${indentation}/*\n` + - commentLines.map((line) => `${indentation} * ${line}`).join("\n") + - `\n${indentation} */` - ) - }) - .join("") -} - -function formatObject( - node: JSONObjectValue, - options: Required, - depth: number, - indent: string -): string { - if (node.members.length === 0) { - const comments = formatComments(node.comments, options, "", true) - return `{${comments}}` - } - - const indentation = indent.repeat(depth) - const nextIndentation = indent.repeat(depth + 1) - - let result = "{" - - // Leading comments (before any members) - if (node.comments?.length) { - const leadingComments = node.comments.filter( - (c) => c.start < node.members[0].start - ) - if (leadingComments.length) { - result += formatComments(leadingComments, options, nextIndentation) - } - } - - // Format each member - node.members.forEach((member, index) => { - const isLast = index === node.members.length - 1 - - // Member's leading comments - if (member.comments?.length) { - const leadingComments = member.comments.filter( - (c) => c.start < member.key.start - ) - if (leadingComments.length) { - result += formatComments(leadingComments, options, nextIndentation) - } - } - - // Member key-value pair - result += "\n" + nextIndentation - result += JSON.stringify(member.key.value) + ": " - result += formatValue(member.value, options, depth + 1, indent) - - // Inline comments after the value - if (member.comments?.length) { - const inlineComments = member.comments.filter((c) => c.start > member.end) - if (inlineComments.length) { - result += formatComments(inlineComments, options, "", true) - } - } - - // Add comma if not last item or if trailing comma is enabled - if (!isLast || options.trailingComma) { - result += "," - } - - // Comments between members - if (!isLast && node.comments?.length) { - const betweenComments = node.comments.filter( - (c) => c.start > member.end && c.end < node.members[index + 1].start - ) - if (betweenComments.length) { - result += formatComments(betweenComments, options, nextIndentation) - } - } +export function prettifyJSONC(str: string) { + const editResult = format(str, undefined, { + insertSpaces: true, + tabSize: 2, + insertFinalNewline: true, }) - - // Trailing comments (after last member) - if (node.comments?.length) { - const trailingComments = node.comments.filter( - (c) => - c.start > node.members[node.members.length - 1].end && c.end < node.end - ) - if (trailingComments.length) { - result += formatComments(trailingComments, options, nextIndentation) - } - } - - result += "\n" + indentation + "}" - return result -} - -function formatArray( - node: JSONArrayValue, - options: Required, - depth: number, - indent: string -): string { - if (node.values.length === 0) { - const comments = formatComments(node.comments, options, "", true) - return `[${comments}]` - } - - const indentation = indent.repeat(depth) - const nextIndentation = indent.repeat(depth + 1) - - let result = "[" - - // Leading comments (before any values) - if (node.comments?.length) { - const leadingComments = node.comments.filter( - (c) => c.start < node.values[0].start - ) - if (leadingComments.length) { - result += formatComments(leadingComments, options, nextIndentation) - } - } - - // Format each value - node.values.forEach((value, index) => { - const isLast = index === node.values.length - 1 - - // Value's leading comments - if ("comments" in value && value.comments?.length) { - const leadingComments = value.comments.filter( - (c) => c.start < value.start - ) - if (leadingComments.length) { - result += formatComments(leadingComments, options, nextIndentation) - } - } - - result += "\n" + nextIndentation - result += formatValue(value, options, depth + 1, indent) - - // Inline comments after the value - if ("comments" in value && value.comments?.length) { - const inlineComments = value.comments.filter((c) => c.start > value.end) - if (inlineComments.length) { - result += formatComments(inlineComments, options, "", true) - } - } - - // Add comma if not last item or if trailing comma is enabled - if (!isLast || options.trailingComma) { - result += "," - } - - // Comments between values - if (!isLast && node.comments?.length) { - const betweenComments = node.comments.filter( - (c) => c.start > value.end && c.end < node.values[index + 1].start - ) - if (betweenComments.length) { - result += formatComments(betweenComments, options, nextIndentation) - } - } - }) - - // Trailing comments (after last value) - if (node.comments?.length) { - const trailingComments = node.comments.filter( - (c) => - c.start > node.values[node.values.length - 1].end && c.end < node.end - ) - if (trailingComments.length) { - result += formatComments(trailingComments, options, nextIndentation) - } - } - - result += "\n" + indentation + "]" - return result -} - -export function prettifyJSONC(str: string, options: PrettifyOptions = {}) { - const ast = jsonParse(str) - return prettify(ast, options) + return applyEdits(str, editResult) } diff --git a/packages/hoppscotch-common/src/helpers/utils/EffectiveURL.ts b/packages/hoppscotch-common/src/helpers/utils/EffectiveURL.ts index 4b7885c7a..460bd1ebf 100644 --- a/packages/hoppscotch-common/src/helpers/utils/EffectiveURL.ts +++ b/packages/hoppscotch-common/src/helpers/utils/EffectiveURL.ts @@ -28,7 +28,7 @@ import { arrayFlatMap, arraySort } from "../functional/array" import { toFormData } from "../functional/formData" import { tupleWithSameKeysToRecord } from "../functional/record" import { isJSONContentType } from "./contenttypes" -import { removeComments } from "../editor/linting/jsonc" +import { stripComments } from "../editor/linting/jsonc" export interface EffectiveHoppRESTRequest extends HoppRESTRequest { /** @@ -384,7 +384,7 @@ export const resolvesEnvsInBody = ( let bodyContent = "" if (isJSONContentType(body.contentType)) - bodyContent = removeComments(body.body) + bodyContent = stripComments(body.body) return { contentType: body.contentType, @@ -476,7 +476,7 @@ function getFinalBodyFromRequest( let bodyContent = request.body.body ?? "" if (isJSONContentType(request.body.contentType)) - bodyContent = removeComments(request.body.body) + bodyContent = stripComments(request.body.body) // body can be null if the content-type is not set return parseBodyEnvVariables(bodyContent, envVariables)