Regression: require.resolve(".", { paths }) behaves differently from require.resolve("./", { paths })

[tolmasky]

Version

18.14.2

Platform

22.3.0 Darwin Kernel Version 22.3.0

Subsystem

loader

What steps will reproduce the bug?

home:/$ docker run --rm -it node:18.14.2-slim bash
node:/# echo "['./', '.'].map(request => console.log(require.resolve(request, { paths: ['..'] })))" > index.js
node:/# mkdir test && cd test
node:/# node ../index.js

This produces:

> /index.js
> node:internal/modules/cjs/loader:1078
  throw err;
  ^

Error: Cannot find module '.'
Require stack:
- /index.js
    at Module._resolveFilename (node:internal/modules/cjs/loader:1075:15)
    at Function.resolve (node:internal/modules/cjs/helpers:116:19)
    at /index.js:1:48
    at Array.map (<anonymous>)
    at Object.<anonymous> (/index.js:1:13)
    at Module._compile (node:internal/modules/cjs/loader:1254:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1308:10)
    at Module.load (node:internal/modules/cjs/loader:1117:32)
    at Module._load (node:internal/modules/cjs/loader:958:12)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:81:12) {
  code: 'MODULE_NOT_FOUND',
  requireStack: [ '/index.js' ]
}
node:/#

The same is true of .. and ../. I leave that as an exercise to the reader. If you rerun the above in node 10, you get the same log for both, as expected:

home:/$ docker run --rm -it node:10.24.1-slim bash
node:/# echo "['./', '.'].map(request => console.log(require.resolve(request, { paths: ['..'] })))" > index.js
node:/# mkdir test && cd test
node:/# node ../index.js
/index.js
/index.js
node:/#

If you want to get more clever, you can put a index.js inside of the test directory, and then it'll just give you different results without crashing:

home:/$ docker run --rm -it node:18.14.2-slim bash
node:/# echo "['./', '.'].map(request => console.log(require.resolve(request, { paths: ['..'] })))" > index.js
node:/# mkdir test && cd test
node:/# echo "['./', '.'].map(request => console.log(require.resolve(request, { paths: ['..'] })))" > index.js
node:/# node ../index.js
/index.js
/test/index.js
node:/#

How often does it reproduce? Is there a required condition?

Every time.

What is the expected behavior?

The same way it worked in node 10:

home:/$ docker run --rm -it node:10.24.1-slim bash
node:/# echo "['./', '.'].map(request => console.log(require.resolve(request, { paths: ['..'] })))" > index.js
node:/# mkdir test && cd test
node:/# node ../index.js
/index.js
/index.js
node:/#

What do you see instead?

Different file paths, crash, etc.

Additional information

This seems to be a result of this pull request: #27598

The code in Module._resolveFilename only checks for "./" and "../" to determine if the path is a relative path:

node/lib/internal/modules/cjs/loader.js

Lines 1027 to 1030 in 3b0c047

	const isRelative = StringPrototypeStartsWith(request, './') ||
	StringPrototypeStartsWith(request, '../') ||
	((isWindows && StringPrototypeStartsWith(request, '.\\')) ||
	StringPrototypeStartsWith(request, '..\\'));

Adding === "." and === ".." should fix the problem. Or, given that earlier in the file in Module._findPath the same thing is calculated slightly differently:

node/lib/internal/modules/cjs/loader.js

Lines 630 to 638 in 3b0c047

	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)))

Perhaps this should be factored out into a isRelativePath function since it seems to be tricky enough to get right to not rely on separate inlined implementations in various different places.

[bjrmatos]

i just got hit by this today, and when debugging i found the solution the same as the linked PR #47019

is anything missing to fix this? or what needs to be done to move forward with the PR?