Browse Source

LibJS: Evaluate function arguments before checking callee type

In the spec, this happens in the EvaluateCall abstract operation
(https://tc39.es/ecma262/#sec-evaluatecall), and the order is defined
as:

    3. Let argList be ? ArgumentListEvaluation of arguments.
    4. If Type(func) is not Object, throw a TypeError exception.
    5. If IsCallable(func) is false, throw a TypeError exception.

In LibJS this is handled by CallExpression::execute(), which had the
callee function check first and would therefore never evaluate the
arguments for a non-function callee.
Linus Groh 3 years ago
parent
commit
99f9609

+ 5 - 5
Userland/Libraries/LibJS/AST.cpp

@@ -239,16 +239,16 @@ Value CallExpression::execute(Interpreter& interpreter, GlobalObject& global_obj
 
 
     VERIFY(!callee.is_empty());
     VERIFY(!callee.is_empty());
 
 
-    if (!callee.is_function()) {
-        throw_type_error_for_callee(interpreter, global_object, callee, "function"sv);
-        return {};
-    }
-
     MarkedValueList arg_list(vm.heap());
     MarkedValueList arg_list(vm.heap());
     argument_list_evaluation(interpreter, global_object, m_arguments, arg_list);
     argument_list_evaluation(interpreter, global_object, m_arguments, arg_list);
     if (interpreter.exception())
     if (interpreter.exception())
         return {};
         return {};
 
 
+    if (!callee.is_function()) {
+        throw_type_error_for_callee(interpreter, global_object, callee, "function"sv);
+        return {};
+    }
+
     auto& function = callee.as_function();
     auto& function = callee.as_function();
 
 
     if (is<Identifier>(*m_callee) && static_cast<Identifier const&>(*m_callee).string() == vm.names.eval.as_string() && &function == global_object.eval_function()) {
     if (is<Identifier>(*m_callee) && static_cast<Identifier const&>(*m_callee).string() == vm.names.eval.as_string() && &function == global_object.eval_function()) {

+ 30 - 0
Userland/Libraries/LibJS/Tests/functions/function-evaluation-order.js

@@ -0,0 +1,30 @@
+test("callee is evaluated before arguments", () => {
+    function foo() {}
+    const values = [];
+
+    [foo][(values.push("callee"), 0)](values.push("args"));
+
+    expect(values).toEqual(["callee", "args"]);
+});
+
+test("arguments are evaluated in order", () => {
+    function foo() {}
+    const values = [];
+
+    foo(values.push("arg1"), values.push("arg2"), values.push("arg3"));
+
+    expect(values).toEqual(["arg1", "arg2", "arg3"]);
+});
+
+test("arguments are evaluated before callee is checked for its type", () => {
+    const values = [];
+
+    expect(() => {
+        "foo"(values.push("args"));
+    }).toThrowWithMessage(TypeError, "foo is not a function");
+    expect(values).toEqual(["args"]);
+
+    expect(() => {
+        "foo"(bar);
+    }).toThrowWithMessage(ReferenceError, "'bar' is not defined");
+});