Skip to content

Commit 4640cea

Browse files
authored
feat: enabling safe functions in expression engine (#5403)
## Description allowing toLowerCase(), replace(), and split() to be used in the expression editor. Real life use-case explained in the discord message. https://discord.com/channels/955905230107738152/1418928616648998992/1419623805541814393 ## Steps for reproduction 1. use any of the mentioned functions in the expression editor 2. they're currently not allowed even though they'd be safe to use 3. those functions (and presumably others as well) can be used in hosted webstudio by deploying my changes on a locally hosted webstudio editor, using the functions, and copying the pages containing the functions to the hosted version. -> validation check bypass ## Code Review - [x] hi @kof, I need you to do - conceptual review (does webstudio WANT to allow these functions?) - test it on preview ## Before requesting a review - [x] made a self-review - [x] added inline comments where things may be not obvious (the "why", not "what") ## Before merging - [ ] tested locally and on preview environment (preview dev login: 0000) - [ ] updated [test cases](https://github.com/webstudio-is/webstudio/blob/main/apps/builder/docs/test-cases.md) document - [ ] added tests - [ ] if any new env variables are added, added them to `.env` file
1 parent a9f838a commit 4640cea

File tree

2 files changed

+191
-2
lines changed

2 files changed

+191
-2
lines changed

packages/sdk/src/expression.test.ts

Lines changed: 141 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ describe("lint expression", () => {
157157
).toEqual([
158158
error(1, 13, "Functions are not supported"),
159159
error(17, 25, "Functions are not supported"),
160-
error(29, 33, "Functions are not supported"),
160+
error(29, 33, `"fn" function is not supported`),
161161
]);
162162
});
163163

@@ -220,6 +220,71 @@ describe("lint expression", () => {
220220
error(1, 8, `"await" keyword is not supported`),
221221
]);
222222
});
223+
224+
test.each([
225+
"toLowerCase",
226+
"replace",
227+
"split",
228+
"at",
229+
"endsWith",
230+
"includes",
231+
"startsWith",
232+
"toUpperCase",
233+
"toLocaleLowerCase",
234+
"toLocaleUpperCase",
235+
])("allow safe string method: %s", (method) => {
236+
expect(
237+
lintExpression({
238+
expression: `title.${method}()`,
239+
availableVariables: new Set(["title"]),
240+
})
241+
).toEqual([]);
242+
});
243+
244+
test.each(["at", "includes", "join", "slice"])(
245+
"allow safe array method: %s",
246+
(method) => {
247+
expect(
248+
lintExpression({
249+
expression: `arr.${method}()`,
250+
availableVariables: new Set(["arr"]),
251+
})
252+
).toEqual([]);
253+
}
254+
);
255+
256+
test("allow chained string methods", () => {
257+
expect(
258+
lintExpression({
259+
expression: `title.toLowerCase().replace(" ", "-").split("-")`,
260+
availableVariables: new Set(["title"]),
261+
})
262+
).toEqual([]);
263+
});
264+
265+
test("forbid unsafe method calls", () => {
266+
expect(
267+
lintExpression({
268+
expression: `arr.pop()`,
269+
availableVariables: new Set(["arr"]),
270+
})
271+
).toEqual([error(0, 9, `"pop" function is not supported`)]);
272+
expect(
273+
lintExpression({
274+
expression: `obj.push(1)`,
275+
availableVariables: new Set(["obj"]),
276+
})
277+
).toEqual([error(0, 11, `"push" function is not supported`)]);
278+
});
279+
280+
test("forbid standalone function calls", () => {
281+
expect(
282+
lintExpression({
283+
expression: `func()`,
284+
availableVariables: new Set(["func"]),
285+
})
286+
).toEqual([error(0, 6, `"func" function is not supported`)]);
287+
});
223288
});
224289

