Skip to content

Conversation

@indutny
Copy link
Member

@indutny indutny commented Nov 8, 2025

src/node_modules.cc needs to be consistent with src/node_file.cc in how it translates the utf8 strings to std::wstring otherwise we might end up in situation where we can read the source code of imported package from disk, but fail to recognize that it is an ESM (or CJS) and cause runtime errors. This type of error is possible on Windows when the path contains unicode characters and "Language for non-Unicode programs" is set to "Chinese (Traditional, Taiwan)".

See: #58768
PR-URL: #60575
Reviewed-By: Anna Henningsen [email protected]
Reviewed-By: Darshan Sen [email protected]
Reviewed-By: Stefan Stojanovic [email protected]
Reviewed-By: Juan José Arboleda [email protected]
Reviewed-By: Joyee Cheung [email protected]
Reviewed-By: Rafael Gonzaga [email protected]

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. v24.x Issues that can be reproduced on v24.x or PRs targeting the v24.x-staging branch. labels Nov 8, 2025
@indutny
Copy link
Member Author

indutny commented Nov 8, 2025

cc @nodejs/lts @addaleax

Copy link
Contributor

@aduh95 aduh95 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm probably saying things you already know, but this backport PR shouldn't land until #60575 has been released on v25.x for at least two weeks.

@indutny
Copy link
Member Author

indutny commented Nov 8, 2025

@aduh95 ah, I didn't know that! well, at least we have the PR open now so that we don't have to do much more work when the commit matures.

`src/node_modules.cc` needs to be consistent with `src/node_file.cc` in
how it translates the utf8 strings to `std::wstring` otherwise we might
end up in situation where we can read the source code of imported
package from disk, but fail to recognize that it is an ESM (or CJS) and
cause runtime errors. This type of error is possible on Windows when the
path contains unicode characters and "Language for non-Unicode programs"
is set to "Chinese (Traditional, Taiwan)".

See: nodejs#58768
PR-URL: nodejs#60575
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Stefan Stojanovic <[email protected]>
Reviewed-By: Juan José Arboleda <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
@targos targos force-pushed the backport-60575-to-24 branch from 6620c44 to b986338 Compare December 5, 2025 14:16
@targos
Copy link
Member

targos commented Dec 5, 2025

Rebased.

@targos
Copy link
Member

targos commented Dec 5, 2025

Backport is no longer needed. I cherry-picked the commit cleanly to v24.x-staging: d92ec21

@targos targos closed this Dec 5, 2025
@indutny-signal
Copy link

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. v24.x Issues that can be reproduced on v24.x or PRs targeting the v24.x-staging branch.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants