Skip to content

Commit

Permalink
module,win: fix long path resolve
Browse files Browse the repository at this point in the history
PR-URL: #53294
Fixes: #50753
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Vinícius Lourenço Claro Cardoso <[email protected]>
  • Loading branch information
huseyinacacak-janea authored and targos committed Aug 14, 2024
1 parent 37960a6 commit e676d98
Show file tree
Hide file tree
Showing 6 changed files with 250 additions and 67 deletions.
20 changes: 16 additions & 4 deletions src/node_file.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3235,8 +3235,14 @@ void BindingData::LegacyMainResolve(const FunctionCallbackInfo<Value>& args) {

for (int i = 0; i < legacy_main_extensions_with_main_end; i++) {
file_path = *initial_file_path + std::string(legacy_main_extensions[i]);

switch (FilePathIsFile(env, file_path)) {
// TODO(anonrig): Remove this when ToNamespacedPath supports std::string
Local<Value> local_file_path =
Buffer::Copy(env->isolate(), file_path.c_str(), file_path.size())
.ToLocalChecked();
BufferValue buff_file_path(isolate, local_file_path);
ToNamespacedPath(env, &buff_file_path);

switch (FilePathIsFile(env, buff_file_path.ToString())) {
case BindingData::FilePathIsFileReturnType::kIsFile:
return args.GetReturnValue().Set(i);
case BindingData::FilePathIsFileReturnType::kIsNotFile:
Expand Down Expand Up @@ -3272,8 +3278,14 @@ void BindingData::LegacyMainResolve(const FunctionCallbackInfo<Value>& args) {
i < legacy_main_extensions_package_fallback_end;
i++) {
file_path = *initial_file_path + std::string(legacy_main_extensions[i]);

switch (FilePathIsFile(env, file_path)) {
// TODO(anonrig): Remove this when ToNamespacedPath supports std::string
Local<Value> local_file_path =
Buffer::Copy(env->isolate(), file_path.c_str(), file_path.size())
.ToLocalChecked();
BufferValue buff_file_path(isolate, local_file_path);
ToNamespacedPath(env, &buff_file_path);

switch (FilePathIsFile(env, buff_file_path.ToString())) {
case BindingData::FilePathIsFileReturnType::kIsFile:
return args.GetReturnValue().Set(i);
case BindingData::FilePathIsFileReturnType::kIsNotFile:
Expand Down
46 changes: 33 additions & 13 deletions src/node_modules.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include "node_errors.h"
#include "node_external_reference.h"
#include "node_url.h"
#include "path.h"
#include "permission/permission.h"
#include "permission/permission_base.h"
#include "util-inl.h"
Expand Down Expand Up @@ -241,7 +242,7 @@ void BindingData::ReadPackageJSON(const FunctionCallbackInfo<Value>& args) {
Realm* realm = Realm::GetCurrent(args);
auto isolate = realm->isolate();

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

// TODO(StefanStojanovic): Remove ifdef after
// path.toNamespacedPath logic is ported to C++
#ifdef _WIN32
ToNamespacedPath(realm->env(), &path);
auto package_json = GetPackageJSON(
realm, "\\\\?\\" + path.ToString(), is_esm ? &error_context : nullptr);
#else
auto package_json =
GetPackageJSON(realm, path.ToString(), is_esm ? &error_context : nullptr);
#endif
realm, path.ToStringView(), is_esm ? &error_context : nullptr);

if (package_json == nullptr) {
return;
}
Expand Down Expand Up @@ -323,9 +319,21 @@ void BindingData::GetNearestParentPackageJSON(
CHECK(args[0]->IsString());

Realm* realm = Realm::GetCurrent(args);
Utf8Value path_value(realm->isolate(), args[0]);
BufferValue path_value(realm->isolate(), args[0]);
// Check if the path has a trailing slash. If so, add it after
// ToNamespacedPath() as it will be deleted by ToNamespacedPath()
bool slashCheck = path_value.ToStringView().ends_with(
std::filesystem::path::preferred_separator);

ToNamespacedPath(realm->env(), &path_value);

std::string path_value_str = path_value.ToString();
if (slashCheck) {
path_value_str.push_back(std::filesystem::path::preferred_separator);
}

auto package_json =
TraverseParent(realm, std::filesystem::path(path_value.ToString()));
TraverseParent(realm, std::filesystem::path(path_value_str));

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

Realm* realm = Realm::GetCurrent(args);
Utf8Value path(realm->isolate(), args[0]);
BufferValue path_value(realm->isolate(), args[0]);
// Check if the path has a trailing slash. If so, add it after
// ToNamespacedPath() as it will be deleted by ToNamespacedPath()
bool slashCheck = path_value.ToStringView().ends_with(
std::filesystem::path::preferred_separator);

ToNamespacedPath(realm->env(), &path_value);

std::string path_value_str = path_value.ToString();
if (slashCheck) {
path_value_str.push_back(std::filesystem::path::preferred_separator);
}

auto package_json =
TraverseParent(realm, std::filesystem::path(path.ToString()));
TraverseParent(realm, std::filesystem::path(path_value_str));

if (package_json == nullptr) {
return;
Expand Down
42 changes: 0 additions & 42 deletions test/es-module/test-GH-50753.js

This file was deleted.

2 changes: 1 addition & 1 deletion test/es-module/test-cjs-esm-warn.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ const { describe, it } = require('node:test');

const requiringCjsAsEsm = path.resolve(fixtures.path('/es-modules/cjs-esm.js'));
const requiringEsm = path.resolve(fixtures.path('/es-modules/cjs-esm-esm.js'));
const pjson = path.resolve(
const pjson = path.toNamespacedPath(
fixtures.path('/es-modules/package-type-module/package.json')
);

Expand Down
18 changes: 11 additions & 7 deletions test/es-module/test-cjs-legacyMainResolve-permission.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,11 @@ describe('legacyMainResolve', () => {
assert.throws(() => legacyMainResolve(packageJsonUrl, packageConfig, base), {
code: 'ERR_ACCESS_DENIED',
resource: path.resolve(
${JSON.stringify(fixtextureFolderEscaped)},
${JSON.stringify(mainOrFolder)},
resource: path.toNamespacedPath(
path.resolve(
${JSON.stringify(fixtextureFolderEscaped)},
${JSON.stringify(mainOrFolder)},
)
)
});
`,
Expand Down Expand Up @@ -120,10 +122,12 @@ describe('legacyMainResolve', () => {
assert.throws(() => legacyMainResolve(packageJsonUrl, packageConfig, base), {
code: 'ERR_ACCESS_DENIED',
resource: path.resolve(
${JSON.stringify(fixtextureFolderEscaped)},
${JSON.stringify(folder)},
${JSON.stringify(expectedFile)},
resource: path.toNamespacedPath(
path.resolve(
${JSON.stringify(fixtextureFolderEscaped)},
${JSON.stringify(folder)},
${JSON.stringify(expectedFile)},
)
)
});
`,
Expand Down
189 changes: 189 additions & 0 deletions test/es-module/test-esm-long-path-win.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,189 @@
'use strict';

// Flags: --expose-internals

const common = require('../common');
if (!common.isWindows) {
common.skip('this test is Windows-specific.');
}

const fs = require('fs');
const path = require('path');
const tmpdir = require('../common/tmpdir');
const { describe, it } = require('node:test');
const { legacyMainResolve } = require('node:internal/modules/esm/resolve');
const { pathToFileURL } = require('node:url');
const { spawnPromisified } = require('../common');
const assert = require('assert');
const { execPath } = require('node:process');

describe('long path on Windows', () => {
it('check long path in ReadPackageJSON', () => {
// Module layout will be the following:
// package.json
// main
// main.js
const packageDirNameLen = Math.max(260 - tmpdir.path.length, 1);
const packageDirName = tmpdir.resolve('x'.repeat(packageDirNameLen));
const packageDirPath = path.resolve(packageDirName);
const packageJsonFilePath = path.join(packageDirPath, 'package.json');
const mainDirName = 'main';
const mainDirPath = path.resolve(packageDirPath, mainDirName);
const mainJsFile = 'main.js';
const mainJsFilePath = path.resolve(mainDirPath, mainJsFile);

tmpdir.refresh();

fs.mkdirSync(packageDirPath);
fs.writeFileSync(
packageJsonFilePath,
`{"main":"${mainDirName}/${mainJsFile}"}`
);
fs.mkdirSync(mainDirPath);
fs.writeFileSync(mainJsFilePath, 'console.log("hello world");');

require('internal/modules/run_main').executeUserEntryPoint(packageDirPath);

tmpdir.refresh();
});

it('check long path in LegacyMainResolve - 1', () => {
// Module layout will be the following:
// package.json
// index.js
const packageDirNameLen = Math.max(260 - tmpdir.path.length, 1);
const packageDirPath = tmpdir.resolve('y'.repeat(packageDirNameLen));
const packageJSPath = path.join(packageDirPath, 'package.json');
const indexJSPath = path.join(packageDirPath, 'index.js');
const packageConfig = { };

tmpdir.refresh();

fs.mkdirSync(packageDirPath);
fs.writeFileSync(packageJSPath, '');
fs.writeFileSync(indexJSPath, '');

const packageJsonUrl = pathToFileURL(
path.resolve(packageJSPath)
);

console.log(legacyMainResolve(packageJsonUrl, packageConfig, ''));
});

it('check long path in LegacyMainResolve - 2', () => {
// Module layout will be the following:
// package.json
// main.js
const packageDirNameLen = Math.max(260 - tmpdir.path.length, 1);
const packageDirPath = tmpdir.resolve('z'.repeat(packageDirNameLen));
const packageJSPath = path.join(packageDirPath, 'package.json');
const indexJSPath = path.join(packageDirPath, 'main.js');
const packageConfig = { main: 'main.js' };

tmpdir.refresh();

fs.mkdirSync(packageDirPath);
fs.writeFileSync(packageJSPath, '');
fs.writeFileSync(indexJSPath, '');

const packageJsonUrl = pathToFileURL(
path.resolve(packageJSPath)
);

console.log(legacyMainResolve(packageJsonUrl, packageConfig, ''));
});

it('check long path in GetNearestParentPackageJSON', async () => {
// Module layout will be the following:
// node_modules
// test-module
// package.json (path is less than 260 chars long)
// cjs
// package.json (path is more than 260 chars long)
// index.js
const testModuleDirPath = 'node_modules/test-module';
let cjsDirPath = path.join(testModuleDirPath, 'cjs');
let modulePackageJSPath = path.join(testModuleDirPath, 'package.json');
let cjsPackageJSPath = path.join(cjsDirPath, 'package.json');
let cjsIndexJSPath = path.join(cjsDirPath, 'index.js');

const tmpDirNameLen = Math.max(
260 - tmpdir.path.length - cjsPackageJSPath.length, 1);
const tmpDirPath = tmpdir.resolve('k'.repeat(tmpDirNameLen));

cjsDirPath = path.join(tmpDirPath, cjsDirPath);
modulePackageJSPath = path.join(tmpDirPath, modulePackageJSPath);
cjsPackageJSPath = path.join(tmpDirPath, cjsPackageJSPath);
cjsIndexJSPath = path.join(tmpDirPath, cjsIndexJSPath);

tmpdir.refresh();

fs.mkdirSync(cjsDirPath, { recursive: true });
fs.writeFileSync(
modulePackageJSPath,
`{
"type": "module"
}`
);
fs.writeFileSync(
cjsPackageJSPath,
`{
"type": "commonjs"
}`
);
fs.writeFileSync(
cjsIndexJSPath,
'const fs = require("fs");'
);
const { code, signal, stderr } = await spawnPromisified(execPath, [cjsIndexJSPath]);
assert.strictEqual(stderr.trim(), '');
assert.strictEqual(code, 0);
assert.strictEqual(signal, null);
});

it('check long path in GetNearestParentPackageJSONType', async () => {
// Module layout will be the following:
// node_modules
// test-module
// package.json (path is less than 260 chars long)
// cjs
// package.json (path is more than 260 chars long)
// index.js
const testModuleDirPath = 'node_modules/test-module';
let cjsDirPath = path.join(testModuleDirPath, 'cjs');
let modulePackageJSPath = path.join(testModuleDirPath, 'package.json');
let cjsPackageJSPath = path.join(cjsDirPath, 'package.json');
let cjsIndexJSPath = path.join(cjsDirPath, 'index.js');

const tmpDirNameLen = Math.max(260 - tmpdir.path.length - cjsPackageJSPath.length, 1);
const tmpDirPath = tmpdir.resolve('l'.repeat(tmpDirNameLen));

cjsDirPath = path.join(tmpDirPath, cjsDirPath);
modulePackageJSPath = path.join(tmpDirPath, modulePackageJSPath);
cjsPackageJSPath = path.join(tmpDirPath, cjsPackageJSPath);
cjsIndexJSPath = path.join(tmpDirPath, cjsIndexJSPath);

tmpdir.refresh();

fs.mkdirSync(cjsDirPath, { recursive: true });
fs.writeFileSync(
modulePackageJSPath,
`{
"type": "module"
}`
);
fs.writeFileSync(
cjsPackageJSPath,
`{
"type": "commonjs"
}`
);

fs.writeFileSync(cjsIndexJSPath, 'import fs from "node:fs/promises";');
const { code, signal, stderr } = await spawnPromisified(execPath, [cjsIndexJSPath]);

assert.ok(stderr.includes('Warning: To load an ES module'));
assert.strictEqual(code, 1);
assert.strictEqual(signal, null);
});
});

0 comments on commit e676d98

Please sign in to comment.