From 1936243ce126c04f13e5adf00f628ff99ee301e1 Mon Sep 17 00:00:00 2001 From: Fedor Indutny <79877362+indutny-signal@users.noreply.github.com> Date: Thu, 13 Nov 2025 13:56:30 -0800 Subject: [PATCH] fix: crash on windows when UTF-8 is in path (#48944) In 6399527761b43abb5a3ea40dd64eb05efad3b9de we changed the path strings that `node_modules.cc` operates on from single-byte to wide strings. Unfortunately this means that `generic_path()` that the "fix: ensure TraverseParent bails on resource path exit" patch was calling was no longer a safe method to call on Windows if the underlying string has unicode characters in it. Here we fix it by using `ConvertGenericPathToUTF8` from the Node.js internal utilities. --- patches/node/.patches | 2 +- .../node/api_remove_deprecated_getisolate.patch | 4 ++-- ...verseparent_bails_on_resource_path_exit.patch | 16 ++++++++-------- ...xpose_readfilesync_override_for_modules.patch | 6 +++--- ...se_cp_utf8_for_wide_file_names_on_win32.patch | 10 +++------- 5 files changed, 17 insertions(+), 21 deletions(-) diff --git a/patches/node/.patches b/patches/node/.patches index 01de6949a00d..6c51988040f8 100644 --- a/patches/node/.patches +++ b/patches/node/.patches @@ -39,7 +39,6 @@ build_change_crdtp_protocoltypetraits_signatures_to_avoid_conflict.patch fix_adjust_wpt_and_webidl_tests_for_enabled_float16array.patch chore_add_createexternalizabletwobytestring_to_globals.patch refactor_attach_cppgc_heap_on_v8_isolate_creation.patch -fix_ensure_traverseparent_bails_on_resource_path_exit.patch cli_move_--trace-atomics-wait_to_eol.patch fix_cppgc_initializing_twice.patch fix_task_starvation_in_inspector_context_test.patch @@ -53,3 +52,4 @@ src_switch_from_get_setprototype_to_get_setprototypev2.patch fix_replace_deprecated_setprototype.patch fix_redefined_macos_sdk_header_symbols.patch src_use_cp_utf8_for_wide_file_names_on_win32.patch +fix_ensure_traverseparent_bails_on_resource_path_exit.patch diff --git a/patches/node/api_remove_deprecated_getisolate.patch b/patches/node/api_remove_deprecated_getisolate.patch index 10b0fce19ace..150c9f1fddf2 100644 --- a/patches/node/api_remove_deprecated_getisolate.patch +++ b/patches/node/api_remove_deprecated_getisolate.patch @@ -592,7 +592,7 @@ index 3c5f38ba4f492749c9d7d82179d2a6563787602b..6e83da3ee975dea431e21209bba9227e data_.Reset(); return ret; diff --git a/src/node_modules.cc b/src/node_modules.cc -index ed22da844a61b14b8580cd3d6bb3a233b8559b38..14f2a35f87e8c2fa17898147d7247cc00c066f35 100644 +index 62050791174563f88b8629d576eed8959b3c2e20..fa04b4a8cdd02a2cad1eaf2e5a848b951d1b1150 100644 --- a/src/node_modules.cc +++ b/src/node_modules.cc @@ -64,7 +64,7 @@ void BindingData::Deserialize(v8::Local context, @@ -604,7 +604,7 @@ index ed22da844a61b14b8580cd3d6bb3a233b8559b38..14f2a35f87e8c2fa17898147d7247cc0 Realm* realm = Realm::GetCurrent(context); BindingData* binding = realm->AddBindingData(holder); CHECK_NOT_NULL(binding); -@@ -656,7 +656,7 @@ void BindingData::CreatePerContextProperties(Local target, +@@ -617,7 +617,7 @@ void BindingData::CreatePerContextProperties(Local target, Realm* realm = Realm::GetCurrent(context); realm->AddBindingData(target); diff --git a/patches/node/fix_ensure_traverseparent_bails_on_resource_path_exit.patch b/patches/node/fix_ensure_traverseparent_bails_on_resource_path_exit.patch index 7c741a545dc1..0b78c4377a59 100644 --- a/patches/node/fix_ensure_traverseparent_bails_on_resource_path_exit.patch +++ b/patches/node/fix_ensure_traverseparent_bails_on_resource_path_exit.patch @@ -8,10 +8,10 @@ resource path. This commit ensures that the TraverseParent function bails out if the parent path is outside of the resource path. diff --git a/src/node_modules.cc b/src/node_modules.cc -index 60b03b1563b230f78d8a9bdd8480da4d2ae90a27..35bfada261258407982d9e24cf7b3e820235c941 100644 +index 947513d5b91e8c13478b25d80a1157edbcd64b64..4e46df6afe6ca57f6df9a64d3fc572450baf7d5c 100644 --- a/src/node_modules.cc +++ b/src/node_modules.cc -@@ -279,8 +279,41 @@ const BindingData::PackageConfig* BindingData::TraverseParent( +@@ -315,8 +315,41 @@ const BindingData::PackageConfig* BindingData::TraverseParent( Realm* realm, const std::filesystem::path& check_path) { std::filesystem::path current_path = check_path; auto env = realm->env(); @@ -47,22 +47,22 @@ index 60b03b1563b230f78d8a9bdd8480da4d2ae90a27..35bfada261258407982d9e24cf7b3e82 + }); + }; + -+ bool did_original_path_start_with_resources_path = starts_with(check_path. -+ generic_string(), resources_path); ++ bool did_original_path_start_with_resources_path = starts_with( ++ ConvertGenericPathToUTF8(check_path), resources_path); + do { current_path = current_path.parent_path(); -@@ -299,6 +332,12 @@ const BindingData::PackageConfig* BindingData::TraverseParent( - return nullptr; +@@ -336,6 +369,12 @@ const BindingData::PackageConfig* BindingData::TraverseParent( + } } + // If current path is outside the resources path, bail. + if (did_original_path_start_with_resources_path && -+ !starts_with(current_path.generic_string(), resources_path)) { ++ !starts_with(ConvertGenericPathToUTF8(current_path), resources_path)) { + return nullptr; + } + // Check if the path ends with `/node_modules` - if (current_path.generic_string().ends_with("/node_modules")) { + if (current_path.filename() == "node_modules") { return nullptr; diff --git a/patches/node/fix_expose_readfilesync_override_for_modules.patch b/patches/node/fix_expose_readfilesync_override_for_modules.patch index f2671eac0e4b..6d6bf830b9f3 100644 --- a/patches/node/fix_expose_readfilesync_override_for_modules.patch +++ b/patches/node/fix_expose_readfilesync_override_for_modules.patch @@ -20,7 +20,7 @@ index 82225b0a53dd828750991a4e15a060b736b6ea2b..4b0d31356a2496a7fc67876a22da2453 V(performance_entry_callback, v8::Function) \ V(prepare_stack_trace_callback, v8::Function) \ diff --git a/src/node_modules.cc b/src/node_modules.cc -index 35bfada261258407982d9e24cf7b3e820235c941..ed22da844a61b14b8580cd3d6bb3a233b8559b38 100644 +index 60b03b1563b230f78d8a9bdd8480da4d2ae90a27..62050791174563f88b8629d576eed8959b3c2e20 100644 --- a/src/node_modules.cc +++ b/src/node_modules.cc @@ -21,6 +21,7 @@ namespace modules { @@ -90,7 +90,7 @@ index 35bfada261258407982d9e24cf7b3e820235c941..ed22da844a61b14b8580cd3d6bb3a233 void BindingData::ReadPackageJSON(const FunctionCallbackInfo& args) { CHECK_GE(args.Length(), 1); // path, [is_esm, base, specifier] CHECK(args[0]->IsString()); // path -@@ -597,6 +633,8 @@ void InitImportMetaPathHelpers(const FunctionCallbackInfo& args) { +@@ -558,6 +594,8 @@ void InitImportMetaPathHelpers(const FunctionCallbackInfo& args) { void BindingData::CreatePerIsolateProperties(IsolateData* isolate_data, Local target) { Isolate* isolate = isolate_data->isolate(); @@ -99,7 +99,7 @@ index 35bfada261258407982d9e24cf7b3e820235c941..ed22da844a61b14b8580cd3d6bb3a233 SetMethod(isolate, target, "readPackageJSON", ReadPackageJSON); SetMethod(isolate, target, -@@ -635,6 +673,8 @@ void BindingData::CreatePerContextProperties(Local target, +@@ -596,6 +634,8 @@ void BindingData::CreatePerContextProperties(Local target, void BindingData::RegisterExternalReferences( ExternalReferenceRegistry* registry) { diff --git a/patches/node/src_use_cp_utf8_for_wide_file_names_on_win32.patch b/patches/node/src_use_cp_utf8_for_wide_file_names_on_win32.patch index d4cae40fec5b..f9beaeb85ed3 100644 --- a/patches/node/src_use_cp_utf8_for_wide_file_names_on_win32.patch +++ b/patches/node/src_use_cp_utf8_for_wide_file_names_on_win32.patch @@ -136,10 +136,10 @@ index 75be21c9e8b413f522240a906da06d26c44d5b71..e94c2b5f2cf7cac413cd5cb782fa1cca std::make_error_code(std::errc::file_exists), "cp", diff --git a/src/node_modules.cc b/src/node_modules.cc -index 14f2a35f87e8c2fa17898147d7247cc00c066f35..871282c6f8780ee0bca1e7230c0c2d83fd0c98c0 100644 +index fa04b4a8cdd02a2cad1eaf2e5a848b951d1b1150..947513d5b91e8c13478b25d80a1157edbcd64b64 100644 --- a/src/node_modules.cc +++ b/src/node_modules.cc -@@ -360,12 +360,13 @@ const BindingData::PackageConfig* BindingData::TraverseParent( +@@ -327,22 +327,24 @@ const BindingData::PackageConfig* BindingData::TraverseParent( // Stop the search when the process doesn't have permissions // to walk upwards @@ -158,10 +158,6 @@ index 14f2a35f87e8c2fa17898147d7247cc00c066f35..871282c6f8780ee0bca1e7230c0c2d83 + } } - // If current path is outside the resources path, bail. -@@ -375,13 +376,14 @@ const BindingData::PackageConfig* BindingData::TraverseParent( - } - // Check if the path ends with `/node_modules` - if (current_path.generic_string().ends_with("/node_modules")) { + if (current_path.filename() == "node_modules") { @@ -176,7 +172,7 @@ index 14f2a35f87e8c2fa17898147d7247cc00c066f35..871282c6f8780ee0bca1e7230c0c2d83 if (package_json != nullptr) { return package_json; } -@@ -403,20 +405,12 @@ void BindingData::GetNearestParentPackageJSONType( +@@ -364,20 +366,12 @@ void BindingData::GetNearestParentPackageJSONType( ToNamespacedPath(realm->env(), &path_value);