[njs] Fixed Array.prototype.reverse() when array is changed while iterating.

[Dmitry Volyntsev]

details:   https://hg.nginx.org/njs/rev/7467158d9f37
branches:  
changeset: 1807:7467158d9f37
user:      Dmitry Volyntsev <xeioex at nginx.com>
date:      Fri Jan 14 14:40:27 2022 +0000
description:
Fixed Array.prototype.reverse() when array is changed while iterating.

Previously, the flat array may be converted to a slow one as a
side-effect of a custom getter invocation for a proto array object.
The function erroneously assumed that the this array remains flat
while iterating.

The fix is to eliminate the micro-optimization which uses direct
pointers.

The problem is similar to the previous commits.

diffstat:

 src/njs_array.c          |  47 -----------------------------------------------
 src/test/njs_unit_test.c |  12 ++++++++++++
 2 files changed, 12 insertions(+), 47 deletions(-)

diffs (86 lines):

diff -r dfbde7620ced -r 7467158d9f37 src/njs_array.c
--- a/src/njs_array.c	Thu Jan 13 18:30:31 2022 +0000
+++ b/src/njs_array.c	Fri Jan 14 14:40:27 2022 +0000
@@ -1367,7 +1367,6 @@ njs_array_prototype_reverse(njs_vm_t *vm
     int64_t      length, l, h;
     njs_int_t    ret, lret, hret;
     njs_value_t  value, lvalue, hvalue, *this;
-    njs_array_t  *array;
 
     this = njs_argument(args, 0);
 
@@ -1386,52 +1385,6 @@ njs_array_prototype_reverse(njs_vm_t *vm
         return NJS_OK;
     }
 
-    if (njs_is_fast_array(this)) {
-        array = njs_array(this);
-
-        for (l = 0, h = length - 1; l < h; l++, h--) {
-            if (njs_fast_path(njs_is_valid(&array->start[l]))) {
-                lvalue = array->start[l];
-                lret = NJS_OK;
-
-            } else {
-                lret = njs_value_property_i64(vm, this, l, &lvalue);
-                if (njs_slow_path(lret == NJS_ERROR)) {
-                    return NJS_ERROR;
-                }
-            }
-
-            if (njs_fast_path(njs_is_valid(&array->start[h]))) {
-                hvalue = array->start[h];
-                hret = NJS_OK;
-
-            } else {
-                hret = njs_value_property_i64(vm, this, h, &hvalue);
-                if (njs_slow_path(hret == NJS_ERROR)) {
-                    return NJS_ERROR;
-                }
-            }
-
-            if (lret == NJS_OK) {
-                array->start[h] = lvalue;
-
-                if (hret == NJS_OK) {
-                    array->start[l] = hvalue;
-
-                } else {
-                    array->start[l] = njs_value_invalid;
-                }
-
-            } else if (hret == NJS_OK) {
-                array->start[l] = hvalue;
-                array->start[h] = njs_value_invalid;
-            }
-        }
-
-        njs_set_array(&vm->retval, array);
-        return NJS_OK;
-    }
-
     for (l = 0, h = length - 1; l < h; l++, h--) {
         lret = njs_value_property_i64(vm, this, l, &lvalue);
         if (njs_slow_path(lret == NJS_ERROR)) {
diff -r dfbde7620ced -r 7467158d9f37 src/test/njs_unit_test.c
--- a/src/test/njs_unit_test.c	Thu Jan 13 18:30:31 2022 +0000
+++ b/src/test/njs_unit_test.c	Fri Jan 14 14:40:27 2022 +0000
@@ -4883,6 +4883,18 @@ static njs_unit_test_t  njs_test[] =
     { njs_str("[,,,3,2,1].reverse()"),
       njs_str("1,2,3,,,") },
 
+    { njs_str("var a = [,,2,1];"
+              "Object.defineProperty(a.__proto__, 0, {"
+              "    get: () => {"
+              "        a.length = 10**6;"
+              "        return 4;"
+              "    },"
+              "    set: (setval) => { Object.defineProperty(a, 0, { value: setval }); },"
+              "});"
+              "a.reverse();"
+              "a.slice(0, 4)"),
+      njs_str("1,2,,4") },
+
     { njs_str("var o = {1:true, 2:'', length:-2}; Array.prototype.reverse.call(o) === o"),
       njs_str("true") },