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 anna@addaleax.net
Reviewed-By: Darshan Sen raisinten@gmail.com
Reviewed-By: Stefan Stojanovic stefan.stojanovic@janeasystems.com
Reviewed-By: Juan José Arboleda soyjuanarbol@gmail.com
Reviewed-By: Joyee Cheung joyeec9h3@gmail.com
Reviewed-By: Rafael Gonzaga rafael.nunu@hotmail.com

richardlau and others added 2 commits November 7, 2025 18:05
Original commit message:

    Fix scratch registers passed to mtvsrdd

    `ra` cannot be r0 as it will be interpreted as Operand(0)

    Change-Id: Idce58191f9d3578dc91dc4aa3872a0bf2939d8b3
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6936113
    Commit-Queue: Milad Farazmand <mfarazma@ibm.com>
    Reviewed-by: Junliang Yan <junyan1@ibm.com>
    Cr-Commit-Position: refs/heads/main@{#102388}

Refs: v8/v8@2abc613
PR-URL: nodejs#60177
Refs: nodejs/undici#4530
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
`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 <anna@addaleax.net>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Stefan Stojanovic <stefan.stojanovic@janeasystems.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
@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.

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.

4 participants