Skip to content

Commit e676d98

Browse files
huseyinacacak-janeatargos
authored andcommitted
module,win: fix long path resolve
PR-URL: #53294Fixes: #50753 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
1 parent 37960a6 commit e676d98

6 files changed

Lines changed: 250 additions & 67 deletions

File tree

‎src/node_file.cc‎

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

32363236
for (int i = 0; i < legacy_main_extensions_with_main_end; i++) {
32373237
file_path = *initial_file_path + std::string(legacy_main_extensions[i]);
3238-
3239-
switch (FilePathIsFile(env, file_path)) {
3238+
// TODO(anonrig): Remove this when ToNamespacedPath supports std::string
3239+
Local<Value> local_file_path =
3240+
Buffer::Copy(env->isolate(), file_path.c_str(), file_path.size())
3241+
.ToLocalChecked();
3242+
BufferValue buff_file_path(isolate, local_file_path);
3243+
ToNamespacedPath(env, &buff_file_path);
3244+
3245+
switch (FilePathIsFile(env, buff_file_path.ToString())) {
32403246
case BindingData::FilePathIsFileReturnType::kIsFile:
32413247
return args.GetReturnValue().Set(i);
32423248
case BindingData::FilePathIsFileReturnType::kIsNotFile:
@@ -3272,8 +3278,14 @@ void BindingData::LegacyMainResolve(const FunctionCallbackInfo<Value>& args) {
32723278
i < legacy_main_extensions_package_fallback_end;
32733279
i++) {
32743280
file_path = *initial_file_path + std::string(legacy_main_extensions[i]);
3275-
3276-
switch (FilePathIsFile(env, file_path)) {
3281+
// TODO(anonrig): Remove this when ToNamespacedPath supports std::string
3282+
Local<Value> local_file_path =
3283+
Buffer::Copy(env->isolate(), file_path.c_str(), file_path.size())
3284+
.ToLocalChecked();
3285+
BufferValue buff_file_path(isolate, local_file_path);
3286+
ToNamespacedPath(env, &buff_file_path);
3287+
3288+
switch (FilePathIsFile(env, buff_file_path.ToString())) {
32773289
case BindingData::FilePathIsFileReturnType::kIsFile:
32783290
return args.GetReturnValue().Set(i);
32793291
case BindingData::FilePathIsFileReturnType::kIsNotFile:

‎src/node_modules.cc‎

Lines changed: 33 additions & 13 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-
Utf8Valuepath(isolate, args[0]);
245+
BufferValuepath(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
265+
ToNamespacedPath(realm->env(), &path);
267266
auto package_json = GetPackageJSON(
268-
realm, "\\\\?\\" + path.ToString(), is_esm ? &error_context : nullptr);
269-
#else
270-
auto package_json =
271-
GetPackageJSON(realm, path.ToString(), is_esm ? &error_context : nullptr);
272-
#endif
267+
realm, path.ToStringView(), is_esm ? &error_context : nullptr);
268+
273269
if (package_json == nullptr) {
274270
return;
275271
}
@@ -323,9 +319,21 @@ 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.ToStringView().ends_with(
326+
std::filesystem::path::preferred_separator);
327+
328+
ToNamespacedPath(realm->env(), &path_value);
329+
330+
std::string path_value_str = path_value.ToString();
331+
if (slashCheck) {
332+
path_value_str.push_back(std::filesystem::path::preferred_separator);
333+
}
334+
327335
auto package_json =
328-
TraverseParent(realm, std::filesystem::path(path_value.ToString()));
336+
TraverseParent(realm, std::filesystem::path(path_value_str));
329337

330338
if (package_json != nullptr) {
331339
args.GetReturnValue().Set(package_json->Serialize(realm));
@@ -338,9 +346,21 @@ void BindingData::GetNearestParentPackageJSONType(
338346
CHECK(args[0]->IsString());
339347

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

345365
if (package_json == nullptr) {
346366
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
constrequiringCjsAsEsm=path.resolve(fixtures.path('/es-modules/cjs-esm.js'));
1212
constrequiringEsm=path.resolve(fixtures.path('/es-modules/cjs-esm-esm.js'));
13-
constpjson=path.resolve(
13+
constpjson=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+
constcommon=require('../common');
6+
if(!common.isWindows){
7+
common.skip('this test is Windows-specific.');
8+
}
9+
10+
constfs=require('fs');
11+
constpath=require('path');
12+
consttmpdir=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+
constassert=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+
constpackageDirNameLen=Math.max(260-tmpdir.path.length,1);
27+
constpackageDirName=tmpdir.resolve('x'.repeat(packageDirNameLen));
28+
constpackageDirPath=path.resolve(packageDirName);
29+
constpackageJsonFilePath=path.join(packageDirPath,'package.json');
30+
constmainDirName='main';
31+
constmainDirPath=path.resolve(packageDirPath,mainDirName);
32+
constmainJsFile='main.js';
33+
constmainJsFilePath=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+
constpackageDirNameLen=Math.max(260-tmpdir.path.length,1);
55+
constpackageDirPath=tmpdir.resolve('y'.repeat(packageDirNameLen));
56+
constpackageJSPath=path.join(packageDirPath,'package.json');
57+
constindexJSPath=path.join(packageDirPath,'index.js');
58+
constpackageConfig={};
59+
60+
tmpdir.refresh();
61+
62+
fs.mkdirSync(packageDirPath);
63+
fs.writeFileSync(packageJSPath,'');
64+
fs.writeFileSync(indexJSPath,'');
65+
66+
constpackageJsonUrl=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+
constpackageDirNameLen=Math.max(260-tmpdir.path.length,1);
78+
constpackageDirPath=tmpdir.resolve('z'.repeat(packageDirNameLen));
79+
constpackageJSPath=path.join(packageDirPath,'package.json');
80+
constindexJSPath=path.join(packageDirPath,'main.js');
81+
constpackageConfig={main: 'main.js'};
82+
83+
tmpdir.refresh();
84+
85+
fs.mkdirSync(packageDirPath);
86+
fs.writeFileSync(packageJSPath,'');
87+
fs.writeFileSync(indexJSPath,'');
88+
89+
constpackageJsonUrl=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+
consttestModuleDirPath='node_modules/test-module';
105+
letcjsDirPath=path.join(testModuleDirPath,'cjs');
106+
letmodulePackageJSPath=path.join(testModuleDirPath,'package.json');
107+
letcjsPackageJSPath=path.join(cjsDirPath,'package.json');
108+
letcjsIndexJSPath=path.join(cjsDirPath,'index.js');
109+
110+
consttmpDirNameLen=Math.max(
111+
260-tmpdir.path.length-cjsPackageJSPath.length,1);
112+
consttmpDirPath=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 }=awaitspawnPromisified(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+
consttestModuleDirPath='node_modules/test-module';
153+
letcjsDirPath=path.join(testModuleDirPath,'cjs');
154+
letmodulePackageJSPath=path.join(testModuleDirPath,'package.json');
155+
letcjsPackageJSPath=path.join(cjsDirPath,'package.json');
156+
letcjsIndexJSPath=path.join(cjsDirPath,'index.js');
157+
158+
consttmpDirNameLen=Math.max(260-tmpdir.path.length-cjsPackageJSPath.length,1);
159+
consttmpDirPath=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 }=awaitspawnPromisified(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)