Skip to content

src: fix generation of path objects in Windows#56696

Closed
yamachu wants to merge 2 commits intonodejs:mainfrom
yamachu:fix-windows-path-string
Closed

src: fix generation of path objects in Windows#56696
yamachu wants to merge 2 commits intonodejs:mainfrom
yamachu:fix-windows-path-string

Conversation

@yamachu
Copy link
Copy Markdown
Contributor

@yamachu yamachu commented Jan 22, 2025

fix: #56650
ref: #56657

This PR fixes a problem that caused a segmentation fault in module resolution when creating a require object with multibyte characters in a non-English environment.

Since the issue occurred when generating a std::filesystem::path object, some patches were applied to the changed areas in the following PR.

a7dad43

Since the issue was reproduced only in different locales, such as ja-JP on Windows, I rewrote the locale in the CI of coverage-windows to run the test.
https://github.com/yamachu/node/pull/2/files#diff-29094741d50149aa772b3e577ad509116bad722ad2de85689b6cb2c01e806a46

.github/workflows/coverage-windows.yml

+      - name: Change locale ja-JP for testing on SJIS environment
+        run: Set-WinSystemLocale -SystemLocale "ja-JP"
+      # to avoid configure, nobuild and noprojgen is needed
+      - name: Test on SJIS environment
+        run: ./vcbuild.bat nobuild noprojgen test-ci-js; node -e 'process.exit(0)'
+        env:
+          NODE_V8_COVERAGE: ./coverage/tmp

The previous PR did not solve the problem when using unicode, so the process was separated for windows and the path object was generated.

The part where std::filesystem::path is used can be fixed by changing this PR.

The results of the test conducted in a Japanese environment can be seen below.
https://github.com/yamachu/node/actions/runs/12903928231/job/35980111577#step:12:16844

Loading
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++. commit-queue-failed An error occurred while landing this pull request using GitHub Actions. commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Segmentation Fault When Passing Paths with Japanese Characters to createRequire in Node.js 22+

5 participants