From e9a8baf95ed969f0e43f4731da689ea39c0400ea Mon Sep 17 00:00:00 2001 From: Samuel Degueldre Date: Tue, 27 Feb 2024 17:40:12 +0100 Subject: [PATCH] [REF] web: simplify confusing curried functions in pyjs Previously, methods on python objects such as sets and strings were implemented as functions returning a function returning the result. While curried functions can be a great tool, they're also confusing at first glance and in this particular case they can be easily replaced by a simple function and the usage of Function.prototype.bind to bind it to the correct argument. We also add a simple whitelist for pyjs functions as a sanity check as it makes no sense to call JS functions from python code. With this change, we can also treat functions and methods from any object more uniformly instead of having many return points that are treated like special cases. task-3762958 Part-of: odoo/odoo#139451 Signed-off-by: Aaron Bohy (aab) --- .../static/src/core/py_js/py_interpreter.js | 182 ++++++++++-------- .../tests/core/py_js/py_interpreter_tests.js | 10 +- 2 files changed, 111 insertions(+), 81 deletions(-) diff --git a/addons/web/static/src/core/py_js/py_interpreter.js b/addons/web/static/src/core/py_js/py_interpreter.js index 47fcf93d831..a0096223c0e 100644 --- a/addons/web/static/src/core/py_js/py_interpreter.js +++ b/addons/web/static/src/core/py_js/py_interpreter.js @@ -249,25 +249,23 @@ function applyBinaryOp(ast, context) { } const DICT = { - get(dict) { - return (...args) => { - const { key, defValue } = parseArgs(args, ["key", "defValue"]); - if (key in dict) { - return dict[key]; - } else if (defValue) { - return defValue; - } - return null; - }; + get(...args) { + const { key, defValue } = parseArgs(args, ["key", "defValue"]); + if (key in this) { + return this[key]; + } else if (defValue) { + return defValue; + } + return null; }, }; const STRING = { - lower(str) { - return () => str.toLowerCase(); + lower() { + return this.toLowerCase(); }, - upper(str) { - return () => str.toUpperCase(); + upper() { + return this.toUpperCase(); }, }; @@ -285,54 +283,48 @@ function applyFunc(key, func, set, ...args) { } const SET = { - intersection(set) { - return (...args) => { - return applyFunc( - "intersection", - (iterable) => { - const intersection = new Set(); - for (const i of iterable) { - if (set.has(i)) { - intersection.add(i); - } + intersection(...args) { + return applyFunc( + "intersection", + (iterable) => { + const intersection = new Set(); + for (const i of iterable) { + if (this.has(i)) { + intersection.add(i); } - return intersection; - }, - set, - ...args - ); - }; + } + return intersection; + }, + this, + ...args + ); }, - difference(set) { - return (...args) => { - return applyFunc( - "difference", - (iterable) => { - iterable = new Set(iterable); - const difference = new Set(); - for (const e of set) { - if (!iterable.has(e)) { - difference.add(e); - } + difference(...args) { + return applyFunc( + "difference", + (iterable) => { + iterable = new Set(iterable); + const difference = new Set(); + for (const e of this) { + if (!iterable.has(e)) { + difference.add(e); } - return difference; - }, - set, - ...args - ); - }; + } + return difference; + }, + this, + ...args + ); }, - union(set) { - return (...args) => { - return applyFunc( - "union", - (iterable) => { - return new Set([...set, ...iterable]); - }, - set, - ...args - ); - }; + union(...args) { + return applyFunc( + "union", + (iterable) => { + return new Set([...this, ...iterable]); + }, + this, + ...args + ); }, }; @@ -340,6 +332,32 @@ const SET = { // Evaluate function // ----------------------------------------------------------------------------- +/** + * @param {Function} _class the class whose methods we want + * @returns {Function[]} an array containing the methods defined on the class, + * including the constructor + */ +function methods(_class) { + return Object.getOwnPropertyNames(_class.prototype).map((prop) => _class.prototype[prop]); +} + +const allowedFns = new Set([ + BUILTINS.time.strftime, + BUILTINS.set, + BUILTINS.bool, + BUILTINS.context_today, + BUILTINS.datetime.datetime.now, + BUILTINS.datetime.datetime.combine, + BUILTINS.datetime.date.today, + ...methods(BUILTINS.relativedelta), + ...Object.values(BUILTINS.datetime).flatMap((obj) => methods(obj)), + ...Object.values(SET), + ...Object.values(DICT), + ...Object.values(STRING), +]); + +const unboundFn = Symbol("unbound function"); + /** * @param {AST} ast * @param {Object} context @@ -360,10 +378,7 @@ export function evaluate(ast, context = {}) { }); } - /** - * @param {AST} ast - */ - function _evaluate(ast) { + function _innerEvaluate(ast) { switch (ast.type) { case 0 /* Number */: case 1 /* String */: @@ -434,28 +449,43 @@ export function evaluate(ast, context = {}) { } } case 15 /* ObjLookup */: { - const left = _evaluate(ast.obj); + let left = _evaluate(ast.obj); + let result; if (dicts.has(left) || Object.isPrototypeOf.call(PY_DICT, left)) { // this is a dictionary => need to apply dict methods - return DICT[ast.key](left); + result = DICT[ast.key]; + } else if (typeof left === "string") { + result = STRING[ast.key]; + } else if (left instanceof Set) { + result = SET[ast.key]; + } else if (ast.key == "get" && typeof left === "object") { + result = DICT[ast.key]; + left = toPyDict(left); + } else { + result = left[ast.key]; } - if (typeof left === "string") { - return STRING[ast.key](left); - } - if (left instanceof Set) { - return SET[ast.key](left); - } - if (ast.key == "get" && typeof left === "object") { - return DICT[ast.key](toPyDict(left)); - } - const result = left[ast.key]; - if (typeof result === "function" && !isConstructor(result)) { - return result.bind(left); + if (typeof result === "function") { + if (!isConstructor(result)) { + const bound = result.bind(left); + bound[unboundFn] = result; + return bound; + } } return result; } } throw new EvaluationError(`AST of type ${ast.type} cannot be evaluated`); } + + /** + * @param {AST} ast + */ + function _evaluate(ast) { + const val = _innerEvaluate(ast); + if (typeof val === "function" && !allowedFns.has(val) && !allowedFns.has(val[unboundFn])) { + throw new Error("Invalid Function Call"); + } + return val; + } return _evaluate(ast); } diff --git a/addons/web/static/tests/core/py_js/py_interpreter_tests.js b/addons/web/static/tests/core/py_js/py_interpreter_tests.js index bed702dcf6d..e2218ffe8d9 100644 --- a/addons/web/static/tests/core/py_js/py_interpreter_tests.js +++ b/addons/web/static/tests/core/py_js/py_interpreter_tests.js @@ -312,9 +312,9 @@ QUnit.module("py", {}, () => { QUnit.module("callables"); - QUnit.test("should call function from context", (assert) => { - assert.strictEqual(evaluateExpr("foo()", { foo: () => 3 }), 3); - assert.strictEqual(evaluateExpr("1 + foo()", { foo: () => 3 }), 4); + QUnit.test("should not call function from context", (assert) => { + assert.throws(() => evaluateExpr("foo()", { foo: () => 3 })); + assert.throws(() => evaluateExpr("1 + foo()", { foo: () => 3 })); }); QUnit.module("dicts"); @@ -359,8 +359,8 @@ QUnit.module("py", {}, () => { assert.strictEqual(evaluateExpr("obj.a.b.c", { obj: { a: { b: { c: 321 } } } }), 321); }); - QUnit.test("can call function in object", (assert) => { - assert.strictEqual(evaluateExpr("obj.f(3)", { obj: { f: (n) => n + 1 } }), 4); + QUnit.test("cannot call function in object", (assert) => { + assert.throws(() => evaluateExpr("obj.f(3)", { obj: { f: (n) => n + 1 } })); }); QUnit.module("if expressions");