225290
test("check simple literals", () => {
@@ -346,6 +411,81 @@ describe("transpile expression", () => {
346411
}
347412
expect(errorString).toEqual(`Unexpected token (1:0) in ""`);
348413
});
414+
415+
test("transpile string methods with optional chaining", () => {
416+
expect(
417+
transpileExpression({
418+
expression: "title.toLowerCase()",
419+
executable: true,
420+
})
421+
).toEqual("title?.toLowerCase?.()");
422+
expect(
423+
transpileExpression({
424+
expression: "user.name.replace(' ', '-')",
425+
executable: true,
426+
})
427+
).toEqual("user?.name?.replace?.(' ', '-')");
428+
expect(
429+
transpileExpression({
430+
expression: "data.title.split('-')",
431+
executable: true,
432+
})
433+
).toEqual("data?.title?.split?.('-')");
434+
});
435+
436+
test("transpile chained string methods with optional chaining", () => {
437+
expect(
438+
transpileExpression({
439+
expression: "title.toLowerCase().replace(/\\s+/g, '-')",
440+
executable: true,
441+
})
442+
).toEqual("title?.toLowerCase?.()?.replace?.(/\\s+/g, '-')");
443+
expect(
444+
transpileExpression({
445+
expression: "user.name.toLowerCase().replace(' ', '-').split('-')",
446+
executable: true,
447+
})
448+
).toEqual("user?.name?.toLowerCase?.()?.replace?.(' ', '-')?.split?.('-')");
449+
});
450+
451+
test("transpile array methods with optional chaining", () => {
452+
expect(
453+
transpileExpression({
454+
expression: "items.map(item => item.id)",
455+
executable: true,
456+
})
457+
).toEqual("items?.map?.(item => item?.id)");
458+
expect(
459+
transpileExpression({
460+
expression: "data.list.filter(x => x > 0)",
461+
executable: true,
462+
})
463+
).toEqual("data?.list?.filter?.(x => x > 0)");
464+
});
465+
466+
test("transpile nested method calls with optional chaining", () => {
467+
expect(
468+
transpileExpression({
469+
expression: "obj.method().prop.anotherMethod()",
470+
executable: true,
471+
})
472+
).toEqual("obj?.method?.()?.prop?.anotherMethod?.()");
473+
});
474+
475+
test("preserve existing optional chaining", () => {
476+
expect(
477+
transpileExpression({
478+
expression: "obj?.method?.()",
479+
executable: true,
480+
})
481+
).toEqual("obj?.method?.()");
482+
expect(
483+
transpileExpression({
484+
expression: "obj?.prop?.method?.()",
485+
executable: true,
486+
})
487+
).toEqual("obj?.prop?.method?.()");
488+
});
349489
});
350490

351491
describe("object expression transformations", () => {

packages/sdk/src/expression.ts

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,21 @@ type ExpressionVisitor = {
2929
[K in Expression["type"]]: (node: Extract<Expression, { type: K }>) => void;
3030
};
3131

32+
const allowedStringMethods = new Set([
33+
"toLowerCase",
34+
"replace",
35+
"split",
36+
"at",
37+
"endsWith",
38+
"includes",
39+
"startsWith",
40+
"toUpperCase",
41+
"toLocaleLowerCase",
42+
"toLocaleUpperCase",
43+
]);
44+
45+
const allowedArrayMethods = new Set(["at", "includes", "join", "slice"]);
46+
3247
export const lintExpression = ({
3348
expression,
3449
availableVariables = new Set(),
@@ -112,7 +127,28 @@ export const lintExpression = ({
112127
ThisExpression: addMessage(`"this" keyword is not supported`),
113128
FunctionExpression: addMessage("Functions are not supported"),
114129
UpdateExpression: addMessage("Increment and decrement are not supported"),
115-
CallExpression: addMessage("Functions are not supported"),
130+
CallExpression(node) {
131+
let calleeName;
132+
if (node.callee.type === "MemberExpression") {
133+
if (node.callee.property.type === "Identifier") {
134+
const methodName = node.callee.property.name;
135+
if (
136+
allowedStringMethods.has(methodName) ||
137+
allowedArrayMethods.has(methodName)
138+
) {
139+
return;
140+
}
141+
calleeName = methodName;
142+
}
143+
} else if (node.callee.type === "Identifier") {
144+
calleeName = node.callee.name;
145+
}
146+
if (calleeName) {
147+
addMessage(`"${calleeName}" function is not supported`)(node);
148+
} else {
149+
addMessage("Functions are not supported")(node);
150+
}
151+
},
116152
NewExpression: addMessage("Classes are not supported"),
117153
SequenceExpression: addMessage(`Only single expression is supported`),
118154
ArrowFunctionExpression: addMessage("Functions are not supported"),
@@ -257,6 +293,19 @@ export const transpileExpression = ({
257293
replacements.push([dotIndex, dotIndex, "?."]);
258294
}
259295
},
296+
CallExpression(node) {
297+
if (executable === false || node.optional) {
298+
return;
299+
}
300+
// Add optional chaining to method calls: obj.method() -> obj?.method?.()
301+
if (node.callee.type === "MemberExpression") {
302+
// Find the opening parenthesis after the method name
303+
const openParenIndex = expression.indexOf("(", node.callee.end);
304+
if (openParenIndex !== -1) {
305+
replacements.push([openParenIndex, openParenIndex, "?."]);
306+
}
307+
}
308+
},
260309
});
261310
// order from the latest to the first insertion to not break other positions
262311
replacements.sort(([leftStart], [rightStart]) => rightStart - leftStart);

0 commit comments

Comments
 (0)