From ed16801c7331dc2027063d81eceb9864a38f1c3e Mon Sep 17 00:00:00 2001 From: Alex Lamar Date: Tue, 29 Mar 2016 23:28:35 -0400 Subject: [PATCH] module: preserve symlinks when requiring Currently, required modules use the real location of the package/file as their __filename and __dirname, instead of the symlinked path if it exists. This behaviour is undocumented (it even goes against documentation in certain scenarios), creating hard-to-debug problems for developers who wish to leverage filesystem abstractions to lay out their application. This patch resolves all required modules to their canonical path while still preserving any symlinks within the path, instead of resolving to their canonical realpath. The one special case observed is when the main module is loaded -- in this case, the realpath does need to be used in order for the main module to load properly. --- lib/module.js | 43 ++++++++------ test/fixtures/module-require-symlink/foo.js | 2 + .../node_modules/bar/index.js | 1 + .../node_modules/dep1/index.js | 2 + .../dep1/node_modules/bar/index.js | 1 + .../node_modules/dep2/index.js | 1 + .../module-require-symlink/symlinked.js | 13 +++++ test/parallel/test-require-symlink.js | 57 +++++++++++++++++++ 8 files changed, 103 insertions(+), 17 deletions(-) create mode 100644 test/fixtures/module-require-symlink/foo.js create mode 100644 test/fixtures/module-require-symlink/node_modules/bar/index.js create mode 100644 test/fixtures/module-require-symlink/node_modules/dep1/index.js create mode 100644 test/fixtures/module-require-symlink/node_modules/dep1/node_modules/bar/index.js create mode 100644 test/fixtures/module-require-symlink/node_modules/dep2/index.js create mode 100644 test/fixtures/module-require-symlink/symlinked.js create mode 100644 test/parallel/test-require-symlink.js diff --git a/lib/module.js b/lib/module.js index f633d62c490c39..fb233d6c4541fe 100644 --- a/lib/module.js +++ b/lib/module.js @@ -97,27 +97,32 @@ function readPackage(requestPath) { return pkg; } -function tryPackage(requestPath, exts) { +function tryPackage(requestPath, exts, isMain) { var pkg = readPackage(requestPath); if (!pkg) return false; var filename = path.resolve(requestPath, pkg); - return tryFile(filename) || - tryExtensions(filename, exts) || - tryExtensions(path.resolve(filename, 'index'), exts); + return tryFile(filename, isMain) || + tryExtensions(filename, exts, isMain) || + tryExtensions(path.resolve(filename, 'index'), exts, isMain); } // check if the file exists and is not a directory -function tryFile(requestPath) { +// resolve to the absolute realpath if running main module, +// otherwise resolve to absolute while keeping symlinks intact. +function tryFile(requestPath, isMain) { const rc = stat(requestPath); - return rc === 0 && fs.realpathSync(requestPath); + if (isMain) { + return rc === 0 && fs.realpathSync(requestPath); + } + return rc === 0 && path.resolve(requestPath); } // given a path check a the file exists with any of the set extensions -function tryExtensions(p, exts) { +function tryExtensions(p, exts, isMain) { for (var i = 0; i < exts.length; i++) { - const filename = tryFile(p + exts[i]); + const filename = tryFile(p + exts[i], isMain); if (filename) { return filename; @@ -127,7 +132,7 @@ function tryExtensions(p, exts) { } var warned = false; -Module._findPath = function(request, paths) { +Module._findPath = function(request, paths, isMain) { if (path.isAbsolute(request)) { paths = ['']; } else if (!paths || paths.length === 0) { @@ -154,32 +159,36 @@ Module._findPath = function(request, paths) { if (!trailingSlash) { const rc = stat(basePath); if (rc === 0) { // File. - filename = fs.realpathSync(basePath); + if (!isMain) { + filename = path.resolve(basePath); + } else { + filename = fs.realpathSync(basePath); + } } else if (rc === 1) { // Directory. if (exts === undefined) exts = Object.keys(Module._extensions); - filename = tryPackage(basePath, exts); + filename = tryPackage(basePath, exts, isMain); } if (!filename) { // try it with each of the extensions if (exts === undefined) exts = Object.keys(Module._extensions); - filename = tryExtensions(basePath, exts); + filename = tryExtensions(basePath, exts, isMain); } } if (!filename) { if (exts === undefined) exts = Object.keys(Module._extensions); - filename = tryPackage(basePath, exts); + filename = tryPackage(basePath, exts, isMain); } if (!filename) { // try it with each of the extensions at "index" if (exts === undefined) exts = Object.keys(Module._extensions); - filename = tryExtensions(path.resolve(basePath, 'index'), exts); + filename = tryExtensions(path.resolve(basePath, 'index'), exts, isMain); } if (filename) { @@ -374,7 +383,7 @@ Module._load = function(request, parent, isMain) { debug('Module._load REQUEST %s parent: %s', request, parent.id); } - var filename = Module._resolveFilename(request, parent); + var filename = Module._resolveFilename(request, parent, isMain); var cachedModule = Module._cache[filename]; if (cachedModule) { @@ -412,7 +421,7 @@ function tryModuleLoad(module, filename) { } } -Module._resolveFilename = function(request, parent) { +Module._resolveFilename = function(request, parent, isMain) { if (NativeModule.nonInternalExists(request)) { return request; } @@ -424,7 +433,7 @@ Module._resolveFilename = function(request, parent) { // look up the filename first, since that's the cache key. debug('looking for %j in %j', id, paths); - var filename = Module._findPath(request, paths); + var filename = Module._findPath(request, paths, isMain); if (!filename) { var err = new Error("Cannot find module '" + request + "'"); err.code = 'MODULE_NOT_FOUND'; diff --git a/test/fixtures/module-require-symlink/foo.js b/test/fixtures/module-require-symlink/foo.js new file mode 100644 index 00000000000000..e7361df25ed218 --- /dev/null +++ b/test/fixtures/module-require-symlink/foo.js @@ -0,0 +1,2 @@ +exports.dep1 = require('dep1'); +exports.dep2 = exports.dep1.dep2; diff --git a/test/fixtures/module-require-symlink/node_modules/bar/index.js b/test/fixtures/module-require-symlink/node_modules/bar/index.js new file mode 100644 index 00000000000000..f8d439ec7e126c --- /dev/null +++ b/test/fixtures/module-require-symlink/node_modules/bar/index.js @@ -0,0 +1 @@ +exports.version = 'INCORRECT_VERSION'; diff --git a/test/fixtures/module-require-symlink/node_modules/dep1/index.js b/test/fixtures/module-require-symlink/node_modules/dep1/index.js new file mode 100644 index 00000000000000..78f255b8f4ad35 --- /dev/null +++ b/test/fixtures/module-require-symlink/node_modules/dep1/index.js @@ -0,0 +1,2 @@ +exports.bar = require('bar'); +exports.dep2 = require('dep2'); diff --git a/test/fixtures/module-require-symlink/node_modules/dep1/node_modules/bar/index.js b/test/fixtures/module-require-symlink/node_modules/dep1/node_modules/bar/index.js new file mode 100644 index 00000000000000..6d8b0321d21fd5 --- /dev/null +++ b/test/fixtures/module-require-symlink/node_modules/dep1/node_modules/bar/index.js @@ -0,0 +1 @@ +exports.version = 'CORRECT_VERSION'; diff --git a/test/fixtures/module-require-symlink/node_modules/dep2/index.js b/test/fixtures/module-require-symlink/node_modules/dep2/index.js new file mode 100644 index 00000000000000..2b81d1512858cd --- /dev/null +++ b/test/fixtures/module-require-symlink/node_modules/dep2/index.js @@ -0,0 +1 @@ +exports.bar = require('bar'); diff --git a/test/fixtures/module-require-symlink/symlinked.js b/test/fixtures/module-require-symlink/symlinked.js new file mode 100644 index 00000000000000..fea1cedb0aa4f8 --- /dev/null +++ b/test/fixtures/module-require-symlink/symlinked.js @@ -0,0 +1,13 @@ +'use strict'; +const assert = require('assert'); +const common = require('../../common'); +const path = require('path'); + +const linkScriptTarget = path.join(common.fixturesDir, + '/module-require-symlink/symlinked.js'); + +var foo = require('./foo'); +assert.equal(foo.dep1.bar.version, 'CORRECT_VERSION'); +assert.equal(foo.dep2.bar.version, 'CORRECT_VERSION'); +assert.equal(__filename, linkScriptTarget); +assert(__filename in require.cache); diff --git a/test/parallel/test-require-symlink.js b/test/parallel/test-require-symlink.js new file mode 100644 index 00000000000000..6b716ffa1381ac --- /dev/null +++ b/test/parallel/test-require-symlink.js @@ -0,0 +1,57 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const path = require('path'); +const fs = require('fs'); +const exec = require('child_process').exec; +const spawn = require('child_process').spawn; + +const linkTarget = path.join(common.fixturesDir, + '/module-require-symlink/node_modules/dep2/'); + +const linkDir = path.join(common.fixturesDir, + '/module-require-symlink/node_modules/dep1/node_modules/dep2'); + +const linkScriptTarget = path.join(common.fixturesDir, + '/module-require-symlink/symlinked.js'); + +const linkScript = path.join(common.tmpDir, 'module-require-symlink.js'); + +if (common.isWindows) { + // On Windows, creating symlinks requires admin privileges. + // We'll only try to run symlink test if we have enough privileges. + exec('whoami /priv', function(err, o) { + if (err || o.indexOf('SeCreateSymbolicLinkPrivilege') == -1) { + console.log('Skipped: insufficient privileges'); + return; + } else { + test(); + } + }); +} else { + test(); +} + +function test() { + process.on('exit', function() { + fs.unlinkSync(linkDir); + fs.unlinkSync(linkScript); + }); + + fs.symlinkSync(linkTarget, linkDir); + fs.symlinkSync(linkScriptTarget, linkScript); + + // load symlinked-module + var fooModule = + require(path.join(common.fixturesDir, '/module-require-symlink/foo.js')); + assert.equal(fooModule.dep1.bar.version, 'CORRECT_VERSION'); + assert.equal(fooModule.dep2.bar.version, 'CORRECT_VERSION'); + + // load symlinked-script as main + var node = process.execPath; + var child = spawn(node, [linkScript]); + child.on('close', function(code, signal) { + assert(!code); + assert(!signal); + }); +}