Merge branch 'fix-module-resolve-bug'

Fix resolution of relative paths which were off by one component.

For instance, if current module ID was `foo/bar` and the module require()'d
`./quux`, this would be incorrectly resolved to `foo/bar/quux` instead of the
expected `foo/quux`.

Testcases were testing for the wrong behavior - they are also fixed by this
commit.  A specific bug testcase was also added for the particular issue
found by andoma.

RELEASES.rst

@@ -550,6 +550,9 @@ Planned
 * Rename RELEASES.txt, AUTHORS.txt, and README.txt files to .rst suffix for
   better automatic formatting
 
+* Fix require() resolution of relative module identifiers, which was off by
+  one component (see GH-48)
+
 * Fix DUK_INVALID_INDEX define value, it used INT_MIN directly
 
 * Fix return value of Duktape.gc() to return true (instead of false) for

ecmascript-testcases/test-bug-commonjs-relative-id.js

@@ -0,0 +1,56 @@
+/*
+ *  Relative resolution bug in Duktape 0.12.0
+ *
+ *  https://github.com/svaarala/duktape/issues/48
+ */
+
+/*---
+{
+    "custom": true
+}
+---*/
+
+/*===
+Duktape.modSearch foo
+Duktape.modSearch quux
+Duktape.modSearch foo/bar
+Duktape.modSearch foo/quux
+===*/
+
+function test() {
+    /*
+     *  Relative resolution from 'foo'
+     */
+
+    Duktape.modSearch = function (id) {
+        print('Duktape.modSearch', id);
+        if (id === 'foo') {
+            return "var text = 'Hello world!';     // not visible outside the module\n" +
+                   "var quux = require('./quux');  // loads quux\n" +
+                   "exports.hello = function () { print(text); };";
+        }
+        return '';   // return a fake empty module
+    };
+    void require('foo');
+
+    /*
+     *  Relative resolution from 'foo/bar'
+     */
+
+    Duktape.modSearch = function (id) {
+        print('Duktape.modSearch', id);
+        if (id === 'foo/bar') {
+            return "var text = 'Hello world!';     // not visible outside the module\n" +
+                   "var quux = require('./quux');  // loads foo/quux\n" +
+                   "exports.hello = function () { print(text); };";
+        }
+        return '';   // return a fake empty module
+    };
+    void require('foo/bar');
+}
+
+try {
+    test();
+} catch (e) {
+    print(e.stack || e);
+}

ecmascript-testcases/test-commonjs-require-environment.js

@@ -9,8 +9,10 @@
 
 /*===
 basic bindings
+modSearch: foo/bar
+modSearch: foo/quux
 require: function false
-module: object foo/bar false false false
+module: object foo/quux false false false
 exports: object true
 ===*/
 
