fix: crash on windows when UTF-8 is in path#48898
Merged
codebytere merged 1 commit intoelectron:mainfrom Nov 13, 2025
Merged
Conversation
indutny-signal
commented
Nov 11, 2025
| - 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); |
Contributor
Author
There was a problem hiding this comment.
Hm... I might actually need to look into how resources_path computed when strings has unicode in it.
Contributor
Author
There was a problem hiding this comment.
Nvm, it is UTF-8 for both of them so we should be good!
Member
|
Can you move the changes to https://github.com/electron/electron/blob/main/patches/node/fix_ensure_traverseparent_bails_on_resource_path_exit.patch, it will allow for |
indutny-signal
force-pushed
the
fix/crash-on-utf8
branch
from
499ea10 to
3078e2b
Compare
Contributor
Author
|
@deepak1556 good idea. Had to reorder patches for this to make more sense sequentially. Thanks! |
deepak1556
added
semver/patch
VerteDinde
approved these changes
Nov 11, 2025
dsanders11
approved these changes
Nov 12, 2025
In 6399527 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.
indutny-signal
force-pushed
the
fix/crash-on-utf8
branch
from
3078e2b to
680e4d3
Compare
|
Release Notes Persisted
|
Contributor
|
I was unable to backport this PR to "39-x-y" cleanly; |
trop
bot
removed
the
target/39-x-y
Contributor
|
I was unable to backport this PR to "38-x-y" cleanly; |
trop
bot
added
needs-manual-bp/39-x-y
needs-manual-bp/38-x-y
and removed
target/38-x-y
Contributor
|
I have automatically backported this PR to "40-x-y", please check out #48942 |
trop
bot
added
in-flight/40-x-y
and removed
target/40-x-y
Contributor
Author
|
Ah, manual backport here we go. |
5 tasks
Contributor
|
@indutny-signal has manually backported this PR to "39-x-y", please check out #48944 |
Contributor
Author
|
Opened #48944 . Thank you so much! |
5 tasks
Contributor
|
@indutny-signal has manually backported this PR to "38-x-y", please check out #48947 |
trop
bot
added
in-flight/38-x-y
merged/40-x-y
nilayarya
pushed a commit
to nilayarya/electron
that referenced
this pull request
Nov 21, 2025
In 6399527 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.
nilayarya
added a commit
to nilayarya/electron
that referenced
this pull request
Nov 21, 2025
In 6399527 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.
nilayarya
added a commit
to nilayarya/electron
that referenced
this pull request
Nov 21, 2025
In 6399527 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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of Change
In 6399527 we changed the path strings that
node_modules.ccoperates on from single-byte to wide strings. Unfortunately this means thatgeneric_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
ConvertGenericPathToUTF8from the Node.js internal utilities.cc @codebytere @deepak1556 @VerteDinde
Checklist
npm testpassesRelease Notes
Notes: fix crash on windows when UTF-8 is in path