Segfaults are fixed when non-existent external object property was

accessed or when existent property does not support operation.

diff --git a/nginx/ngx_http_js_module.c b/nginx/ngx_http_js_module.c
index 237971d7a9c30713457a7d86e600e1902248d3f3..591c20ad10362003db046ca1750ac80ecc39ffd7 100644 (file)
--- a/nginx/ngx_http_js_module.c
+++ b/nginx/ngx_http_js_module.c
@@ -52,8 +52,6 @@ static void *ngx_http_js_calloc(void *mem, size_t size);
 static void *ngx_http_js_memalign(void *mem, size_t alignment, size_t size);
 static void ngx_http_js_free(void *mem, void *p);
 
-static njs_ret_t ngx_http_js_ext_undefined(njs_vm_t *vm, njs_value_t *value,
-    void *obj, uintptr_t data);
 static njs_ret_t ngx_http_js_ext_get_string(njs_vm_t *vm, njs_value_t *value,
     void *obj, uintptr_t data);
 static njs_ret_t ngx_http_js_ext_set_string(njs_vm_t *vm, void *obj,
@@ -260,7 +258,7 @@ static njs_external_t  ngx_http_js_ext_request[] = {
       NJS_EXTERN_OBJECT,
       ngx_http_js_ext_response,
       nxt_nitems(ngx_http_js_ext_response),
-      ngx_http_js_ext_undefined,
+      NULL,
       NULL,
       NULL,
       NULL,
@@ -348,7 +346,7 @@ static njs_external_t  ngx_http_js_externals[] = {
       NJS_EXTERN_OBJECT,
       ngx_http_js_ext_request,
       nxt_nitems(ngx_http_js_ext_request),
-      ngx_http_js_ext_undefined,
+      NULL,
       NULL,
       NULL,
       NULL,
@@ -499,16 +497,6 @@ ngx_http_js_free(void *mem, void *p)
 }
 
 
-static njs_ret_t
-ngx_http_js_ext_undefined(njs_vm_t *vm, njs_value_t *value, void *obj,
-    uintptr_t data)
-{
-    njs_void_set(value);
-
-    return NJS_OK;
-}
-
-
 static njs_ret_t
 ngx_http_js_ext_get_string(njs_vm_t *vm, njs_value_t *value, void *obj,
     uintptr_t data)

diff --git a/njs/njs_vm.c b/njs/njs_vm.c
index 01626a65b80a98f31d6089957b00b979caba4a9a..f6c306464aeefb3ca872a0a47538ce7bd7a53d0e 100644 (file)
--- a/njs/njs_vm.c
+++ b/njs/njs_vm.c
@@ -545,12 +545,14 @@ njs_vmcode_property_get(njs_vm_t *vm, njs_value_t *object,
 
         vm->retval = njs_value_void;
 
-        ret = ext->get(vm, &vm->retval, vm->external[ext->object], data);
-        if (nxt_slow_path(ret != NXT_OK)) {
-            return ret;
-        }
+        if (ext->get != NULL) {
+            ret = ext->get(vm, &vm->retval, vm->external[ext->object], data);
+            if (nxt_slow_path(ret != NXT_OK)) {
+                return ret;
+            }
 
-        /* The vm->retval is already retained by ext->get(). */
+            /* The vm->retval is already retained by ext->get(). */
+        }
 
         return sizeof(njs_vmcode_prop_get_t);
 
@@ -636,16 +638,18 @@ njs_vmcode_property_set(njs_vm_t *vm, njs_value_t *object,
             data = (uintptr_t) &pq.lhq.key;
         }
 
-        ret = njs_value_to_ext_string(vm, &s, value);
-        if (nxt_slow_path(ret != NXT_OK)) {
-            return ret;
-        }
+        if (ext->set != NULL) {
+            ret = njs_value_to_ext_string(vm, &s, value);
+            if (nxt_slow_path(ret != NXT_OK)) {
+                return ret;
+            }
 
-        /* TODO retain value if it is string. */
+            /* TODO retain value if it is string. */
 
-        ret = ext->set(vm, vm->external[ext->object], data, &s);
-        if (nxt_slow_path(ret != NXT_OK)) {
-            return ret;
+            ret = ext->set(vm, vm->external[ext->object], data, &s);
+            if (nxt_slow_path(ret != NXT_OK)) {
+                return ret;
+            }
         }
 
         return sizeof(njs_vmcode_prop_set_t);
@@ -709,27 +713,22 @@ njs_vmcode_property_in(njs_vm_t *vm, njs_value_t *object, njs_value_t *property)
         ret = nxt_lvlhsh_find(&ext->hash, &pq.lhq);
 
         if (ret == NXT_OK) {
-            ext = pq.lhq.value;
-
-            if ((ext->type & NJS_EXTERN_OBJECT) != 0) {
-                retval = &njs_value_true;
-                break;
-            }
-
-            data = ext->data;
+            retval = &njs_value_true;
 
         } else {
             data = (uintptr_t) &pq.lhq.key;
-        }
 
-        ret = ext->find(vm, vm->external[ext->object], data, 0);
+            if (ext->find != NULL) {
+                ret = ext->find(vm, vm->external[ext->object], data, 0);
 
-        if (nxt_slow_path(ret == NXT_ERROR)) {
-            return ret;
-        }
+                if (nxt_slow_path(ret == NXT_ERROR)) {
+                    return ret;
+                }
 
-        if (ret == NXT_OK) {
-            retval = &njs_value_true;
+                if (ret == NXT_OK) {
+                    retval = &njs_value_true;
+                }
+            }
         }
 
         break;
@@ -803,23 +802,26 @@ njs_vmcode_property_delete(njs_vm_t *vm, njs_value_t *object,
             ext = pq.lhq.value;
 
             if ((ext->type & NJS_EXTERN_OBJECT) != 0) {
-                break;
-            }
+                data = (uintptr_t) &ext->value;
 
-            data = ext->data;
+            } else {
+                data = ext->data;
+            }
 
         } else {
             data = (uintptr_t) &pq.lhq.key;
         }
 
-        ret = ext->find(vm, vm->external[ext->object], data, 1);
+        if (ext->find != NULL) {
+            ret = ext->find(vm, vm->external[ext->object], data, 1);
 
-        if (nxt_slow_path(ret == NXT_ERROR)) {
-            return ret;
-        }
+            if (nxt_slow_path(ret == NXT_ERROR)) {
+                return ret;
+            }
 
-        if (ret == NXT_OK) {
-            retval = &njs_value_true;
+            if (ret == NXT_OK) {
+                retval = &njs_value_true;
+            }
         }
 
         break;

diff --git a/njs/test/njs_unit_test.c b/njs/test/njs_unit_test.c
index 2de1ae969d84dfb58011feed655d45f96f8f3821..bde2ed3d4c6f175781228d16a6df3bc16b4cde25 100644 (file)
--- a/njs/test/njs_unit_test.c
+++ b/njs/test/njs_unit_test.c
@@ -2452,6 +2452,21 @@ static njs_unit_test_t  njs_test[] =
     { nxt_string("$r.external('YES')"),
       nxt_string("АБВ") },
 
+    { nxt_string("for (p in $r.external);"),
+      nxt_string("undefined") },
+
+    { nxt_string("'uri' in $r"),
+      nxt_string("true") },
+
+    { nxt_string("'one' in $r"),
+      nxt_string("false") },
+
+    { nxt_string("delete $r.uri"),
+      nxt_string("false") },
+
+    { nxt_string("delete $r.one"),
+      nxt_string("false") },
+
 #if 0
     { nxt_string("$r.external.call($r, 'YES')"),
       nxt_string("АБВ") },
@@ -2463,6 +2478,9 @@ static njs_unit_test_t  njs_test[] =
     { nxt_string("$r.nonexistent"),
       nxt_string("undefined") },
 
+    { nxt_string("$r.error = 'OK'"),
+      nxt_string("OK") },
+
     { nxt_string("var a = { toString: function() { return 1 } }; a"),
       nxt_string("1") },
 
@@ -3630,16 +3648,6 @@ njs_unit_test_method_external(njs_vm_t *vm, njs_param_t *param)
 }
 
 
-static njs_ret_t
-njs_unit_test_undefined_external(njs_vm_t *vm, njs_value_t *value, void *obj,
-    uintptr_t data)
-{
-    njs_void_set(value);
-
-    return NJS_OK;
-}
-
-
 static njs_external_t  njs_unit_test_r_external[] = {
 
     { nxt_string("uri"),
@@ -3699,7 +3707,7 @@ static njs_external_t  nxt_test_external[] = {
       NJS_EXTERN_OBJECT,
       njs_unit_test_r_external,
       nxt_nitems(njs_unit_test_r_external),
-      njs_unit_test_undefined_external,
+      NULL,
       NULL,
       NULL,
       NULL,