From e8e21fb6f8172e82dd2b846dfa53826ea90abf27 Mon Sep 17 00:00:00 2001 From: Dario Piotrowicz Date: Sat, 25 Jan 2025 23:32:13 +0000 Subject: [PATCH] module: fix bad `require.resolve` with option paths for `.` and `..` this change fixes `require.resolve` used with the `paths` option not considering `.` and `..` as relative Fixes: https://github.com/nodejs/node/issues/47000 PR-URL: https://github.com/nodejs/node/pull/56735 Reviewed-By: Yagiz Nizipli Reviewed-By: James M Snell Reviewed-By: Jordan Harband Reviewed-By: Matteo Collina --- lib/internal/modules/cjs/loader.js | 34 +++++++-------- .../relative/subdir/relative-subdir.js | 1 + ...est-require-resolve-opts-paths-relative.js | 43 +++++++++++++++++++ 3 files changed, 61 insertions(+), 17 deletions(-) create mode 100644 test/fixtures/module-require/relative/subdir/relative-subdir.js create mode 100644 test/parallel/test-require-resolve-opts-paths-relative.js diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 236b2484049c43..e21bcb798c4d53 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -713,18 +713,8 @@ Module._findPath = function(request, paths, isMain) { ) )); - const isRelative = StringPrototypeCharCodeAt(request, 0) === CHAR_DOT && - ( - request.length === 1 || - StringPrototypeCharCodeAt(request, 1) === CHAR_FORWARD_SLASH || - (isWindows && StringPrototypeCharCodeAt(request, 1) === CHAR_BACKWARD_SLASH) || - (StringPrototypeCharCodeAt(request, 1) === CHAR_DOT && (( - request.length === 2 || - StringPrototypeCharCodeAt(request, 2) === CHAR_FORWARD_SLASH) || - (isWindows && StringPrototypeCharCodeAt(request, 2) === CHAR_BACKWARD_SLASH))) - ); let insidePath = true; - if (isRelative) { + if (isRelative(request)) { const normalizedRequest = path.normalize(request); if (StringPrototypeStartsWith(normalizedRequest, '..')) { insidePath = false; @@ -1164,12 +1154,7 @@ Module._resolveFilename = function(request, parent, isMain, options) { if (typeof options === 'object' && options !== null) { if (ArrayIsArray(options.paths)) { - const isRelative = StringPrototypeStartsWith(request, './') || - StringPrototypeStartsWith(request, '../') || - ((isWindows && StringPrototypeStartsWith(request, '.\\')) || - StringPrototypeStartsWith(request, '..\\')); - - if (isRelative) { + if (isRelative(request)) { paths = options.paths; } else { const fakeParent = new Module('', null); @@ -1774,6 +1759,21 @@ function createRequire(filename) { return createRequireFromPath(filepath); } +/** + * Checks if a path is relative + * @param {string} path the target path + * @returns {boolean} true if the path is relative, false otherwise + */ +function isRelative(path) { + if (StringPrototypeCharCodeAt(path, 0) !== CHAR_DOT) { return false; } + + return path.length === 1 || path === '..' || + StringPrototypeStartsWith(path, './') || + StringPrototypeStartsWith(path, '../') || + ((isWindows && StringPrototypeStartsWith(path, '.\\')) || + StringPrototypeStartsWith(path, '..\\')); +} + Module.createRequire = createRequire; /** diff --git a/test/fixtures/module-require/relative/subdir/relative-subdir.js b/test/fixtures/module-require/relative/subdir/relative-subdir.js new file mode 100644 index 00000000000000..34eb71b3c6ca39 --- /dev/null +++ b/test/fixtures/module-require/relative/subdir/relative-subdir.js @@ -0,0 +1 @@ +exports.value = 'relative subdir'; diff --git a/test/parallel/test-require-resolve-opts-paths-relative.js b/test/parallel/test-require-resolve-opts-paths-relative.js new file mode 100644 index 00000000000000..522a1fdbce82a4 --- /dev/null +++ b/test/parallel/test-require-resolve-opts-paths-relative.js @@ -0,0 +1,43 @@ +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const fixtures = require('../common/fixtures'); + +if (!common.isMainThread) + common.skip('process.chdir is not available in Workers'); + +const subdir = fixtures.path('module-require', 'relative', 'subdir'); + +process.chdir(subdir); + +// Parent directory paths (`..`) work as intended +{ + assert(require.resolve('.', { paths: ['../'] }).endsWith('index.js')); + assert(require.resolve('./index.js', { paths: ['../'] }).endsWith('index.js')); + + // paths: [".."] should resolve like paths: ["../"] + assert(require.resolve('.', { paths: ['..'] }).endsWith('index.js')); + assert(require.resolve('./index.js', { paths: ['..'] }).endsWith('index.js')); +} + +process.chdir('..'); + +// Current directory paths (`.`) work as intended +{ + assert(require.resolve('.', { paths: ['.'] }).endsWith('index.js')); + assert(require.resolve('./index.js', { paths: ['./'] }).endsWith('index.js')); + + // paths: ["."] should resolve like paths: ["../"] + assert(require.resolve('.', { paths: ['.'] }).endsWith('index.js')); + assert(require.resolve('./index.js', { paths: ['.'] }).endsWith('index.js')); +} + +// Sub directory paths work as intended +{ + // assert.deepStrictEqual(fs.readdirSync('./subdir'), [5]); + assert(require.resolve('./relative-subdir.js', { paths: ['./subdir'] }).endsWith('relative-subdir.js')); + + // paths: ["subdir"] should resolve like paths: ["./subdir"] + assert(require.resolve('./relative-subdir.js', { paths: ['subdir'] }).endsWith('relative-subdir.js')); +}