From 34e4faf65a2ecd4051be6115759d319bbfc68c0e Mon Sep 17 00:00:00 2001 From: Igor Sysoev Date: Wed, 13 Jan 2016 19:11:36 +0300 Subject: [PATCH] Segfaults are fixed when non-existent external object property was accessed or when existent property does not support operation. --- nginx/ngx_http_js_module.c | 16 +------- njs/njs_vm.c | 76 +++++++++++++++++++------------------- njs/test/njs_unit_test.c | 30 +++++++++------ 3 files changed, 60 insertions(+), 62 deletions(-) diff --git a/nginx/ngx_http_js_module.c b/nginx/ngx_http_js_module.c index 237971d7..591c20ad 100644 --- 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 01626a65..f6c30646 100644 --- 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 2de1ae96..bde2ed3d 100644 --- 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, -- 2.47.3