From 4fb870e4495d7a5c67e8f53f5126944cca67e0ae Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Fri, 4 Oct 2024 11:30:10 +0100 Subject: [PATCH] Make processStringSync's throw behaviour opt-in. --- .../components/common/bindings/BindingPanel.svelte | 14 ++++++++++---- .../server/src/jsRunner/tests/jsRunner.spec.ts | 10 +++++----- .../server/src/utilities/rowProcessor/utils.ts | 11 ++--------- .../string-templates/src/helpers/javascript.ts | 10 +++++++++- packages/string-templates/src/index.ts | 13 +++---------- packages/string-templates/src/types.ts | 1 + 6 files changed, 30 insertions(+), 29 deletions(-) diff --git a/packages/builder/src/components/common/bindings/BindingPanel.svelte b/packages/builder/src/components/common/bindings/BindingPanel.svelte index 2f39dc6f53..4186cb54cc 100644 --- a/packages/builder/src/components/common/bindings/BindingPanel.svelte +++ b/packages/builder/src/components/common/bindings/BindingPanel.svelte @@ -145,10 +145,16 @@ const debouncedEval = Utils.debounce((expression, context, snippets) => { try { expressionError = null - expressionResult = processStringSync(expression || "", { - ...context, - snippets, - }) + expressionResult = processStringSync( + expression || "", + { + ...context, + snippets, + }, + { + noThrow: false, + } + ) } catch (err) { expressionResult = null expressionError = err diff --git a/packages/server/src/jsRunner/tests/jsRunner.spec.ts b/packages/server/src/jsRunner/tests/jsRunner.spec.ts index 7d2df17c1f..c7ab86f014 100644 --- a/packages/server/src/jsRunner/tests/jsRunner.spec.ts +++ b/packages/server/src/jsRunner/tests/jsRunner.spec.ts @@ -41,11 +41,11 @@ describe("jsRunner (using isolated-vm)", () => { }) it("should prevent sandbox escape", async () => { - await expect( - processJS(`return this.constructor.constructor("return process.env")()`) - ).rejects.toThrow( - "error while running user-supplied JavaScript: ReferenceError: process is not defined" - ) + expect( + await processJS( + `return this.constructor.constructor("return process.env")()` + ) + ).toEqual("ReferenceError: process is not defined") }) describe("helpers", () => { diff --git a/packages/server/src/utilities/rowProcessor/utils.ts b/packages/server/src/utilities/rowProcessor/utils.ts index 29a5de221f..0fa6f62807 100644 --- a/packages/server/src/utilities/rowProcessor/utils.ts +++ b/packages/server/src/utilities/rowProcessor/utils.ts @@ -1,5 +1,5 @@ import { AutoFieldDefaultNames } from "../../constants" -import { processStringSync, UserScriptError } from "@budibase/string-templates" +import { processStringSync } from "@budibase/string-templates" import { AutoColumnFieldMetadata, FieldSchema, @@ -81,14 +81,7 @@ export async function processFormulas( ...row, [column]: tracer.trace("processStringSync", {}, span => { span?.addTags({ table_id: table._id, column, static: isStatic }) - try { - return processStringSync(formula, context) - } catch (err: any) { - if (err.code === UserScriptError.code) { - return err.userScriptError.toString() - } - throw err - } + return processStringSync(formula, context) }), } } diff --git a/packages/string-templates/src/helpers/javascript.ts b/packages/string-templates/src/helpers/javascript.ts index ed8d4ad6d7..2f850b0533 100644 --- a/packages/string-templates/src/helpers/javascript.ts +++ b/packages/string-templates/src/helpers/javascript.ts @@ -95,6 +95,8 @@ export function processJS(handlebars: string, context: any) { } catch (error: any) { onErrorLog && onErrorLog(error) + const { noThrow = true } = context.__opts || {} + // The error handling below is quite messy, because it has fallen to // string-templates to handle a variety of types of error specific to usages // above it in the stack. It would be nice some day to refactor this to @@ -123,11 +125,17 @@ export function processJS(handlebars: string, context: any) { // This is to catch the error that happens if a user-supplied JS script // throws for reasons introduced by the user. if (error.code === UserScriptError.code) { + if (noThrow) { + return error.userScriptError.toString() + } throw error } if (error.name === "SyntaxError") { - return error.toString() + if (noThrow) { + return error.toString() + } + throw error } return "Error while executing JS" diff --git a/packages/string-templates/src/index.ts b/packages/string-templates/src/index.ts index 55b2587b99..77391aefa3 100644 --- a/packages/string-templates/src/index.ts +++ b/packages/string-templates/src/index.ts @@ -123,7 +123,7 @@ function createTemplate( export async function processObject>( object: T, context: object, - opts?: { noHelpers?: boolean; escapeNewlines?: boolean; onlyFound?: boolean } + opts?: ProcessOptions ): Promise { testObject(object) @@ -173,17 +173,13 @@ export async function processString( export function processObjectSync( object: { [x: string]: any }, context: any, - opts: any + opts?: ProcessOptions ): object | Array { testObject(object) for (let key of Object.keys(object || {})) { let val = object[key] if (typeof val === "string") { - try { - object[key] = processStringSync(object[key], context, opts) - } catch (error: any) { - object[key] = error.toString() - } + object[key] = processStringSync(object[key], context, opts) } else if (typeof val === "object") { object[key] = processObjectSync(object[key], context, opts) } @@ -235,9 +231,6 @@ export function processStringSync( return process(string) } } catch (err: any) { - if (err.code === UserScriptError.code) { - throw err - } return input } } diff --git a/packages/string-templates/src/types.ts b/packages/string-templates/src/types.ts index 19785b9273..2a7a430bee 100644 --- a/packages/string-templates/src/types.ts +++ b/packages/string-templates/src/types.ts @@ -3,6 +3,7 @@ export interface ProcessOptions { noEscaping?: boolean noHelpers?: boolean noFinalise?: boolean + noThrow?: boolean escapeNewlines?: boolean onlyFound?: boolean disabledHelpers?: string[]