Skip to content

Commit

Permalink
module: preserve symlinks when requiring
Browse files Browse the repository at this point in the history
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.

PR-URL: #5950
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
lamara authored and jasnell committed Apr 26, 2016
1 parent fa74805 commit c43b182
Show file tree
Hide file tree
Showing 8 changed files with 103 additions and 17 deletions.
43 changes: 26 additions & 17 deletions lib/module.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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) {
Expand All @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
}
Expand All @@ -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';
Expand Down
2 changes: 2 additions & 0 deletions test/fixtures/module-require-symlink/foo.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
exports.dep1 = require('dep1');
exports.dep2 = exports.dep1.dep2;

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

13 changes: 13 additions & 0 deletions test/fixtures/module-require-symlink/symlinked.js
Original file line number Diff line number Diff line change
@@ -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);
57 changes: 57 additions & 0 deletions test/parallel/test-require-symlink.js
Original file line number Diff line number Diff line change
@@ -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);
});
}

0 comments on commit c43b182

Please sign in to comment.