details: https://hg.nginx.org/njs/rev/b79a22d6d7f3
branches:
changeset: 1132:b79a22d6d7f3
user: Dmitry Volyntsev <xeioex@nginx.com>
date: Thu Aug 15 19:22:01 2019 +0300
description:
Fixed module importing using require().

Previously, require() did not make a mutable copy of imported module.
As a result, cloned VMs could change a shared module object making
module.object->hash inconsistent.

Before 04d7a5d93ae6, the problem manifested itself only in "PROP GET"
operations, for example, using "require('fs').renameSync", because
njs_value_property() makes a mutable copy of shared methods in
module.object->hash.

After 04d7a5d93ae6, "METHOD CALL" operations, such as
"require('fs').renameSync()", now have the same problem as in "PROP
GET".

Importing modules using "import" statement is not affected, because
for it "OBJECT COPY" instruction is generated, which makes a mutable
copy of imported module object.

The fix is to make a mutable copy of a shared module in require().

This closes #206 issue on Github.

diffstat:

src/njs_module.c | 18 ++++++++++++---
src/test/njs_unit_test.c | 52 ++++++++++++++++++++++++++++++++++++++++-------
2 files changed, 58 insertions(+), 12 deletions(-)

diffs (134 lines):

diff -r 8b2f303ab48a -r b79a22d6d7f3 src/njs_module.c
--- a/src/njs_module.c Wed Aug 14 20:22:32 2019 +0300
+++ b/src/njs_module.c Thu Aug 15 19:22:01 2019 +0300
@@ -503,9 +503,10 @@ njs_module_insert(njs_vm_t *vm, njs_modu


njs_int_t
-njs_module_require(njs_vm_t *vm, njs_value_t *args,
- njs_uint_t nargs, njs_index_t unused)
+njs_module_require(njs_vm_t *vm, njs_value_t *args, njs_uint_t nargs,
+ njs_index_t unused)
{
+ njs_object_t *object;
njs_module_t *module;
njs_lvlhsh_query_t lhq;

@@ -520,9 +521,18 @@ njs_module_require(njs_vm_t *vm, njs_val

if (njs_lvlhsh_find(&vm->modules_hash, &lhq) == NJS_OK) {
module = lhq.value;
- module->object.__proto__ = &vm->prototypes[NJS_PROTOTYPE_OBJECT].object;

- njs_set_object(&vm->retval, &module->object);
+ object = njs_mp_alloc(vm->mem_pool, sizeof(njs_object_t));
+ if (njs_slow_path(object == NULL)) {
+ njs_memory_error(vm);
+ return NJS_ERROR;
+ }
+
+ *object = module->object;
+ object->__proto__ = &vm->prototypes[NJS_PROTOTYPE_OBJECT].object;
+ object->shared = 0;
+
+ njs_set_object(&vm->retval, object);

return NJS_OK;
}
diff -r 8b2f303ab48a -r b79a22d6d7f3 src/test/njs_unit_test.c
--- a/src/test/njs_unit_test.c Wed Aug 14 20:22:32 2019 +0300
+++ b/src/test/njs_unit_test.c Thu Aug 15 19:22:01 2019 +0300
@@ -13489,6 +13489,22 @@ static njs_unit_test_t njs_module_test[
};


+static njs_unit_test_t njs_shared_test[] =
+{
+ { njs_str("var cr = require('crypto'); cr.createHash"),
+ njs_str("[object Function]") },
+
+ { njs_str("var cr = require('crypto'); cr.createHash('md5')"),
+ njs_str("[object Hash]") },
+
+ { njs_str("import cr from 'crypto'; cr.createHash"),
+ njs_str("[object Function]") },
+
+ { njs_str("import cr from 'crypto'; cr.createHash('md5')"),
+ njs_str("[object Hash]") },
+};
+
+
static njs_unit_test_t njs_tz_test[] =
{
{ njs_str("var d = new Date(1); d = d + ''; d.slice(0, 33)"),
@@ -14278,6 +14294,7 @@ typedef struct {
njs_bool_t disassemble;
njs_bool_t verbose;
njs_bool_t module;
+ njs_uint_t repeat;
} njs_opts_t;


@@ -14308,7 +14325,7 @@ njs_unit_test(njs_unit_test_t tests[], s
njs_vm_t *vm, *nvm;
njs_int_t ret;
njs_str_t s;
- njs_uint_t i;
+ njs_uint_t i, repeat;
njs_stat_t prev;
njs_bool_t success;
njs_vm_opt_t options;
@@ -14350,13 +14367,21 @@ njs_unit_test(njs_unit_test_t tests[], s
njs_disassembler(vm);
}

- nvm = njs_vm_clone(vm, NULL);
- if (nvm == NULL) {
- njs_printf("njs_vm_clone() failed\n");
- goto done;
- }
-
- ret = njs_vm_start(nvm);
+ repeat = opts->repeat;
+
+ do {
+ if (nvm != NULL) {
+ njs_vm_destroy(nvm);
+ }
+
+ nvm = njs_vm_clone(vm, NULL);
+ if (nvm == NULL) {
+ njs_printf("njs_vm_clone() failed\n");
+ goto done;
+ }
+
+ ret = njs_vm_start(nvm);
+ } while (--repeat != 0);

if (njs_vm_retval_string(nvm, &s) != NJS_OK) {
njs_printf("njs_vm_retval_string() failed\n");
@@ -14844,6 +14869,8 @@ main(int argc, char **argv)

njs_memzero(&stat, sizeof(njs_stat_t));

+ opts.repeat = 1;
+
ret = njs_unit_test(njs_test, njs_nitems(njs_test), "script tests",
&opts, &stat);
if (ret != NJS_OK) {
@@ -14878,6 +14905,15 @@ main(int argc, char **argv)
return ret;
}

+ opts.module = 0;
+ opts.repeat = 128;
+
+ ret = njs_unit_test(njs_shared_test, njs_nitems(njs_shared_test),
+ "shared tests", &opts, &stat);
+ if (ret != NJS_OK) {
+ return ret;
+ }
+
njs_printf("TOTAL: %s [%ui/%ui]\n", stat.failed ? "FAILED" : "PASSED",
stat.passed, stat.passed + stat.failed);

_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel