[njs] Fixed type confusion bug while resolving promises.

Dmitry Volyntsev xeioex at nginx.com
Wed Jan 19 13:12:54 UTC 2022


details:   https://hg.nginx.org/njs/rev/c419a4e34998
branches:  
changeset: 1812:c419a4e34998
user:      Dmitry Volyntsev <xeioex at nginx.com>
date:      Wed Jan 19 13:12:09 2022 +0000
description:
Fixed type confusion bug while resolving promises.

Previously, the internal function njs_promise_perform_then() which
implements PerformPromiseThen() expects its first argument to always be
a promise instance.  This assertion might be invalid because the
functions corresponding to Promise.prototype.then() and
Promise.resolve() incorrectly verified their arguments.

Specifically, the functions recognized their first argument as promise
if it was an object which was an Promise or had Promise object in its
prototype chain.  The later condition is not correct because internal
slots are not inherited according to the spec.

This closes #447 issue in Github.

diffstat:

 src/njs_promise.c                                    |  33 ++++++-------------
 src/njs_vmcode.c                                     |   2 +-
 test/js/promise_prototype_reject_type_confusion.t.js |  11 ++++++
 test/js/promise_prototype_then_type_confusion.t.js   |  11 ++++++
 4 files changed, 34 insertions(+), 23 deletions(-)

diffs (109 lines):

diff -r 79b109076c13 -r c419a4e34998 src/njs_promise.c
--- a/src/njs_promise.c	Tue Jan 18 15:37:11 2022 +0000
+++ b/src/njs_promise.c	Wed Jan 19 13:12:09 2022 +0000
@@ -771,25 +771,19 @@ njs_promise_resolve(njs_vm_t *vm, njs_va
 {
     njs_int_t                 ret;
     njs_value_t               value;
-    njs_object_t              *object;
     njs_promise_capability_t  *capability;
 
     static const njs_value_t  string_constructor = njs_string("constructor");
 
-    if (njs_is_object(x)) {
-        object = njs_object_proto_lookup(njs_object(x), NJS_PROMISE,
-                                         njs_object_t);
+    if (njs_is_promise(x)) {
+        ret = njs_value_property(vm, x, njs_value_arg(&string_constructor),
+                                 &value);
+        if (njs_slow_path(ret == NJS_ERROR)) {
+            return NULL;
+        }
 
-        if (object != NULL) {
-            ret = njs_value_property(vm, x, njs_value_arg(&string_constructor),
-                                     &value);
-            if (njs_slow_path(ret == NJS_ERROR)) {
-                return NULL;
-            }
-
-            if (njs_values_same(&value, constructor)) {
-                return njs_promise(x);
-            }
+        if (njs_values_same(&value, constructor)) {
+            return njs_promise(x);
         }
     }
 
@@ -875,19 +869,12 @@ njs_promise_prototype_then(njs_vm_t *vm,
 {
     njs_int_t                 ret;
     njs_value_t               *promise, *fulfilled, *rejected, constructor;
-    njs_object_t              *object;
     njs_function_t            *function;
     njs_promise_capability_t  *capability;
 
     promise = njs_argument(args, 0);
 
-    if (njs_slow_path(!njs_is_object(promise))) {
-        goto failed;
-    }
-
-    object = njs_object_proto_lookup(njs_object(promise), NJS_PROMISE,
-                                     njs_object_t);
-    if (njs_slow_path(object == NULL)) {
+    if (njs_slow_path(!njs_is_promise(promise))) {
         goto failed;
     }
 
@@ -933,6 +920,8 @@ njs_promise_perform_then(njs_vm_t *vm, n
     njs_promise_data_t      *data;
     njs_promise_reaction_t  *fulfilled_reaction, *rejected_reaction;
 
+    njs_assert(njs_is_promise(value));
+
     if (!njs_is_function(fulfilled)) {
         fulfilled = njs_value_arg(&njs_value_undefined);
     }
diff -r 79b109076c13 -r c419a4e34998 src/njs_vmcode.c
--- a/src/njs_vmcode.c	Tue Jan 18 15:37:11 2022 +0000
+++ b/src/njs_vmcode.c	Wed Jan 19 13:12:09 2022 +0000
@@ -1895,7 +1895,7 @@ njs_vmcode_await(njs_vm_t *vm, njs_vmcod
     rejected->args_count = 1;
     rejected->u.native = njs_await_rejected;
 
-    njs_set_object(&val, &promise->object);
+    njs_set_promise(&val, promise);
     njs_set_function(&on_fulfilled, fulfilled);
     njs_set_function(&on_rejected, rejected);
 
diff -r 79b109076c13 -r c419a4e34998 test/js/promise_prototype_reject_type_confusion.t.js
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/js/promise_prototype_reject_type_confusion.t.js	Wed Jan 19 13:12:09 2022 +0000
@@ -0,0 +1,11 @@
+/*---
+includes: []
+flags: [async]
+---*/
+
+Symbol.__proto__ = new Promise(()=>{});
+
+Promise.reject(Symbol)
+.then(v => $DONOTEVALUATE())
+.catch(err => assert.sameValue(err.name, 'Symbol'))
+.then($DONE, $DONE);
diff -r 79b109076c13 -r c419a4e34998 test/js/promise_prototype_then_type_confusion.t.js
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/js/promise_prototype_then_type_confusion.t.js	Wed Jan 19 13:12:09 2022 +0000
@@ -0,0 +1,11 @@
+/*---
+includes: []
+flags: [async]
+---*/
+
+Symbol.__proto__ = new Promise(()=>{});
+
+Promise.resolve(Symbol)
+.then(v => $DONOTEVALUATE())
+.catch(err => assert.sameValue(err.name, 'TypeError'))
+.then($DONE, $DONE);



More information about the nginx-devel mailing list