Skip to content

Commit

Permalink
module: accept Windows relative path
Browse files Browse the repository at this point in the history
Before this change, require('.\\test.js') failed while
require('./test.js') succeeded. This changes _resolveLookupPaths so
that both ways are accepted on Windows.

Fixes: #21918
PR-URL: #22186
Reviewed-By: Phillip Johnsen <johphi@gmail.com>
Reviewed-By: John-David Dalton <john.david.dalton@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
  • Loading branch information
joaocgreis committed Sep 12, 2018
1 parent 3993793 commit b36c581
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 13 deletions.
15 changes: 9 additions & 6 deletions lib/internal/modules/cjs/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,8 @@ const {
CHAR_9,
} = require('internal/constants');

const isWindows = process.platform === 'win32';

function stat(filename) {
filename = path.toNamespacedPath(filename);
const cache = stat.cache;
Expand Down Expand Up @@ -310,7 +312,7 @@ Module._findPath = function(request, paths, isMain) {
// 'node_modules' character codes reversed
var nmChars = [ 115, 101, 108, 117, 100, 111, 109, 95, 101, 100, 111, 110 ];
var nmLen = nmChars.length;
if (process.platform === 'win32') {
if (isWindows) {
// 'from' is the __dirname of the module.
Module._nodeModulePaths = function(from) {
// guarantee that 'from' is absolute.
Expand Down Expand Up @@ -403,11 +405,12 @@ Module._resolveLookupPaths = function(request, parent, newReturn) {
return (newReturn ? null : [request, []]);
}

// Check for relative path
// Check for non-relative path
if (request.length < 2 ||
request.charCodeAt(0) !== CHAR_DOT ||
(request.charCodeAt(1) !== CHAR_DOT &&
request.charCodeAt(1) !== CHAR_FORWARD_SLASH)) {
request.charCodeAt(1) !== CHAR_FORWARD_SLASH &&
(!isWindows || request.charCodeAt(1) !== CHAR_BACKWARD_SLASH))) {
var paths = modulePaths;
if (parent) {
if (!parent.paths)
Expand Down Expand Up @@ -480,7 +483,9 @@ Module._resolveLookupPaths = function(request, parent, newReturn) {

// make sure require('./path') and require('path') get distinct ids, even
// when called from the toplevel js file
if (parentIdPath === '.' && id.indexOf('/') === -1) {
if (parentIdPath === '.' &&
id.indexOf('/') === -1 &&
(!isWindows || id.indexOf('\\') === -1)) {
id = './' + id;
}

Expand Down Expand Up @@ -746,8 +751,6 @@ Module.runMain = function() {
};

Module._initPaths = function() {
const isWindows = process.platform === 'win32';

var homeDir;
var nodePath;
if (isWindows) {
Expand Down
24 changes: 17 additions & 7 deletions test/parallel/test-module-relative-lookup.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,25 @@
'use strict';

require('../common');
const common = require('../common');
const assert = require('assert');
const _module = require('module'); // avoid collision with global.module
const lookupResults = _module._resolveLookupPaths('./lodash');
let paths = lookupResults[1];

// Current directory gets highest priority for local modules
assert.strictEqual(paths[0], '.');
function testFirstInPath(moduleName, isLocalModule) {
const assertFunction = isLocalModule ?
assert.strictEqual :
assert.notStrictEqual;

paths = _module._resolveLookupPaths('./lodash', null, true);
const lookupResults = _module._resolveLookupPaths(moduleName);

// Current directory gets highest priority for local modules
assert.strictEqual(paths && paths[0], '.');
let paths = lookupResults[1];
assertFunction(paths[0], '.');

paths = _module._resolveLookupPaths(moduleName, null, true);
assertFunction(paths && paths[0], '.');
}

testFirstInPath('./lodash', true);

// Relative path on Windows, but a regular file name elsewhere
testFirstInPath('.\\lodash', common.isWindows);
20 changes: 20 additions & 0 deletions test/parallel/test-preload.js
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,26 @@ childProcess.exec(
}
);

// test that preloading with a relative path works
process.chdir(fixtures.fixturesDir);
childProcess.exec(
`"${nodeBinary}" ${preloadOption(['./printA.js'])} "${fixtureB}"`,
common.mustCall(function(err, stdout, stderr) {
assert.ifError(err);
assert.strictEqual(stdout, 'A\nB\n');
})
);
if (common.isWindows) {
// https://github.com/nodejs/node/issues/21918
childProcess.exec(
`"${nodeBinary}" ${preloadOption(['.\\printA.js'])} "${fixtureB}"`,
common.mustCall(function(err, stdout, stderr) {
assert.ifError(err);
assert.strictEqual(stdout, 'A\nB\n');
})
);
}

// https://github.com/nodejs/node/issues/1691
process.chdir(fixtures.fixturesDir);
childProcess.exec(
Expand Down

0 comments on commit b36c581

Please sign in to comment.