Skip to content

Commit 14a1de0

Browse files
module,win: fix long path resolve
1 parent c1dc307 commit 14a1de0

File tree

6 files changed

+251
-66
lines changed

6 files changed

+251
-66
lines changed

src/node_file.cc

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3096,8 +3096,14 @@ void BindingData::LegacyMainResolve(const FunctionCallbackInfo<Value>& args) {
30963096

30973097
for (int i = 0; i < legacy_main_extensions_with_main_end; i++) {
30983098
file_path = *initial_file_path + std::string(legacy_main_extensions[i]);
3099-
3100-
switch (FilePathIsFile(env, file_path)) {
3099+
Local<Value> local_file_path =
3100+
Buffer::Copy(
3101+
env->isolate(), file_path.c_str(), strlen(file_path.c_str()))
3102+
.ToLocalChecked();
3103+
BufferValue buff_file_path(isolate, local_file_path);
3104+
ToNamespacedPath(env, &buff_file_path);
3105+
3106+
switch (FilePathIsFile(env, buff_file_path.ToString())) {
31013107
case BindingData::FilePathIsFileReturnType::kIsFile:
31023108
return args.GetReturnValue().Set(i);
31033109
case BindingData::FilePathIsFileReturnType::kIsNotFile:
@@ -3133,8 +3139,14 @@ void BindingData::LegacyMainResolve(const FunctionCallbackInfo<Value>& args) {
31333139
i < legacy_main_extensions_package_fallback_end;
31343140
i++) {
31353141
file_path = *initial_file_path + std::string(legacy_main_extensions[i]);
3136-
3137-
switch (FilePathIsFile(env, file_path)) {
3142+
Local<Value> local_file_path =
3143+
Buffer::Copy(
3144+
env->isolate(), file_path.c_str(), strlen(file_path.c_str()))
3145+
.ToLocalChecked();
3146+
BufferValue buff_file_path(isolate, local_file_path);
3147+
ToNamespacedPath(env, &buff_file_path);
3148+
3149+
switch (FilePathIsFile(env, buff_file_path.ToString())) {
31383150
case BindingData::FilePathIsFileReturnType::kIsFile:
31393151
return args.GetReturnValue().Set(i);
31403152
case BindingData::FilePathIsFileReturnType::kIsNotFile:

src/node_modules.cc

Lines changed: 34 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#include "node_errors.h"
55
#include "node_external_reference.h"
66
#include "node_url.h"
7+
#include "path.h"
78
#include "permission/permission.h"
89
#include "permission/permission_base.h"
910
#include "util-inl.h"
@@ -241,7 +242,7 @@ void BindingData::ReadPackageJSON(const FunctionCallbackInfo<Value>& args) {
241242
Realm* realm = Realm::GetCurrent(args);
242243
auto isolate = realm->isolate();
243244

244-
Utf8Value path(isolate, args[0]);
245+
BufferValue path(isolate, args[0]);
245246
bool is_esm = args[1]->IsTrue();
246247
auto error_context = ErrorContext();
247248
if (is_esm) {
@@ -261,15 +262,10 @@ void BindingData::ReadPackageJSON(const FunctionCallbackInfo<Value>& args) {
261262
permission::PermissionScope::kFileSystemRead,
262263
path.ToStringView());
263264

264-
// TODO(StefanStojanovic): Remove ifdef after
265-
// path.toNamespacedPath logic is ported to C++
266-
#ifdef _WIN32
267-
auto package_json = GetPackageJSON(
268-
realm, "\\\\?\\" + path.ToString(), is_esm ? &error_context : nullptr);
269-
#else
265+
ToNamespacedPath(realm->env(), &path);
270266
auto package_json =
271267
GetPackageJSON(realm, path.ToString(), is_esm ? &error_context : nullptr);
272-
#endif
268+
273269
if (package_json == nullptr) {
274270
return;
275271
}
@@ -323,9 +319,22 @@ void BindingData::GetNearestParentPackageJSON(
323319
CHECK(args[0]->IsString());
324320

325321
Realm* realm = Realm::GetCurrent(args);
326-
Utf8Value path_value(realm->isolate(), args[0]);
322+
BufferValue path_value(realm->isolate(), args[0]);
323+
// Check if the path has a trailing slash. If so, add it after
324+
// ToNamespacedPath() as it will be deleted by ToNamespacedPath()
325+
bool slashCheck = !path_value.ToString().empty() &&
326+
path_value.ToString().back() ==
327+
std::filesystem::path::preferred_separator;
328+
329+
ToNamespacedPath(realm->env(), &path_value);
330+
331+
std::string path_value_str = path_value.ToString();
332+
if (slashCheck) {
333+
path_value_str.push_back(std::filesystem::path::preferred_separator);
334+
}
335+
327336
auto package_json =
328-
TraverseParent(realm, std::filesystem::path(path_value.ToString()));
337+
TraverseParent(realm, std::filesystem::path(path_value_str));
329338

330339
if (package_json != nullptr) {
331340
args.GetReturnValue().Set(package_json->Serialize(realm));
@@ -338,9 +347,22 @@ void BindingData::GetNearestParentPackageJSONType(
338347
CHECK(args[0]->IsString());
339348

340349
Realm* realm = Realm::GetCurrent(args);
341-
Utf8Value path(realm->isolate(), args[0]);
350+
BufferValue path_value(realm->isolate(), args[0]);
351+
// Check if the path has a trailing slash. If so, add it after
352+
// ToNamespacedPath() as it will be deleted by ToNamespacedPath()
353+
bool slashCheck = !path_value.ToString().empty() &&
354+
path_value.ToString().back() ==
355+
std::filesystem::path::preferred_separator;
356+
357+
ToNamespacedPath(realm->env(), &path_value);
358+
359+
std::string path_value_str = path_value.ToString();
360+
if (slashCheck) {
361+
path_value_str.push_back(std::filesystem::path::preferred_separator);
362+
}
363+
342364
auto package_json =
343-
TraverseParent(realm, std::filesystem::path(path.ToString()));
365+
TraverseParent(realm, std::filesystem::path(path_value_str));
344366

345367
if (package_json == nullptr) {
346368
return;

test/es-module/test-GH-50753.js

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

test/es-module/test-cjs-esm-warn.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ const { describe, it } = require('node:test');
1010

1111
const requiringCjsAsEsm = path.resolve(fixtures.path('/es-modules/cjs-esm.js'));
1212
const requiringEsm = path.resolve(fixtures.path('/es-modules/cjs-esm-esm.js'));
13-
const pjson = path.resolve(
13+
const pjson = path.toNamespacedPath(
1414
fixtures.path('/es-modules/package-type-module/package.json')
1515
);
1616

test/es-module/test-cjs-legacyMainResolve-permission.js

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -63,9 +63,11 @@ describe('legacyMainResolve', () => {
6363
6464
assert.throws(() => legacyMainResolve(packageJsonUrl, packageConfig, base), {
6565
code: 'ERR_ACCESS_DENIED',
66-
resource: path.resolve(
67-
${JSON.stringify(fixtextureFolderEscaped)},
68-
${JSON.stringify(mainOrFolder)},
66+
resource: path.toNamespacedPath(
67+
path.resolve(
68+
${JSON.stringify(fixtextureFolderEscaped)},
69+
${JSON.stringify(mainOrFolder)},
70+
)
6971
)
7072
});
7173
`,
@@ -120,10 +122,12 @@ describe('legacyMainResolve', () => {
120122
121123
assert.throws(() => legacyMainResolve(packageJsonUrl, packageConfig, base), {
122124
code: 'ERR_ACCESS_DENIED',
123-
resource: path.resolve(
124-
${JSON.stringify(fixtextureFolderEscaped)},
125-
${JSON.stringify(folder)},
126-
${JSON.stringify(expectedFile)},
125+
resource: path.toNamespacedPath(
126+
path.resolve(
127+
${JSON.stringify(fixtextureFolderEscaped)},
128+
${JSON.stringify(folder)},
129+
${JSON.stringify(expectedFile)},
130+
)
127131
)
128132
});
129133
`,
Lines changed: 189 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,189 @@
1+
'use strict';
2+
3+
// Flags: --expose-internals
4+
5+
const common = require('../common');
6+
if (!common.isWindows) {
7+
common.skip('this test is Windows-specific.');
8+
}
9+
10+
const fs = require('fs');
11+
const path = require('path');
12+
const tmpdir = require('../common/tmpdir');
13+
const { describe, it } = require('node:test');
14+
const { legacyMainResolve } = require('node:internal/modules/esm/resolve');
15+
const { pathToFileURL } = require('node:url');
16+
const { spawnPromisified } = require('../common');
17+
const assert = require('assert');
18+
const { execPath } = require('node:process');
19+
20+
describe('long path on Windows', () => {
21+
it('check long path in ReadPackageJSON', () => {
22+
// Module layout will be the following:
23+
// package.json
24+
// main
25+
// main.js
26+
const packageDirNameLen = Math.max(260 - tmpdir.path.length, 1);
27+
const packageDirName = tmpdir.resolve('x'.repeat(packageDirNameLen));
28+
const packageDirPath = path.resolve(packageDirName);
29+
const packageJsonFilePath = path.join(packageDirPath, 'package.json');
30+
const mainDirName = 'main';
31+
const mainDirPath = path.resolve(packageDirPath, mainDirName);
32+
const mainJsFile = 'main.js';
33+
const mainJsFilePath = path.resolve(mainDirPath, mainJsFile);
34+
35+
tmpdir.refresh();
36+
37+
fs.mkdirSync(packageDirPath);
38+
fs.writeFileSync(
39+
packageJsonFilePath,
40+
`{"main":"${mainDirName}/${mainJsFile}"}`
41+
);
42+
fs.mkdirSync(mainDirPath);
43+
fs.writeFileSync(mainJsFilePath, 'console.log("hello world");');
44+
45+
require('internal/modules/run_main').executeUserEntryPoint(packageDirPath);
46+
47+
tmpdir.refresh();
48+
});
49+
50+
it('check long path in LegacyMainResolve - 1', () => {
51+
// Module layout will be the following:
52+
// package.json
53+
// index.js
54+
const packageDirNameLen = Math.max(260 - tmpdir.path.length, 1);
55+
const packageDirPath = tmpdir.resolve('y'.repeat(packageDirNameLen));
56+
const packageJSPath = path.join(packageDirPath, 'package.json');
57+
const indexJSPath = path.join(packageDirPath, 'index.js');
58+
const packageConfig = { };
59+
60+
tmpdir.refresh();
61+
62+
fs.mkdirSync(packageDirPath);
63+
fs.writeFileSync(packageJSPath, '');
64+
fs.writeFileSync(indexJSPath, '');
65+
66+
const packageJsonUrl = pathToFileURL(
67+
path.resolve(packageJSPath)
68+
);
69+
70+
console.log(legacyMainResolve(packageJsonUrl, packageConfig, ''));
71+
});
72+
73+
it('check long path in LegacyMainResolve - 2', () => {
74+
// Module layout will be the following:
75+
// package.json
76+
// main.js
77+
const packageDirNameLen = Math.max(260 - tmpdir.path.length, 1);
78+
const packageDirPath = tmpdir.resolve('z'.repeat(packageDirNameLen));
79+
const packageJSPath = path.join(packageDirPath, 'package.json');
80+
const indexJSPath = path.join(packageDirPath, 'main.js');
81+
const packageConfig = { main: 'main.js' };
82+
83+
tmpdir.refresh();
84+
85+
fs.mkdirSync(packageDirPath);
86+
fs.writeFileSync(packageJSPath, '');
87+
fs.writeFileSync(indexJSPath, '');
88+
89+
const packageJsonUrl = pathToFileURL(
90+
path.resolve(packageJSPath)
91+
);
92+
93+
console.log(legacyMainResolve(packageJsonUrl, packageConfig, ''));
94+
});
95+
96+
it('check long path in GetNearestParentPackageJSON', async () => {
97+
// Module layout will be the following:
98+
// node_modules
99+
// test-module
100+
// package.json (path is less than 260 chars long)
101+
// cjs
102+
// package.json (path is more than 260 chars long)
103+
// index.js
104+
const testModuleDirPath = 'node_modules/test-module';
105+
let cjsDirPath = path.join(testModuleDirPath, 'cjs');
106+
let modulePackageJSPath = path.join(testModuleDirPath, 'package.json');
107+
let cjsPackageJSPath = path.join(cjsDirPath, 'package.json');
108+
let cjsIndexJSPath = path.join(cjsDirPath, 'index.js');
109+
110+
const tmpDirNameLen = Math.max(
111+
260 - tmpdir.path.length - cjsPackageJSPath.length, 1);
112+
const tmpDirPath = tmpdir.resolve('k'.repeat(tmpDirNameLen));
113+
114+
cjsDirPath = path.join(tmpDirPath, cjsDirPath);
115+
modulePackageJSPath = path.join(tmpDirPath, modulePackageJSPath);
116+
cjsPackageJSPath = path.join(tmpDirPath, cjsPackageJSPath);
117+
cjsIndexJSPath = path.join(tmpDirPath, cjsIndexJSPath);
118+
119+
tmpdir.refresh();
120+
121+
fs.mkdirSync(cjsDirPath, { recursive: true });
122+
fs.writeFileSync(
123+
modulePackageJSPath,
124+
`{
125+
"type": "module"
126+
}`
127+
);
128+
fs.writeFileSync(
129+
cjsPackageJSPath,
130+
`{
131+
"type": "commonjs"
132+
}`
133+
);
134+
fs.writeFileSync(
135+
cjsIndexJSPath,
136+
'const fs = require("fs");'
137+
);
138+
const { code, signal, stderr } = await spawnPromisified(execPath, [cjsIndexJSPath]);
139+
assert.strictEqual(stderr.trim(), '');
140+
assert.strictEqual(code, 0);
141+
assert.strictEqual(signal, null);
142+
});
143+
144+
it('check long path in GetNearestParentPackageJSONType', async () => {
145+
// Module layout will be the following:
146+
// node_modules
147+
// test-module
148+
// package.json (path is less than 260 chars long)
149+
// cjs
150+
// package.json (path is more than 260 chars long)
151+
// index.js
152+
const testModuleDirPath = 'node_modules/test-module';
153+
let cjsDirPath = path.join(testModuleDirPath, 'cjs');
154+
let modulePackageJSPath = path.join(testModuleDirPath, 'package.json');
155+
let cjsPackageJSPath = path.join(cjsDirPath, 'package.json');
156+
let cjsIndexJSPath = path.join(cjsDirPath, 'index.js');
157+
158+
const tmpDirNameLen = Math.max(260 - tmpdir.path.length - cjsPackageJSPath.length, 1);
159+
const tmpDirPath = tmpdir.resolve('l'.repeat(tmpDirNameLen));
160+
161+
cjsDirPath = path.join(tmpDirPath, cjsDirPath);
162+
modulePackageJSPath = path.join(tmpDirPath, modulePackageJSPath);
163+
cjsPackageJSPath = path.join(tmpDirPath, cjsPackageJSPath);
164+
cjsIndexJSPath = path.join(tmpDirPath, cjsIndexJSPath);
165+
166+
tmpdir.refresh();
167+
168+
fs.mkdirSync(cjsDirPath, { recursive: true });
169+
fs.writeFileSync(
170+
modulePackageJSPath,
171+
`{
172+
"type": "module"
173+
}`
174+
);
175+
fs.writeFileSync(
176+
cjsPackageJSPath,
177+
`{
178+
"type": "commonjs"
179+
}`
180+
);
181+
182+
fs.writeFileSync(cjsIndexJSPath, 'import fs from "node:fs/promises";');
183+
const { code, signal, stderr } = await spawnPromisified(execPath, [cjsIndexJSPath]);
184+
185+
assert.ok(stderr.includes('Warning: To load an ES module'));
186+
assert.strictEqual(code, 1);
187+
assert.strictEqual(signal, null);
188+
});
189+
});

0 commit comments

Comments
 (0)