From f846489fa9b36ca98435d6b8c853aec6f3fc17d9 Mon Sep 17 00:00:00 2001 From: hongzhidao Date: Thu, 30 Jan 2020 21:14:30 +0800 Subject: [PATCH] Fixed non-native module importing. Previously, string from "import statement" was used as a key in a hash to keep track of already loaded modules. This works fine for built-in modules. It can cause an issue when different modules from different paths but with identical names are loaded. This patch avoids the issue by using a full path to a file as a key. This closes #282 issue on GitHub. --- src/njs_module.c | 20 ++++++++++++++------ test/module/lib1.js | 3 ++- test/module/libs/hash.js | 3 ++- test/module/libs/name.js | 1 + test/module/name.js | 1 + test/module/normal.js | 10 ++++++++++ test/module/recursive.js | 4 +--- test/njs_expect_test.exp | 8 ++++---- 8 files changed, 35 insertions(+), 15 deletions(-) create mode 100644 test/module/libs/name.js create mode 100644 test/module/name.js diff --git a/src/njs_module.c b/src/njs_module.c index 1ea3cb86..3443b19e 100644 --- a/src/njs_module.c +++ b/src/njs_module.c @@ -113,7 +113,7 @@ njs_parser_module(njs_vm_t *vm, njs_parser_t *parser) parser->node = NULL; module = njs_module_find(vm, &name, 0); - if (module != NULL) { + if (module != NULL && module->function.native) { goto found; } @@ -138,18 +138,24 @@ njs_parser_module(njs_vm_t *vm, njs_parser_t *parser) goto fail; } + module = njs_module_find(vm, &info.file, 0); + if (module != NULL) { + (void) close(info.fd); + goto found; + } + ret = njs_module_read(vm, info.fd, &text); (void) close(info.fd); if (njs_slow_path(ret != NJS_OK)) { - njs_internal_error(vm, "while reading \"%V\" module", &name); + njs_internal_error(vm, "while reading \"%V\" module", &info.file); goto fail; } if (njs_module_realpath_equal(&prev->file, &info.file)) { njs_parser_syntax_error(vm, parser, "Cannot import itself \"%V\"", - &name); + &info.file); goto fail; } @@ -171,7 +177,7 @@ njs_parser_module(njs_vm_t *vm, njs_parser_t *parser) goto fail; } - module = njs_module_add(vm, &name); + module = njs_module_add(vm, &info.file); if (njs_slow_path(module == NULL)) { goto fail; } @@ -229,7 +235,8 @@ njs_module_lookup(njs_vm_t *vm, const njs_str_t *cwd, njs_module_info_t *info) } ret = njs_module_relative_path(vm, cwd, info); - if (ret == NJS_OK) { + + if (ret != NJS_DECLINED) { return ret; } @@ -241,7 +248,8 @@ njs_module_lookup(njs_vm_t *vm, const njs_str_t *cwd, njs_module_info_t *info) for (i = 0; i < vm->paths->items; i++) { ret = njs_module_relative_path(vm, path, info); - if (ret == NJS_OK) { + + if (ret != NJS_DECLINED) { return ret; } diff --git a/test/module/lib1.js b/test/module/lib1.js index f2909605..a5778173 100644 --- a/test/module/lib1.js +++ b/test/module/lib1.js @@ -10,6 +10,7 @@ function hash() { return v; } +import hashlib from 'hash.js'; import crypto from 'crypto'; var state = {count:0} @@ -22,4 +23,4 @@ function get() { return state.count; } -export default {hash, inc, get}; +export default {hash, inc, get, name: hashlib.name} diff --git a/test/module/libs/hash.js b/test/module/libs/hash.js index 87692286..85f91ef8 100644 --- a/test/module/libs/hash.js +++ b/test/module/libs/hash.js @@ -4,6 +4,7 @@ function hash() { return v; } +import name from 'name.js'; import crypto from 'crypto'; -export default {hash}; +export default {hash, name}; diff --git a/test/module/libs/name.js b/test/module/libs/name.js new file mode 100644 index 00000000..4c64d29f --- /dev/null +++ b/test/module/libs/name.js @@ -0,0 +1 @@ +export default 'libs.name'; diff --git a/test/module/name.js b/test/module/name.js new file mode 100644 index 00000000..bc8f1133 --- /dev/null +++ b/test/module/name.js @@ -0,0 +1 @@ +export default 'name'; diff --git a/test/module/normal.js b/test/module/normal.js index 675b391f..678da8c7 100644 --- a/test/module/normal.js +++ b/test/module/normal.js @@ -1,3 +1,4 @@ +import name from 'name.js'; import lib1 from 'lib1.js'; import lib2 from 'lib2.js'; import lib1_2 from 'lib1.js'; @@ -7,6 +8,15 @@ var h = crypto.createHash('md5'); var hash = h.update('AB').digest('hex'); var fails = 0; + +if (name != 'name') { + fails++; +} + +if (lib1.name != 'libs.name') { + fails++; +} + if (lib1.hash() != hash) { fails++; } diff --git a/test/module/recursive.js b/test/module/recursive.js index 86870971..cd08ec1b 100644 --- a/test/module/recursive.js +++ b/test/module/recursive.js @@ -1,3 +1 @@ - - -import lib from './recursive.js'; +import lib from 'recursive.js'; diff --git a/test/njs_expect_test.exp b/test/njs_expect_test.exp index 3ed1c961..fe167da3 100644 --- a/test/njs_expect_test.exp +++ b/test/njs_expect_test.exp @@ -757,13 +757,13 @@ njs_run {"-p" "test/module" "-p" "test/module/libs" "./test/module/normal.js"} \ "passed!" njs_run {"./test/module/normal.js"} \ - "SyntaxError: Cannot find module \"hash.js\" in sub2.js:5" + "SyntaxError: Cannot find module \"hash.js\" in lib1.js:13" njs_run {"-p" "test/module/libs" "./test/module/exception.js"} \ "at error \\(sub1.js:5\\)" njs_run {"-p" "test/module" "./test/module/recursive.js"} \ - "SyntaxError: Cannot import itself \"./recursive.js\" in recursive.js:3" + "SyntaxError: Cannot import itself \"./test/module/recursive.js\" in recursive.js:1" # CLI OPTIONS @@ -843,7 +843,7 @@ njs_test { "Error: loading exception\r\n at module \\(loading_exception.js:1\\)"} {"import lib3 from 'lib1.js'\r\n" "undefined\r\n"} -} "-p test/module/" +} "-p test/module/ -p test/module/libs/" njs_test { {"import m from 'export_name.js'\r\n" @@ -861,7 +861,7 @@ njs_test { } "-p test/module/" njs_run {"-q" "./test/module/normal.js"} \ - "SyntaxError: Cannot find module \"hash.js\" in 5" + "SyntaxError: Cannot find module \"hash.js\" in 13" njs_run {"-p" "test/module/libs/" "-d" "./test/module/normal.js"} \ "passed!" -- 2.47.3