Skip to content

Commit c43b182

Browse files
lamarajasnell
authored andcommitted
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. PR-URL: #5950 Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent fa74805 commit c43b182

File tree

8 files changed

+103
-17
lines changed

8 files changed

+103
-17
lines changed

lib/module.js

Lines changed: 26 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -97,27 +97,32 @@ function readPackage(requestPath) {
9797
return pkg;
9898
}
9999

100-
function tryPackage(requestPath, exts) {
100+
function tryPackage(requestPath, exts, isMain) {
101101
var pkg = readPackage(requestPath);
102102

103103
if (!pkg) return false;
104104

105105
var filename = path.resolve(requestPath, pkg);
106-
return tryFile(filename) ||
107-
tryExtensions(filename, exts) ||
108-
tryExtensions(path.resolve(filename, 'index'), exts);
106+
return tryFile(filename, isMain) ||
107+
tryExtensions(filename, exts, isMain) ||
108+
tryExtensions(path.resolve(filename, 'index'), exts, isMain);
109109
}
110110

111111
// check if the file exists and is not a directory
112-
function tryFile(requestPath) {
112+
// resolve to the absolute realpath if running main module,
113+
// otherwise resolve to absolute while keeping symlinks intact.
114+
function tryFile(requestPath, isMain) {
113115
const rc = stat(requestPath);
114-
return rc === 0 && fs.realpathSync(requestPath);
116+
if (isMain) {
117+
return rc === 0 && fs.realpathSync(requestPath);
118+
}
119+
return rc === 0 && path.resolve(requestPath);
115120
}
116121

117122
// given a path check a the file exists with any of the set extensions
118-
function tryExtensions(p, exts) {
123+
function tryExtensions(p, exts, isMain) {
119124
for (var i = 0; i < exts.length; i++) {
120-
const filename = tryFile(p + exts[i]);
125+
const filename = tryFile(p + exts[i], isMain);
121126

122127
if (filename) {
123128
return filename;
@@ -127,7 +132,7 @@ function tryExtensions(p, exts) {
127132
}
128133

129134
var warned = false;
130-
Module._findPath = function(request, paths) {
135+
Module._findPath = function(request, paths, isMain) {
131136
if (path.isAbsolute(request)) {
132137
paths = [''];
133138
} else if (!paths || paths.length === 0) {
@@ -154,32 +159,36 @@ Module._findPath = function(request, paths) {
154159
if (!trailingSlash) {
155160
const rc = stat(basePath);
156161
if (rc === 0) { // File.
157-
filename = fs.realpathSync(basePath);
162+
if (!isMain) {
163+
filename = path.resolve(basePath);
164+
} else {
165+
filename = fs.realpathSync(basePath);
166+
}
158167
} else if (rc === 1) { // Directory.
159168
if (exts === undefined)
160169
exts = Object.keys(Module._extensions);
161-
filename = tryPackage(basePath, exts);
170+
filename = tryPackage(basePath, exts, isMain);
162171
}
163172

164173
if (!filename) {
165174
// try it with each of the extensions
166175
if (exts === undefined)
167176
exts = Object.keys(Module._extensions);
168-
filename = tryExtensions(basePath, exts);
177+
filename = tryExtensions(basePath, exts, isMain);
169178
}
170179
}
171180

172181
if (!filename) {
173182
if (exts === undefined)
174183
exts = Object.keys(Module._extensions);
175-
filename = tryPackage(basePath, exts);
184+
filename = tryPackage(basePath, exts, isMain);
176185
}
177186

178187
if (!filename) {
179188
// try it with each of the extensions at "index"
180189
if (exts === undefined)
181190
exts = Object.keys(Module._extensions);
182-
filename = tryExtensions(path.resolve(basePath, 'index'), exts);
191+
filename = tryExtensions(path.resolve(basePath, 'index'), exts, isMain);
183192
}
184193

185194
if (filename) {
@@ -374,7 +383,7 @@ Module._load = function(request, parent, isMain) {
374383
debug('Module._load REQUEST %s parent: %s', request, parent.id);
375384
}
376385

377-
var filename = Module._resolveFilename(request, parent);
386+
var filename = Module._resolveFilename(request, parent, isMain);
378387

379388
var cachedModule = Module._cache[filename];
380389
if (cachedModule) {
@@ -412,7 +421,7 @@ function tryModuleLoad(module, filename) {
412421
}
413422
}
414423

415-
Module._resolveFilename = function(request, parent) {
424+
Module._resolveFilename = function(request, parent, isMain) {
416425
if (NativeModule.nonInternalExists(request)) {
417426
return request;
418427
}
@@ -424,7 +433,7 @@ Module._resolveFilename = function(request, parent) {
424433
// look up the filename first, since that's the cache key.
425434
debug('looking for %j in %j', id, paths);
426435

427-
var filename = Module._findPath(request, paths);
436+
var filename = Module._findPath(request, paths, isMain);
428437
if (!filename) {
429438
var err = new Error("Cannot find module '" + request + "'");
430439
err.code = 'MODULE_NOT_FOUND';
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
exports.dep1 = require('dep1');
2+
exports.dep2 = exports.dep1.dep2;

test/fixtures/module-require-symlink/node_modules/bar/index.js

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

test/fixtures/module-require-symlink/node_modules/dep1/index.js

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

test/fixtures/module-require-symlink/node_modules/dep1/node_modules/bar/index.js

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

test/fixtures/module-require-symlink/node_modules/dep2/index.js

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
'use strict';
2+
const assert = require('assert');
3+
const common = require('../../common');
4+
const path = require('path');
5+
6+
const linkScriptTarget = path.join(common.fixturesDir,
7+
'/module-require-symlink/symlinked.js');
8+
9+
var foo = require('./foo');
10+
assert.equal(foo.dep1.bar.version, 'CORRECT_VERSION');
11+
assert.equal(foo.dep2.bar.version, 'CORRECT_VERSION');
12+
assert.equal(__filename, linkScriptTarget);
13+
assert(__filename in require.cache);

test/parallel/test-require-symlink.js

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
'use strict';
2+
const common = require('../common');
3+
const assert = require('assert');
4+
const path = require('path');
5+
const fs = require('fs');
6+
const exec = require('child_process').exec;
7+
const spawn = require('child_process').spawn;
8+
9+
const linkTarget = path.join(common.fixturesDir,
10+
'/module-require-symlink/node_modules/dep2/');
11+
12+
const linkDir = path.join(common.fixturesDir,
13+
'/module-require-symlink/node_modules/dep1/node_modules/dep2');
14+
15+
const linkScriptTarget = path.join(common.fixturesDir,
16+
'/module-require-symlink/symlinked.js');
17+
18+
const linkScript = path.join(common.tmpDir, 'module-require-symlink.js');
19+
20+
if (common.isWindows) {
21+
// On Windows, creating symlinks requires admin privileges.
22+
// We'll only try to run symlink test if we have enough privileges.
23+
exec('whoami /priv', function(err, o) {
24+
if (err || o.indexOf('SeCreateSymbolicLinkPrivilege') == -1) {
25+
console.log('Skipped: insufficient privileges');
26+
return;
27+
} else {
28+
test();
29+
}
30+
});
31+
} else {
32+
test();
33+
}
34+
35+
function test() {
36+
process.on('exit', function() {
37+
fs.unlinkSync(linkDir);
38+
fs.unlinkSync(linkScript);
39+
});
40+
41+
fs.symlinkSync(linkTarget, linkDir);
42+
fs.symlinkSync(linkScriptTarget, linkScript);
43+
44+
// load symlinked-module
45+
var fooModule =
46+
require(path.join(common.fixturesDir, '/module-require-symlink/foo.js'));
47+
assert.equal(fooModule.dep1.bar.version, 'CORRECT_VERSION');
48+
assert.equal(fooModule.dep2.bar.version, 'CORRECT_VERSION');
49+
50+
// load symlinked-script as main
51+
var node = process.execPath;
52+
var child = spawn(node, [linkScript]);
53+
child.on('close', function(code, signal) {
54+
assert(!code);
55+
assert(!signal);
56+
});
57+
}

0 commit comments

Comments
 (0)