From 4dcdfaf9291a5d6a2ed25b5742151a0de562a4a1 Mon Sep 17 00:00:00 2001 From: isaacs Date: Tue, 22 Dec 2009 16:24:32 +0100 Subject: [PATCH] Fix require("../blah") issues Added some more tests, and refactored the uri and path modules to use the same normalization logic, so that nothing is relying on flaky regexps. http://groups.google.com/group/nodejs/browse_thread/thread/34779f8c10098c5e http://groups.google.com/group/nodejs/browse_thread/thread/1aa0146b92582679#msg_9822c03998cb4064 --- lib/uri.js | 15 +----- src/node.js | 49 ++++++++++--------- test/mjsunit/fixtures/cycles/folder/foo.js | 6 +++ test/mjsunit/fixtures/cycles/root.js | 10 ++++ .../fixtures/nested-index/one/hello.js | 2 + .../fixtures/nested-index/one/index.js | 1 + .../fixtures/nested-index/two/hello.js | 2 + .../fixtures/nested-index/two/index.js | 1 + test/mjsunit/test-module-loading.js | 11 +++++ test/mjsunit/test-readdir.js | 3 +- 10 files changed, 62 insertions(+), 38 deletions(-) create mode 100644 test/mjsunit/fixtures/cycles/folder/foo.js create mode 100644 test/mjsunit/fixtures/cycles/root.js create mode 100644 test/mjsunit/fixtures/nested-index/one/hello.js create mode 100644 test/mjsunit/fixtures/nested-index/one/index.js create mode 100644 test/mjsunit/fixtures/nested-index/two/hello.js create mode 100644 test/mjsunit/fixtures/nested-index/two/index.js diff --git a/lib/uri.js b/lib/uri.js index 45187e0fa69..e44dd40a464 100644 --- a/lib/uri.js +++ b/lib/uri.js @@ -96,20 +96,7 @@ function uri_parse (url) { } /* normalize */ - var directories = []; - for (var i = 0; i < items.directories.length; i++) { - var directory = items.directories[i]; - if (directory == '.') { - } else if (directory == '..') { - if (directories.length && directories[directories.length - 1] != '..') - directories.pop(); - else - directories.push('..'); - } else { - directories.push(directory); - } - } - items.directories = directories; + items.directories = require("path").normalizeArray(items.directories); items.domains = items.domain.split("."); diff --git a/src/node.js b/src/node.js index 500d7f42978..e255c192971 100644 --- a/src/node.js +++ b/src/node.js @@ -667,31 +667,31 @@ var posix = posixModule.exports; var pathModule = createInternalModule("path", function (exports) { exports.join = function () { - var joined = "", - dotre = /^\.\//, - dotreplace = "", - dotdotre = /(^|(\/)([^\/]+\/)?)\.\.\//g, - dotdotreplace = "" - for (var i = 0; i < arguments.length; i++) { - var part = arguments[i].toString(); - - /* Some logic to shorten paths */ - if (part === ".") continue; - while (dotre.exec(part)) part = part.replace(dotre, dotreplace); - - if (i === 0) { - part = part.replace(/\/*$/, "/"); - } else if (i === arguments.length - 1) { - part = part.replace(/^\/*/, ""); + return exports.normalize(Array.prototype.join.call(arguments, "/")); + }; + + function normalizeArray (parts) { + var directories = []; + for (var i = 0; i < parts.length; i++) { + var directory = parts[i]; + if (directory === "." || (directory === "" && directories.length)) { + continue; + } + if ( + directory === ".." + && directories.length + && directories[directories.length - 1] != '..' + ) { + directories.pop(); } else { - part = part.replace(/^\/*/, "").replace(/\/*$/, "/"); + directories.push(directory); } - joined += part; } - // replace /foo/../bar/baz with /bar/baz - while (dotdotre.exec(joined)) joined = joined.replace(dotdotre, dotdotreplace); - return joined; - + return directories; + } + + exports.normalize = function (path) { + return normalizeArray(path.split("/")).join("/"); }; exports.dirname = function (path) { @@ -898,7 +898,6 @@ Module.prototype.loadScript = function (filename, loadPromise) { require.paths = process.paths; require.async = requireAsync; require.main = process.mainModule; - // create wrapper function var wrapper = "var __wrap__ = function (exports, require, module, __filename) { " + content @@ -958,6 +957,10 @@ process.mainModule = createModule("."); var loadPromise = new process.Promise(); process.mainModule.load(process.ARGV[1], loadPromise); +loadPromise.addErrback(function(e) { + throw e; +}); + // All our arguments are loaded. We've evaluated all of the scripts. We // might even have created TCP servers. Now we enter the main eventloop. If // there are no watchers on the loop (except for the ones that were diff --git a/test/mjsunit/fixtures/cycles/folder/foo.js b/test/mjsunit/fixtures/cycles/folder/foo.js new file mode 100644 index 00000000000..3763a118754 --- /dev/null +++ b/test/mjsunit/fixtures/cycles/folder/foo.js @@ -0,0 +1,6 @@ + +var root = require("./../root"); + +exports.hello = function () { + return root.calledFromFoo(); +}; \ No newline at end of file diff --git a/test/mjsunit/fixtures/cycles/root.js b/test/mjsunit/fixtures/cycles/root.js new file mode 100644 index 00000000000..b515639a238 --- /dev/null +++ b/test/mjsunit/fixtures/cycles/root.js @@ -0,0 +1,10 @@ + +var foo = exports.foo = require("./folder/foo"); + +exports.hello = "hello"; +exports.sayHello = function () { + return foo.hello(); +}; +exports.calledFromFoo = function () { + return exports.hello; +}; \ No newline at end of file diff --git a/test/mjsunit/fixtures/nested-index/one/hello.js b/test/mjsunit/fixtures/nested-index/one/hello.js new file mode 100644 index 00000000000..e2872756292 --- /dev/null +++ b/test/mjsunit/fixtures/nested-index/one/hello.js @@ -0,0 +1,2 @@ +exports.hello = "hello from one!"; + diff --git a/test/mjsunit/fixtures/nested-index/one/index.js b/test/mjsunit/fixtures/nested-index/one/index.js new file mode 100644 index 00000000000..d959a288b18 --- /dev/null +++ b/test/mjsunit/fixtures/nested-index/one/index.js @@ -0,0 +1 @@ +exports.hello = require('./hello').hello; diff --git a/test/mjsunit/fixtures/nested-index/two/hello.js b/test/mjsunit/fixtures/nested-index/two/hello.js new file mode 100644 index 00000000000..fead2bde46b --- /dev/null +++ b/test/mjsunit/fixtures/nested-index/two/hello.js @@ -0,0 +1,2 @@ +exports.hello = "hello from two!"; + diff --git a/test/mjsunit/fixtures/nested-index/two/index.js b/test/mjsunit/fixtures/nested-index/two/index.js new file mode 100644 index 00000000000..d959a288b18 --- /dev/null +++ b/test/mjsunit/fixtures/nested-index/two/index.js @@ -0,0 +1 @@ +exports.hello = require('./hello').hello; diff --git a/test/mjsunit/test-module-loading.js b/test/mjsunit/test-module-loading.js index c6aea253846..09b58074f99 100644 --- a/test/mjsunit/test-module-loading.js +++ b/test/mjsunit/test-module-loading.js @@ -33,6 +33,17 @@ assert.equal("D", d3.D()); assert.equal(true, d4.D instanceof Function); assert.equal("D", d4.D()); +debug("test index.js modules ids and relative loading") +var one = require("./fixtures/nested-index/one"), + two = require("./fixtures/nested-index/two"); +assert.notEqual(one.hello, two.hello); + +debug("test cycles containing a .. path"); +var root = require("./fixtures/cycles/root"), + foo = require("./fixtures/cycles/folder/foo"); +assert.equal(root.foo, foo); +assert.equal(root.sayHello(), root.hello); + var errorThrown = false; try { require("./fixtures/throws_error"); diff --git a/test/mjsunit/test-readdir.js b/test/mjsunit/test-readdir.js index 1b98f0fe03c..16c46a2aa27 100644 --- a/test/mjsunit/test-readdir.js +++ b/test/mjsunit/test-readdir.js @@ -7,7 +7,8 @@ puts("readdir " + fixturesDir); promise.addCallback(function (files) { p(files); - assert.deepEqual(["a.js", "b", "multipart.js", "test_ca.pem", + assert.deepEqual(["a.js", "b","cycles", "multipart.js", + "nested-index","test_ca.pem", "test_cert.pem", "test_key.pem", "throws_error.js", "x.txt"], files.sort()); });