@@ -19,10 +21,11 @@ exports: object true
 var global_require = require;
 
 Duktape.modSearch = function (id) {
-    if (id === 'foo') {
-        return 'var mod = require("./bar");\n'
-    }
+    print('modSearch:', id);
     if (id === 'foo/bar') {
+        return 'var mod = require("./quux");\n'
+    }
+    if (id === 'foo/quux') {
         return 'var pd;\n' +
                'print("require:", typeof require, require === global_require);\n' +
                'pd = Object.getOwnPropertyDescriptor(module, "id");\n' +
@@ -36,7 +39,7 @@ Duktape.modSearch = function (id) {
 print('basic bindings');
 
 function bindingTest() {
-    var mod = require('foo');
+    var mod = require('foo/bar');
 
     // module.id must be a resolved absolute path so that it can be used
     // to require the correct module from any other module

ecmascript-testcases/test-commonjs-require-resolution.js

@@ -61,8 +61,12 @@ global require: foo/mod1/. -> TypeError
 global require: foo/mod1/.. -> TypeError
 Duktape.modSearch baz
 Duktape.modSearch xxx
-Duktape.modSearch baz/xxx
-Duktape.modSearch baz/xxx/yyy
+Duktape.modSearch xxy
+Duktape.modSearch xxx/yyy
+Duktape.modSearch quux/foo
+Duktape.modSearch xxz
+Duktape.modSearch quux/xxw
+Duktape.modSearch quux/xxw/yyy
 Duktape.modSearch zzz
 Duktape.modSearch www
 ===*/
@@ -144,8 +148,25 @@ function basicResolutionTest() {
         print('Duktape.modSearch', id);
         if (id === 'baz') {
             return 'require("xxx");\n' +           // absolute
-                   'require("./xxx");\n' +         // relative
-                   'require("./xxx/yyy");\n' +     // relative
+                   'require("./xxy");\n' +         // relative
+                   'require("./xxx/yyy");\n'       // relative
+                   ;
+        }
+        return '';   // return a fake empty module
+    };
+
+    void require('baz');
+
+    /*
+     *  Require from inside a module with a few more path components.
+     */
+
+    Duktape.modSearch = function (id) {
+        print('Duktape.modSearch', id);
+        if (id === 'quux/foo') {
+            return 'require("xxz");\n' +           // absolute
+                   'require("./xxw");\n' +         // relative
+                   'require("./xxw/yyy");\n' +     // relative
                    'require("../zzz");\n' +        // relative
                    'require("././../www");\n'
                    ;
@@ -153,7 +174,7 @@ function basicResolutionTest() {
         return '';   // return a fake empty module
     };
 
-    void require('baz');
+    void require('quux/foo');
 }
 
 print('basic resolution');
@@ -260,14 +281,14 @@ Duktape.modSearch foo/bar
 268: TypeError
 269: TypeError
 270: TypeError
-230: foo/bar
-231: foo/bar
-232: foo/bar
-233: foo/bar
-234: foo/bar
-235: foo/bar
-236: foo/bar
-237: foo/bar
+230: bar
+231: bar
+232: bar
+233: bar
+234: bar
+235: TypeError
+236: TypeError
+237: TypeError
 238: TypeError
 239: TypeError
 240: TypeError
@@ -304,8 +325,8 @@ Duktape.modSearch foo/bar
 ===*/
 
 /* Test the current implementation limit for ID lengths.  This also
-* does some boundary value testing for ID length.
-*/
+ * does some boundary value testing for ID length.
+ */
 
 function lengthTest() {
     var i;

ecmascript-testcases/test-commonjs-require-tweaked-id.js

@@ -12,24 +12,24 @@
 /*===
 Duktape.modSearch quux
 quux: Error
-Duktape.modSearch foo/bar/quux
-./quux: Error
 Duktape.modSearch foo/quux
+./quux: Error
+Duktape.modSearch quux
 ../quux: Error
-Duktape.modSearch testModule
+Duktape.modSearch testModule/subModule
 Duktape.modSearch test/innerRequire
 testModule: Error
 ===*/
 
 Duktape.modSearch = function (id) {
     print('Duktape.modSearch', id);
-    if (id == 'testModule') {
+    if (id == 'testModule/subModule') {
         // require.id is non-writable but is configurable, so its value must
         // be changed with Object.defineProperty().
         return 'var mod;\n' +
-               'exports.name = "testModule";\n' +
-               'Object.defineProperty(require, "id", { value: "./././testModule/foo/../../test" });\n' +   // same as 'test' but non-canonical
-               'mod = require("./innerRequire");\n';
+               'exports.name = "testModule/subModule";\n' +
+               'Object.defineProperty(require, "id", { value: "./././testModule/subModule/foo/../../../test/foo" });\n' +   // same as 'test/foo' but non-canonical
+               'mod = require("./innerRequire");\n';  // test/foo + ./innerRequire -> test/innerRequire
     }
     throw new Error('cannot find module');
 }
@@ -51,13 +51,13 @@ function tweakedIdentifierTest() {
 
     require.id = './foo//./bar';  // same as 'foo/bar'
     globalTest('quux');
-    globalTest('./quux');
-    globalTest('../quux');
+    globalTest('./quux');         // foo/bar + ./quux -> foo/quux
+    globalTest('../quux');        // foo/bar + ../quux -> quux
     delete require.id;
 
     // Module 'id' not relative
     try {
-        mod = require('testModule');
+        mod = require('testModule/subModule');
         print('never here');
     } catch (e) {
         print('testModule:', e.name);

src/duk_bi_global.c

@@ -810,16 +810,20 @@ static void duk__bi_global_resolve_module_id(duk_context *ctx, const char *req_i
 	 *  Set up the resolution input which is the requested ID directly
 	 *  (if absolute or no current module path) or with current module
 	 *  ID prepended (if relative and current module path exists).
+	 *
+	 *  Suppose current module is 'foo/bar' and relative path is './quux'.
+	 *  The 'bar' component must be replaced so the initial input here is
+	 *  'foo/bar/.././quux'.
 	 */
 
 	req_id_len = DUK_STRLEN(req_id);
 	if (mod_id != NULL && req_id[0] == '.') {
 		mod_id_len = DUK_STRLEN(mod_id);
-		if (mod_id_len + 1 + req_id_len + 1 >= sizeof(buf_in)) {
+		if (mod_id_len + 4 + req_id_len + 1 >= sizeof(buf_in)) {
 			DUK_DD(DUK_DDPRINT("resolve error: current and requested module ID don't fit into resolve input buffer"));
 			goto resolve_error;
 		}
-		(void) DUK_SNPRINTF((char *) buf_in, sizeof(buf_in), "%s/%s", (const char *) mod_id, (const char *) req_id);
+		(void) DUK_SNPRINTF((char *) buf_in, sizeof(buf_in), "%s/../%s", (const char *) mod_id, (const char *) req_id);
 	} else {
 		if (req_id_len + 1 >= sizeof(buf_in)) {
 			DUK_DD(DUK_DDPRINT("resolve error: requested module ID doesn't fit into resolve input buffer"));