mirrors/electron
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages 1 Wiki Activity Actions 1

fix: rework and improve legacyMainResolve patch (#45754)

Browse source 
fix: rework and improve legacyMainResolve patch
This commit is contained in:
Shelley Vohr 2025-02-22 17:06:04 +01:00 • committed by GitHub
parent e3f61b465d
commit 3e51ee516e
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 131 additions and 173 deletions
Download patch file Download diff file Expand all files Collapse all files

2
patches/node/.patches
View file

@ -18,7 +18,7 @@ test_formally_mark_some_tests_as_flaky.patch
fix_do_not_resolve_electron_entrypoints.patch
ci_ensure_node_tests_set_electron_run_as_node.patch
fix_assert_module_in_the_renderer_process.patch
fix_revert_src_lb_reducing_c_calls_of_esm_legacy_main_resolve.patch
fix_allow_passing_fileexists_fn_to_legacymainresolve.patch
fix_remove_deprecated_errno_constants.patch
build_enable_perfetto.patch
fix_add_source_location_for_v8_task_runner.patch

129
patches/node/fix_allow_passing_fileexists_fn_to_legacymainresolve.patch Normal file
View file

@ -0,0 +1,129 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Shelley Vohr <shelley.vohr@gmail.com>
Date: Sat, 17 Feb 2024 22:17:13 -0800
Subject: fix: allow passing fileExists fn to legacyMainResolve
This switch to native legacyMainResolve doesn't take asar into account, and can
cause errors when a project using ESM and asar tries to load a dependency which
uses commonJS.
We can fix this by allowing the C++ implementation of legacyMainResolve to use
a fileExists function that does take Asar into account.
diff --git a/lib/internal/modules/esm/resolve.js b/lib/internal/modules/esm/resolve.js
index 2879e5cf541fb4d226cfd7cc0fe367ca448fb926..03082f0ec4f91382933eec48e77331cdf6f04943 100644
--- a/lib/internal/modules/esm/resolve.js
+++ b/lib/internal/modules/esm/resolve.js
@@ -28,14 +28,13 @@ const { BuiltinModule } = require('internal/bootstrap/realm');
const fs = require('fs');
const { getOptionValue } = require('internal/options');
// Do not eagerly grab .manifest, it may be in TDZ
-const { sep, posix: { relative: relativePosixPath }, resolve } = require('path');
+const { sep, posix: { relative: relativePosixPath }, toNamespacedPath, resolve } = require('path');
const preserveSymlinks = getOptionValue('--preserve-symlinks');
const preserveSymlinksMain = getOptionValue('--preserve-symlinks-main');
const inputTypeFlag = getOptionValue('--input-type');
-const { URL, pathToFileURL, fileURLToPath, isURL, URLParse } = require('internal/url');
+const { URL, pathToFileURL, fileURLToPath, isURL, URLParse, toPathIfFileURL } = require('internal/url');
const { getCWDURL, setOwnProperty } = require('internal/util');
const { canParse: URLCanParse } = internalBinding('url');
-const { legacyMainResolve: FSLegacyMainResolve } = internalBinding('fs');
const {
ERR_INPUT_TYPE_NOT_ALLOWED,
ERR_INVALID_ARG_TYPE,
@@ -183,6 +182,11 @@ const legacyMainResolveExtensionsIndexes = {
kResolvedByPackageAndNode: 9,
};
+function fileExists(url) {
+ const namespaced = toNamespacedPath(toPathIfFileURL(url));
+ return internalFsBinding.internalModuleStat(internalFsBinding, namespaced) === 0;
+}
+
/**
* Legacy CommonJS main resolution:
* 1. let M = pkg_url + (json main field)
@@ -201,7 +205,7 @@ function legacyMainResolve(packageJSONUrl, packageConfig, base) {
const baseStringified = isURL(base) ? base.href : base;
- const resolvedOption = FSLegacyMainResolve(pkgPath, packageConfig.main, baseStringified);
+ const resolvedOption = internalFsBinding.legacyMainResolve(pkgPath, packageConfig.main, baseStringified, fileExists);
const maybeMain = resolvedOption <= legacyMainResolveExtensionsIndexes.kResolvedByMainIndexNode ?
packageConfig.main || './' : '';
diff --git a/src/node_file.cc b/src/node_file.cc
index 1d22e19f16d5ad82466b0724971b2e4a685682f7..9619d10710ffbbdc73fa7d59d1b797c8d0b3a956 100644
--- a/src/node_file.cc
+++ b/src/node_file.cc
@@ -3220,13 +3220,25 @@ static void CpSyncCheckPaths(const FunctionCallbackInfo<Value>& args) {
}
BindingData::FilePathIsFileReturnType BindingData::FilePathIsFile(
- Environment* env, const std::string& file_path) {
+ Environment* env, const std::string& file_path, v8::Local<v8::Function> is_file_function) {
THROW_IF_INSUFFICIENT_PERMISSIONS(
env,
permission::PermissionScope::kFileSystemRead,
file_path,
BindingData::FilePathIsFileReturnType::kThrowInsufficientPermissions);
+ if (!is_file_function.IsEmpty()) {
+ v8::Local<v8::Value> argv[] = {v8::String::NewFromUtf8(
+ env->isolate(), file_path.c_str(), v8::NewStringType::kNormal)
+ .ToLocalChecked()};
+ MaybeLocal<Value> maybe_is_file = is_file_function->Call(env->context(), v8::Undefined(env->isolate()), 1, argv);
+ if (maybe_is_file.IsEmpty()) {
+ bool is_file = maybe_is_file.ToLocalChecked()->BooleanValue(env->isolate());
+ return is_file ? BindingData::FilePathIsFileReturnType::kIsFile
+ : BindingData::FilePathIsFileReturnType::kIsNotFile;
+ }
+ }
+
uv_fs_t req;
int rc = uv_fs_stat(env->event_loop(), &req, file_path.c_str(), nullptr);
@@ -3284,6 +3296,11 @@ void BindingData::LegacyMainResolve(const FunctionCallbackInfo<Value>& args) {
std::optional<std::string> initial_file_path;
std::string file_path;
+ v8::Local<v8::Function> is_file_function;
+ if (args.Length() >= 3 && args[3]->IsFunction()) {
+ is_file_function = args[3].As<v8::Function>();
+ }
+
if (args.Length() >= 2 && args[1]->IsString()) {
auto package_config_main = Utf8Value(isolate, args[1]).ToString();
@@ -3304,7 +3321,7 @@ void BindingData::LegacyMainResolve(const FunctionCallbackInfo<Value>& args) {
BufferValue buff_file_path(isolate, local_file_path);
ToNamespacedPath(env, &buff_file_path);
- switch (FilePathIsFile(env, buff_file_path.ToString())) {
+ switch (FilePathIsFile(env, buff_file_path.ToString(), is_file_function)) {
case BindingData::FilePathIsFileReturnType::kIsFile:
return args.GetReturnValue().Set(i);
case BindingData::FilePathIsFileReturnType::kIsNotFile:
@@ -3341,7 +3358,7 @@ void BindingData::LegacyMainResolve(const FunctionCallbackInfo<Value>& args) {
BufferValue buff_file_path(isolate, local_file_path);
ToNamespacedPath(env, &buff_file_path);
- switch (FilePathIsFile(env, buff_file_path.ToString())) {
+ switch (FilePathIsFile(env, buff_file_path.ToString(), is_file_function)) {
case BindingData::FilePathIsFileReturnType::kIsFile:
return args.GetReturnValue().Set(i);
case BindingData::FilePathIsFileReturnType::kIsNotFile:
diff --git a/src/node_file.h b/src/node_file.h
index bdad1ae25f4892cbbfd8cc30c4d8b4a6f600edbc..099488319f53bc7718313d6e30df2237cad6771d 100644
--- a/src/node_file.h
+++ b/src/node_file.h
@@ -101,7 +101,8 @@ class BindingData : public SnapshotableObject {
InternalFieldInfo* internal_field_info_ = nullptr;
static FilePathIsFileReturnType FilePathIsFile(Environment* env,
- const std::string& file_path);
+ const std::string& file_path,
+ v8::Local<v8::Function> is_file_function = v8::Local<v8::Function>());
};
// structure used to store state during a complex operation, e.g., mkdirp.

170
patches/node/fix_revert_src_lb_reducing_c_calls_of_esm_legacy_main_resolve.patch
View file

@ -1,170 +0,0 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: VerteDinde <vertedinde@electronjs.org>
Date: Sat, 17 Feb 2024 22:17:13 -0800
Subject: fix: revert "src,lb: reducing C++ calls of esm legacy main resolve"
This switch to native legacyMainResolve doesn't take asar into account, and can
cause errors when a project using ESM and asar tries to load a dependency which
uses commonJS. This will need to be fixed forward, but revert for Electron 29's
stable release to avoid potentially breaking apps with a riskier fix.
This patch can be removed when node's
native implementation has been patched
to recognize asar files.
This reverts commit 9cf2e1f55b8446a7cde23699d00a3be73aa0c8f1.
diff --git a/lib/internal/modules/esm/resolve.js b/lib/internal/modules/esm/resolve.js
index 2879e5cf541fb4d226cfd7cc0fe367ca448fb926..19df6fab3e118ac237f930b1e5d41c02c9050fa2 100644
--- a/lib/internal/modules/esm/resolve.js
+++ b/lib/internal/modules/esm/resolve.js
@@ -28,15 +28,15 @@ const { BuiltinModule } = require('internal/bootstrap/realm');
const fs = require('fs');
const { getOptionValue } = require('internal/options');
// Do not eagerly grab .manifest, it may be in TDZ
-const { sep, posix: { relative: relativePosixPath }, resolve } = require('path');
+const { sep, posix: { relative: relativePosixPath }, toNamespacedPath, resolve } = require('path');
const preserveSymlinks = getOptionValue('--preserve-symlinks');
const preserveSymlinksMain = getOptionValue('--preserve-symlinks-main');
const inputTypeFlag = getOptionValue('--input-type');
-const { URL, pathToFileURL, fileURLToPath, isURL, URLParse } = require('internal/url');
+const { URL, pathToFileURL, fileURLToPath, isURL, URLParse, toPathIfFileURL } = require('internal/url');
const { getCWDURL, setOwnProperty } = require('internal/util');
const { canParse: URLCanParse } = internalBinding('url');
-const { legacyMainResolve: FSLegacyMainResolve } = internalBinding('fs');
const {
+ ERR_ACCESS_DENIED,
ERR_INPUT_TYPE_NOT_ALLOWED,
ERR_INVALID_ARG_TYPE,
ERR_INVALID_MODULE_SPECIFIER,
@@ -53,6 +53,7 @@ const { Module: CJSModule } = require('internal/modules/cjs/loader');
const { getConditionsSet } = require('internal/modules/esm/utils');
const packageJsonReader = require('internal/modules/package_json_reader');
const internalFsBinding = internalBinding('fs');
+const permission = require('internal/process/permission');
/**
* @typedef {import('internal/modules/esm/package_config.js').PackageConfig} PackageConfig
@@ -183,6 +184,20 @@ const legacyMainResolveExtensionsIndexes = {
kResolvedByPackageAndNode: 9,
};
+/**
+ * @param {string | URL} url
+ * @returns {boolean}
+ */
+function fileExists(url) {
+ const { Buffer } = require('buffer');
+ const namespaced = toNamespacedPath(toPathIfFileURL(url));
+ if (permission.isEnabled() && !permission.has('fs.read', namespaced)) {
+ const resource = Buffer.isBuffer(namespaced) ? Buffer.toString(namespaced) : namespaced;
+ throw new ERR_ACCESS_DENIED('Access to this API has been restricted', 'FileSystemRead', resource);
+ }
+ return internalFsBinding.internalModuleStat(internalFsBinding, namespaced) === 0;
+}
+
/**
* Legacy CommonJS main resolution:
* 1. let M = pkg_url + (json main field)
@@ -199,18 +214,45 @@ function legacyMainResolve(packageJSONUrl, packageConfig, base) {
assert(isURL(packageJSONUrl));
const pkgPath = fileURLToPath(new URL('.', packageJSONUrl));
- const baseStringified = isURL(base) ? base.href : base;
-
- const resolvedOption = FSLegacyMainResolve(pkgPath, packageConfig.main, baseStringified);
-
- const maybeMain = resolvedOption <= legacyMainResolveExtensionsIndexes.kResolvedByMainIndexNode ?
- packageConfig.main || './' : '';
- const resolvedPath = resolve(pkgPath, maybeMain + legacyMainResolveExtensions[resolvedOption]);
- const resolvedUrl = pathToFileURL(resolvedPath);
-
- emitLegacyIndexDeprecation(resolvedUrl, resolvedPath, pkgPath, base, packageConfig.main);
+ let guess;
+ if (packageConfig.main !== undefined) {
+ // Note: fs check redundances will be handled by Descriptor cache here.
+ if (fileExists(guess = new URL(`./${packageConfig.main}`,
+ packageJSONUrl))) {
+ return guess;
+ } else if (fileExists(guess = new URL(`./${packageConfig.main}.js`,
+ packageJSONUrl)));
+ else if (fileExists(guess = new URL(`./${packageConfig.main}.json`,
+ packageJSONUrl)));
+ else if (fileExists(guess = new URL(`./${packageConfig.main}.node`,
+ packageJSONUrl)));
+ else if (fileExists(guess = new URL(`./${packageConfig.main}/index.js`,
+ packageJSONUrl)));
+ else if (fileExists(guess = new URL(`./${packageConfig.main}/index.json`,
+ packageJSONUrl)));
+ else if (fileExists(guess = new URL(`./${packageConfig.main}/index.node`,
+ packageJSONUrl)));
+ else guess = undefined;
+ if (guess) {
+ emitLegacyIndexDeprecation(guess, fileURLToPath(guess), pkgPath,
+ base, packageConfig.main);
+ return guess;
+ }
+ }
- return resolvedUrl;
+ // Fallthrough.
+ if (fileExists(guess = new URL('./index.js', packageJSONUrl)));
+ // So fs.
+ else if (fileExists(guess = new URL('./index.json', packageJSONUrl)));
+ else if (fileExists(guess = new URL('./index.node', packageJSONUrl)));
+ else guess = undefined;
+ if (guess) {
+ emitLegacyIndexDeprecation(guess, fileURLToPath(guess), pkgPath,
+ base, packageConfig.main);
+ return guess;
+ }
+ // Not found.
+ throw new ERR_MODULE_NOT_FOUND(pkgPath, fileURLToPath(base), null);
}
const encodedSepRegEx = /%2F|%5C/i;
diff --git a/test/es-module/test-cjs-legacyMainResolve.js b/test/es-module/test-cjs-legacyMainResolve.js
index edb567bce403f2d4df482c2549c6f7cec78c3588..4567ddc3715ac0d11facb0b567c5f5763699f4c9 100644
--- a/test/es-module/test-cjs-legacyMainResolve.js
+++ b/test/es-module/test-cjs-legacyMainResolve.js
@@ -129,7 +129,7 @@ describe('legacyMainResolve', () => {
);
assert.throws(
() => legacyMainResolve(packageJsonUrl, { main: null }, packageJsonUrl),
- { message: /index\.js/, code: 'ERR_MODULE_NOT_FOUND' },
+ { code: 'ERR_MODULE_NOT_FOUND' },
);
});
@@ -137,7 +137,7 @@ describe('legacyMainResolve', () => {
const packageJsonUrl = pathToFileURL('/c/file%20with%20percents/package.json');
assert.throws(
() => legacyMainResolve(packageJsonUrl, { main: null }, packageJsonUrl),
- { message: /index\.js/, code: 'ERR_MODULE_NOT_FOUND' },
+ { code: 'ERR_MODULE_NOT_FOUND' },
);
});
@@ -150,7 +150,7 @@ describe('legacyMainResolve', () => {
);
assert.throws(
() => legacyMainResolve(packageJsonUrl, { main: './index.node' }, packageJsonUrl),
- { message: /index\.node/, code: 'ERR_MODULE_NOT_FOUND' },
+ { code: 'ERR_MODULE_NOT_FOUND' },
);
});
@@ -163,11 +163,11 @@ describe('legacyMainResolve', () => {
);
assert.throws(
() => legacyMainResolve(packageJsonUrl, { main: null }, undefined),
- { message: /"base" argument must be/, code: 'ERR_INVALID_ARG_TYPE' },
+ { message: 'The "path" argument must be of type string or an instance of URL. Received undefined', code: 'ERR_INVALID_ARG_TYPE' },
);
});
- it('should interpret main as a path, not a URL', () => {
+ it.skip('should interpret main as a path, not a URL', () => {
const packageJsonUrl = fixtures.fileURL('/es-modules/legacy-main-resolver/package.json');
assert.deepStrictEqual(
legacyMainResolve(packageJsonUrl, { main: '../folder%25with percentage#/' }, packageJsonUrl),

2
patches/node/src_stop_using_deprecated_fields_of_fastapicallbackoptions.patch
View file

@ -40,7 +40,7 @@ index 0f0cde7be431dcb80c5314b1a9da49886c436d1c..f6d2bd439cad8b9f91c9d9a6cdb302e6
}
HistogramBase* histogram;
diff --git a/src/node_file.cc b/src/node_file.cc
index 1d22e19f16d5ad82466b0724971b2e4a685682f7..7531664c37833da9804d24c642a38a60c336c2c7 100644
index 9619d10710ffbbdc73fa7d59d1b797c8d0b3a956..f2a5c0939b60c582dbbecc07add1e903a9183b95 100644
--- a/src/node_file.cc
+++ b/src/node_file.cc
@@ -1061,13 +1061,8 @@ static int32_t FastInternalModuleStat(

1
script/node-disabled-tests.json
View file

@ -72,7 +72,6 @@
"parallel/test-snapshot-weak-reference",
"parallel/test-snapshot-worker",
"parallel/test-strace-openat-openssl",
"parallel/test-find-package-json",
"parallel/test-tls-alpn-server-client",
"parallel/test-tls-cli-min-version-1.0",
"parallel/test-tls-cli-max-version-1.2",