Skip to content

Commit 7ca9bc5

Browse files
committed
module: Revert "preserve symlinks when requiring"
After much discussion, it seems that the change broke the way require detects unique identity of the binary modules. While resolving module's full path is not sufficient and maybe even not elegant (like anything that requires the use of realpath() except for certain security scenarios), we cannot break multiple inclusions of binary modules across symlinked, substed or otherwise hidden paths. There should be a test case invoving a binary module to check for issues related to the binary module identity. This reverts commit de1dc0a.
1 parent 8ebec08 commit 7ca9bc5

File tree

8 files changed

+17
-103
lines changed

8 files changed

+17
-103
lines changed

β€Žlib/module.jsβ€Ž

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

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

103103
if (!pkg) return false;
104104

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

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

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

127122
if (filename) {
128123
return filename;
@@ -132,7 +127,7 @@ function tryExtensions(p, exts, isMain) {
132127
}
133128

134129
var warned = false;
135-
Module._findPath = function(request, paths, isMain) {
130+
Module._findPath = function(request, paths) {
136131
if (path.isAbsolute(request)) {
137132
paths = [''];
138133
} else if (!paths || paths.length === 0) {
@@ -159,36 +154,32 @@ Module._findPath = function(request, paths, isMain) {
159154
if (!trailingSlash) {
160155
const rc = stat(basePath);
161156
if (rc === 0) { // File.
162-
if (!isMain) {
163-
filename = path.resolve(basePath);
164-
} else {
165-
filename = fs.realpathSync(basePath);
166-
}
157+
filename = fs.realpathSync(basePath);
167158
} else if (rc === 1) { // Directory.
168159
if (exts === undefined)
169160
exts = Object.keys(Module._extensions);
170-
filename = tryPackage(basePath, exts, isMain);
161+
filename = tryPackage(basePath, exts);
171162
}
172163

173164
if (!filename) {
174165
// try it with each of the extensions
175166
if (exts === undefined)
176167
exts = Object.keys(Module._extensions);
177-
filename = tryExtensions(basePath, exts, isMain);
168+
filename = tryExtensions(basePath, exts);
178169
}
179170
}
180171

181172
if (!filename) {
182173
if (exts === undefined)
183174
exts = Object.keys(Module._extensions);
184-
filename = tryPackage(basePath, exts, isMain);
175+
filename = tryPackage(basePath, exts);
185176
}
186177

187178
if (!filename) {
188179
// try it with each of the extensions at "index"
189180
if (exts === undefined)
190181
exts = Object.keys(Module._extensions);
191-
filename = tryExtensions(path.resolve(basePath, 'index'), exts, isMain);
182+
filename = tryExtensions(path.resolve(basePath, 'index'), exts);
192183
}
193184

194185
if (filename) {
@@ -383,7 +374,7 @@ Module._load = function(request, parent, isMain) {
383374
debug('Module._load REQUEST %s parent: %s', request, parent.id);
384375
}
385376

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

388379
var cachedModule = Module._cache[filename];
389380
if (cachedModule) {
@@ -421,7 +412,7 @@ function tryModuleLoad(module, filename) {
421412
}
422413
}
423414

424-
Module._resolveFilename = function(request, parent, isMain) {
415+
Module._resolveFilename = function(request, parent) {
425416
if (NativeModule.nonInternalExists(request)) {
426417
return request;
427418
}
@@ -433,7 +424,7 @@ Module._resolveFilename = function(request, parent, isMain) {
433424
// look up the filename first, since that's the cache key.
434425
debug('looking for %j in %j', id, paths);
435426

436-
var filename = Module._findPath(request, paths, isMain);
427+
var filename = Module._findPath(request, paths);
437428
if (!filename) {
438429
var err = new Error("Cannot find module '" + request + "'");
439430
err.code = 'MODULE_NOT_FOUND';

β€Žtest/fixtures/module-require-symlink/foo.jsβ€Ž

Lines changed: 0 additions & 2 deletions
This file was deleted.

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

Lines changed: 0 additions & 1 deletion
This file was deleted.

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

Lines changed: 0 additions & 2 deletions
This file was deleted.

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

Lines changed: 0 additions & 1 deletion
This file was deleted.

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

Lines changed: 0 additions & 1 deletion
This file was deleted.

β€Žtest/fixtures/module-require-symlink/symlinked.jsβ€Ž

Lines changed: 0 additions & 13 deletions
This file was deleted.

β€Žtest/parallel/test-require-symlink.jsβ€Ž

Lines changed: 0 additions & 57 deletions
This file was deleted.

0 commit comments

Comments
Β (0)