FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

module,win: fix long path resolve · nodejs/node@e676d98 · GitHub

/ node Public

Commit e676d98

Browse files
huseyinacacak-janea
authored and
targos
committed
module,win: fix long path resolve
PR-URL: #53294 Fixes: #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-
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
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
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
`,

‎test/es-module/test-esm-long-path-win.js‎

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)
Back | FazBrowse Home | New Git URL