Open Jas2042 opened 2 years ago
Still important
Haven't dug into the code deep enough yet, but chatGPT suggested the following template for an approach to replacing new Function(...)
:
"One way to eliminate the new Function call and still evaluate the expression is to employ a safe expression evaluator. Here, we can make use of a simplistic evaluator that handles basic arithmetic operations, which may serve many use cases.
Here's a version of your tryConvertExpr function that uses this approach:
function tryConvertExpr(string) {
var exprRes = /^expr\((.*)\)$/.exec(string);
if (exprRes) {
var expression = exprRes[1];
return function(width, height, dpr) {
return safeEval(expression, { width, height, dpr });
};
}
}
function safeEval(expr, vars) {
// Simple expression evaluator. Supports basic arithmetic operations.
// Note: This evaluator is rather simplistic. Depending on the range of
// expressions you expect, you may want to expand its capabilities or use
// a third-party library.
function evaluate(node) {
switch (node.type) {
case "Literal": return node.value;
case "Identifier": return vars[node.name];
case "BinaryExpression": {
let left = evaluate(node.left);
let right = evaluate(node.right);
switch (node.operator) {
case "+": return left + right;
case "-": return left - right;
case "*": return left * right;
case "/": return left / right;
default: throw new Error(`Unsupported operator: ${node.operator}`);
}
}
default: throw new Error(`Unsupported node type: ${node.type}`);
}
}
let ast;
try {
// Using the Esprima library to parse the expression into an AST.
// You will need to include the Esprima library for this to work.
ast = esprima.parse(expr).body[0].expression;
} catch (e) {
throw new Error("Invalid expression.");
}
return evaluate(ast);
}
In this version:
The safeEval
function evaluates an expression safely without running arbitrary JavaScript. This is achieved by parsing the expression into an abstract syntax tree (AST) using the esprima
library and then evaluating the AST.
The tryConvertExpr
function returns a function that, when called with width
, height
, and dpr
, evaluates the expression in the context of those variables.
Note: You'll need to include the Esprima library for this code to work, as it is used to parse the expressions.
Ensure you understand the limits of this evaluator. Depending on the complexity of the expressions you need to handle, you might need to expand the evaluator or consider other libraries or techniques."
A quick look and where tryConvertExpr()
is called seems to indicate the returned Function()
is dynamically called on the 3D Scene. (Not really sure tho)
The code above would not be very performant but might be good enough. It depends. Perhaps someone who understands the use case can comment. (Give the age of the codebase I am not very hopeful)
If something like the above is used then we only want to create the ast when the new function is created and not every time it's called.
function evaluateConvertExpr(node, vars) {
switch (node.type) {
case "Literal": return node.value;
case "Identifier": return vars[node.name];
case "BinaryExpression": {
const left = evaluateConvertExpr(node.left, vars);
const right = evaluateConvertExpr(node.right, vars);
switch (node.operator) {
case "+": return left + right;
case "-": return left - right;
case "*": return left * right;
case "/": return left / right;
default: throw new Error(`Unsupported operator: ${node.operator}`);
}
}
default: throw new Error(`Unsupported node type: ${node.type}`);
}
}
function tryConvertExpr(string) {
const exprRes = /^expr\((.*)\)$/.exec(string);
if (exprRes) {
try {
// Using the Esprima library to parse the expression into an AST.
// You will need to include the Esprima library for this to work.
const ast = esprima.parse(exprRes[1]).body[0].expression;
return function(width, height, dpr) {
return evaluateConvertExpr(ast, { width, height, dpr });
};
} catch (e) {
throw new Error("Invalid expression.");
}
}
}
Really needs the spec / docs for what is allowed in the expression string. If ONLY "width, height, dpr" are allowed in the expression then the above works fine.
Also an approach like the following libs use may be feasible:
The two functions that call tryConvertExpr are:
function createSizeSetHandler(name, exprFunc) {
return function (renderer) {
// PENDING viewport size or window size
var dpr = renderer.getDevicePixelRatio();
// PENDING If multiply dpr ?
var width = renderer.getWidth();
var height = renderer.getHeight();
var result = exprFunc(width, height, dpr);
this.setParameter(name, result);
};
}
function createSizeParser(name, exprFunc, scale) {
scale = scale || 1;
return function (renderer) {
var dpr = renderer.getDevicePixelRatio();
var width = renderer.getWidth() * scale;
var height = renderer.getHeight() * scale;
return exprFunc(width, height, dpr);
};
}
both of which only pass in the width, height, and dpr into the generated function.
For my simple use case (scatterGL), here were the unique expressions generated:
[width * 1.0 / 16, height * 1.0 / 16]
[width * 1.0 / 16, height / 16]
[width * 1.0 / 2, height * 1.0 / 2]
[width * 1.0 / 2, height / 2]
[width * 1.0 / 2, height / 2]
[width * 1.0 / 32, height / 32]
[width * 1.0 / 4, height * 1.0 / 4]
[width * 1.0 / 4, height / 4]
[width * 1.0 / 8, height * 1.0 / 8]
[width * 1.0 / 8, height / 8]
[width * 1.0, height * 1.0]
height * 1.0
height * 1.0 / 16
height * 1.0 / 2
height * 1.0 / 32
height * 1.0 / 4
height * 1.0 / 8
width * 1.0
width * 1.0 / 16
width * 1.0 / 2
width * 1.0 / 32
width * 1.0 / 4
width * 1.0 / 8
Not sure if the two types shown here (single arithmetic expression and array of arithmetic expressions) is exhaustive.
Took a shot at a PR, tested locally to ensure it produces the exact same output / results as the existing tryConvertExpr function.
Took a shot at a PR, tested locally to ensure it produces the exact same output / results as the existing tryConvertExpr function.
Good day, the problem is still relevant. How did you solve this problem in your project? If the PR is not accepted Thanks in advance
I am using HTTP Header: "Content-Security-Policy" without script-src "unsafe-eval" (See unsafe_eval_expressions)
claygl is incompatible with theses settings due to the function "tryConvertExpr(string)"
It uses
Function()
which is disallowed unless using "unsafe-eval"