njs icon indicating copy to clipboard operation
njs copied to clipboard

Function.name is broken – doesn’s return name of the function

Open jirutka opened this issue 5 years ago • 1 comments

>> function foo () {}
>> foo.name
''  // should be: 'foo'
>> var foo = function () {}
>> foo.name
''  // should be: 'foo'
>> var foo = () => {}
>> foo.name
''  // should be: 'foo'
>> (function () {}).name
''  // this is correct
>> (() => {}).name
''  // this is correct

Reference:

Environment:

  • njs 0.4.4 (cli)
  • Alpine Linux

jirutka avatar Nov 28 '20 11:11 jirutka

Also affected:

> var o = { [Symbol.iterator]: () => {}, method: () => {}, ['Yeah' + '!!!']() {} }; [o[Symbol.iterator].name, o.method.name, o['Yeah!!!'].name];
[ '[Symbol.iterator]', 'method', 'Yeah!!!' ]

Maybe it's worth to add a third arg to FUNCTION vmcode, which points to a string/symbol value.

POC:

diff --git a/src/njs_generator.c b/src/njs_generator.c
index 1730986..40e0531 100644
--- a/src/njs_generator.c
+++ b/src/njs_generator.c
@@ -2059,6 +2059,8 @@ njs_generate_function(njs_vm_t *vm, njs_generator_t *generator,
 
     function->retval = node->index;
 
+    function->name = (njs_index_t) &njs_string_empty;
+
     return NJS_OK;
 }
 
diff --git a/src/njs_vmcode.c b/src/njs_vmcode.c
index 31b4239..ee1948a 100644
--- a/src/njs_vmcode.c
+++ b/src/njs_vmcode.c
@@ -1004,7 +1004,11 @@ njs_vmcode_array(njs_vm_t *vm, u_char *pc)
 static njs_jump_off_t
 njs_vmcode_function(njs_vm_t *vm, u_char *pc)
 {
+    njs_int_t              ret;
     njs_function_t         *function;
+    const njs_value_t      *name;
+    njs_object_prop_t      *prop;
+    njs_lvlhsh_query_t     lhq;
     njs_function_lambda_t  *lambda;
     njs_vmcode_function_t  *code;
 
@@ -1019,6 +1023,28 @@ njs_vmcode_function(njs_vm_t *vm, u_char *pc)
 
     function->args_count = lambda->nargs - lambda->rest_parameters;
 
+    name = njs_vmcode_operand(vm, code->name);
+
+    prop = njs_object_prop_alloc(vm, &njs_string_name, name, 0);
+    if (njs_slow_path(prop == NULL)) {
+        return NJS_ERROR;
+    }
+    prop->configurable = 1;
+
+    lhq.proto = &njs_object_hash_proto;
+    lhq.pool = vm->mem_pool;
+    lhq.replace = 0;
+
+    njs_string_get(&njs_string_name, &lhq.key);
+    lhq.key_hash = NJS_NAME_HASH;
+    lhq.value = prop;
+
+    ret = njs_lvlhsh_insert(&function->object.hash, &lhq);
+    if (njs_slow_path(ret != NJS_OK)) {
+        njs_internal_error(vm, "lvlhsh insert failed");
+        return NJS_ERROR;
+    }
+
     njs_set_function(&vm->retval, function);
 
     return sizeof(njs_vmcode_function_t);
diff --git a/src/njs_vmcode.h b/src/njs_vmcode.h
index 9f1dd20..0e4773f 100644
--- a/src/njs_vmcode.h
+++ b/src/njs_vmcode.h
@@ -202,6 +202,7 @@ typedef struct {
     njs_vmcode_t               code;
     njs_index_t                retval;
     njs_function_lambda_t      *lambda;
+    njs_index_t                name;
 } njs_vmcode_function_t;
 
 
diff --git a/src/test/njs_unit_test.c b/src/test/njs_unit_test.c
index 4b8b505..c001e53 100644
--- a/src/test/njs_unit_test.c
+++ b/src/test/njs_unit_test.c
@@ -12733,6 +12733,23 @@ static njs_unit_test_t  njs_test[] =
     { njs_str("this.NN = {}; var f = Function('eval = 42;'); f()"),
       njs_str("SyntaxError: Identifier \"eval\" is forbidden as left-hand in assignment in runtime:1") },
 
+    /*
+      Function.name
+     */
+
+    { njs_str("var d = Object.getOwnPropertyDescriptor(function () {}, 'name');"
+              "[d.writable, d.enumerable, d.configurable, d.value, typeof d.value]"),
+      njs_str("false,false,true,,string") },
+
+    { njs_str("var d = Object.getOwnPropertyDescriptor(() => {}, 'name');"
+              "[d.writable, d.enumerable, d.configurable, d.value, typeof d.value]"),
+      njs_str("false,false,true,,string") },
+
+    { njs_str("var id = (x) => x;"
+              "var d = Object.getOwnPropertyDescriptor(id(() => {}), 'name');"
+              "[d.writable, d.enumerable, d.configurable, d.value, typeof d.value]"),
+      njs_str("false,false,true,,string") },
+
     { njs_str("RegExp()"),
       njs_str("/(?:)/") },
 
@@ -13921,7 +13938,7 @@ static njs_unit_test_t  njs_test[] =
       njs_str("0,1,2,length") },
 
     { njs_str("Object.getOwnPropertyNames(function() {})"),
-      njs_str("length,prototype") },
+      njs_str("name,length,prototype") },
 
     { njs_str("Object.getOwnPropertyNames(Array)"),
       njs_str("name,length,prototype,isArray,of") },

drsm avatar Nov 29 '20 10:11 drsm

@drsm fixed all the cases, including ComputedPropertyName. Also the committed patch avoids runtime overhead when "name" is not used in most cases.

xeioex avatar Sep 21 '22 00:09 xeioex