details: https://hg.nginx.org/njs/rev/ee5fa0d312df
branches:
changeset: 1341:ee5fa0d312df
user: hongzhidao <hongzhidao@gmail.com>
date: Thu Jan 30 21:14:30 2020 +0800
description:
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.

diffstat:

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(-)

diffs (176 lines):

diff -r 3313d0d593a0 -r ee5fa0d312df src/njs_module.c
--- a/src/njs_module.c Thu Feb 27 14:30:14 2020 +0300
+++ b/src/njs_module.c Thu Jan 30 21:14:30 2020 +0800
@@ -113,7 +113,7 @@ njs_parser_module(njs_vm_t *vm, njs_pars
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_pars
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_pars
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 nj
}

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 nj

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 -r 3313d0d593a0 -r ee5fa0d312df test/module/lib1.js
--- a/test/module/lib1.js Thu Feb 27 14:30:14 2020 +0300
+++ b/test/module/lib1.js Thu Jan 30 21:14:30 2020 +0800
@@ -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 -r 3313d0d593a0 -r ee5fa0d312df test/module/libs/hash.js
--- a/test/module/libs/hash.js Thu Feb 27 14:30:14 2020 +0300
+++ b/test/module/libs/hash.js Thu Jan 30 21:14:30 2020 +0800
@@ -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 -r 3313d0d593a0 -r ee5fa0d312df test/module/libs/name.js
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/test/module/libs/name.js Thu Jan 30 21:14:30 2020 +0800
@@ -0,0 +1,1 @@
+export default 'libs.name';
diff -r 3313d0d593a0 -r ee5fa0d312df test/module/name.js
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/test/module/name.js Thu Jan 30 21:14:30 2020 +0800
@@ -0,0 +1,1 @@
+export default 'name';
diff -r 3313d0d593a0 -r ee5fa0d312df test/module/normal.js
--- a/test/module/normal.js Thu Feb 27 14:30:14 2020 +0300
+++ b/test/module/normal.js Thu Jan 30 21:14:30 2020 +0800
@@ -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 -r 3313d0d593a0 -r ee5fa0d312df test/module/recursive.js
--- a/test/module/recursive.js Thu Feb 27 14:30:14 2020 +0300
+++ b/test/module/recursive.js Thu Jan 30 21:14:30 2020 +0800
@@ -1,3 +1,1 @@
-
-
-import lib from './recursive.js';
+import lib from 'recursive.js';
diff -r 3313d0d593a0 -r ee5fa0d312df test/njs_expect_test.exp
--- a/test/njs_expect_test.exp Thu Feb 27 14:30:14 2020 +0300
+++ b/test/njs_expect_test.exp Thu Jan 30 21:14:30 2020 +0800
@@ -757,13 +757,13 @@ njs_run {"-p" "test/module" "-p" "test/m
"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!"
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel