Fix Object.defineProperty() with reentrant descriptors

Previously, njs_object_prop_define() retained a pointer to a target hash
property while descriptor getters executed user code.  Reentrant mutation of
the target could resize the flat hash and leave the retained pointer dangling.
Descriptor getters were also evaluated again after fast-array conversion.

The fix is to convert the descriptor before querying the target and to place
the retry point after descriptor conversion.

The issue was introduced in 2f288a9c (0.9.1).

This closes #1089 issue on GitHub.

diff --git a/src/njs_object_prop.c b/src/njs_object_prop.c
index aeaeb71ba721d938cf8072372b2351e275aaa713..aaf8b683fc3cbad982d231916d671ba26c8786be 100644 (file)
--- a/src/njs_object_prop.c
+++ b/src/njs_object_prop.c
@@ -139,24 +139,13 @@ njs_object_prop_define(njs_vm_t *vm, njs_value_t *object, unsigned atom_id,
     njs_array_t           *array;
     njs_value_t           key, retval;
     njs_object_prop_t     _prop;
-    njs_object_prop_t     *prop = &_prop, *prev, *obj_prop;
+    njs_object_prop_t     *prop, *prev, *obj_prop;
     njs_property_query_t  pq;
 
-again:
-
     set_enumerable = 1;
     set_configurable = 1;
     set_writable = 1;
-
-    njs_property_query_init(&pq, NJS_PROPERTY_QUERY_SET, 1);
-
-    ret = (flags & NJS_OBJECT_PROP_CREATE)
-                ? NJS_DECLINED
-                : njs_property_query(vm, &pq, object, atom_id);
-
-    if (njs_slow_path(ret == NJS_ERROR)) {
-        return ret;
-    }
+    prop = &_prop;
 
     switch (njs_prop_type(flags)) {
     case NJS_OBJECT_PROP_DESCRIPTOR:
@@ -208,6 +197,18 @@ again:
         break;
     }
 
+again:
+
+    njs_property_query_init(&pq, NJS_PROPERTY_QUERY_SET, 1);
+
+    ret = (flags & NJS_OBJECT_PROP_CREATE)
+                ? NJS_DECLINED
+                : njs_property_query(vm, &pq, object, atom_id);
+
+    if (njs_slow_path(ret == NJS_ERROR)) {
+        return ret;
+    }
+
     if (njs_fast_path(ret == NJS_DECLINED)) {
 
 set_prop:

diff --git a/src/test/njs_unit_test.c b/src/test/njs_unit_test.c
index 3fc8690881491de4b710d21d8e2d7b4edbe22237..9d5d8b6f4324cc34f739b62aefb39b4dd887ac05 100644 (file)
--- a/src/test/njs_unit_test.c
+++ b/src/test/njs_unit_test.c
@@ -16002,6 +16002,18 @@ static njs_unit_test_t  njs_test[] =
               "njs.dump(arr)"),
       njs_str("[1,'[Getter]']") },
 
+    { njs_str("var a = [1], n = 0;"
+              "Object.defineProperty(a, 0, {"
+              "  get get() { n++; a.unshift(0); return function() {}; }});"
+              "n"),
+      njs_str("1") },
+
+    { njs_str("var a = [1], n = 0;"
+              "Object.defineProperty(a, 'length', {"
+              "  get writable() { n++; return false; }});"
+              "n"),
+      njs_str("1") },
+
     { njs_str("Object.defineProperties()"),
       njs_str("TypeError: Object.defineProperties is called on non-object") },