From d0e410666db344fe7bb8ee136d428e8ee9255b3d Mon Sep 17 00:00:00 2001 From: islandryu Date: Fri, 15 Nov 2024 17:06:46 +0900 Subject: [PATCH 1/8] src: use wide string for findPackageJson onWindows Fix error when searching for package.json with non-ASCII characters in paths Fixes: https://github.com/nodejs/node/issues/55773 --- src/node_file.cc | 49 --------------------------------- src/node_modules.cc | 15 ++++------ src/util.cc | 39 ++++++++++++++++++++++++++ src/util.h | 22 +++++++++++++++ test/parallel/test-non-ascii.js | 20 ++++++++++++++ 5 files changed, 87 insertions(+), 58 deletions(-) create mode 100644 test/parallel/test-non-ascii.js diff --git a/src/node_file.cc b/src/node_file.cc index 5a50aacb1b939d..f77cc88fcb4246 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -3125,55 +3125,6 @@ static void GetFormatOfExtensionlessFile( return args.GetReturnValue().Set(EXTENSIONLESS_FORMAT_JAVASCRIPT); } -#ifdef _WIN32 -std::wstring ConvertToWideString(const std::string& str) { - int size_needed = MultiByteToWideChar( - CP_UTF8, 0, &str[0], static_cast(str.size()), nullptr, 0); - std::wstring wstrTo(size_needed, 0); - MultiByteToWideChar(CP_UTF8, - 0, - &str[0], - static_cast(str.size()), - &wstrTo[0], - size_needed); - return wstrTo; -} - -#define BufferValueToPath(str) \ - std::filesystem::path(ConvertToWideString(str.ToString())) - -std::string ConvertWideToUTF8(const std::wstring& wstr) { - if (wstr.empty()) return std::string(); - - int size_needed = WideCharToMultiByte(CP_UTF8, - 0, - &wstr[0], - static_cast(wstr.size()), - nullptr, - 0, - nullptr, - nullptr); - std::string strTo(size_needed, 0); - WideCharToMultiByte(CP_UTF8, - 0, - &wstr[0], - static_cast(wstr.size()), - &strTo[0], - size_needed, - nullptr, - nullptr); - return strTo; -} - -#define PathToString(path) ConvertWideToUTF8(path.wstring()); - -#else // _WIN32 - -#define BufferValueToPath(str) std::filesystem::path(str.ToStringView()); -#define PathToString(path) path.native(); - -#endif // _WIN32 - static void CpSyncCheckPaths(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); Isolate* isolate = env->isolate(); diff --git a/src/node_modules.cc b/src/node_modules.cc index 94ed9bc4b3c157..4678061a9321e3 100644 --- a/src/node_modules.cc +++ b/src/node_modules.cc @@ -337,14 +337,13 @@ void BindingData::GetNearestParentPackageJSON( bool slashCheck = path_value.ToStringView().ends_with(kPathSeparator); ToNamespacedPath(realm->env(), &path_value); - - std::string path_value_str = path_value.ToString(); + std::filesystem::path path = BufferValueToPath(path_value); if (slashCheck) { - path_value_str.push_back(kPathSeparator); + path += kPathSeparator; } auto package_json = - TraverseParent(realm, std::filesystem::path(path_value_str)); + TraverseParent(realm, path); if (package_json != nullptr) { args.GetReturnValue().Set(package_json->Serialize(realm)); @@ -363,14 +362,12 @@ void BindingData::GetNearestParentPackageJSONType( bool slashCheck = path_value.ToStringView().ends_with(kPathSeparator); ToNamespacedPath(realm->env(), &path_value); - - std::string path_value_str = path_value.ToString(); + std::filesystem::path path = BufferValueToPath(path_value); if (slashCheck) { - path_value_str.push_back(kPathSeparator); + path += kPathSeparator; } - auto package_json = - TraverseParent(realm, std::filesystem::path(path_value_str)); + auto package_json = TraverseParent(realm, path); if (package_json == nullptr) { return; diff --git a/src/util.cc b/src/util.cc index a372eb4d88ca1d..531a5330c2cb74 100644 --- a/src/util.cc +++ b/src/util.cc @@ -885,4 +885,43 @@ v8::Maybe GetValidFileMode(Environment* env, return v8::Just(mode); } +#ifdef _WIN32 +std::wstring ConvertToWideString(const std::string& str) { + int size_needed = MultiByteToWideChar( + CP_UTF8, 0, &str[0], static_cast(str.size()), nullptr, 0); + std::wstring wstrTo(size_needed, 0); + MultiByteToWideChar(CP_UTF8, + 0, + &str[0], + static_cast(str.size()), + &wstrTo[0], + size_needed); + return wstrTo; +} + +std::string ConvertWideToUTF8(const std::wstring& wstr) { + if (wstr.empty()) return std::string(); + + int size_needed = WideCharToMultiByte(CP_UTF8, + 0, + &wstr[0], + static_cast(wstr.size()), + nullptr, + 0, + nullptr, + nullptr); + std::string strTo(size_needed, 0); + WideCharToMultiByte(CP_UTF8, + 0, + &wstr[0], + static_cast(wstr.size()), + &strTo[0], + size_needed, + nullptr, + nullptr); + return strTo; +} + +#endif // _WIN32 + } // namespace node diff --git a/src/util.h b/src/util.h index b1f316eebc7199..d32c71cd70aee8 100644 --- a/src/util.h +++ b/src/util.h @@ -49,6 +49,11 @@ #include #include #include +#ifdef _WIN32 +#include +#else +#include +#endif #ifdef __GNUC__ #define MUST_USE_RESULT __attribute__((warn_unused_result)) @@ -1013,6 +1018,23 @@ v8::Maybe GetValidFileMode(Environment* env, // case insensitive. inline bool IsWindowsBatchFile(const char* filename); +#ifdef _WIN32 +std::wstring ConvertToWideString(const std::string& str); + +#define BufferValueToPath(str) \ + std::filesystem::path(ConvertToWideString(str.ToString())) + +std::string ConvertWideToUTF8(const std::wstring& wstr); + +#define PathToString(path) ConvertWideToUTF8(path.wstring()); + +#else // _WIN32 + +#define BufferValueToPath(str) std::filesystem::path(str.ToStringView()); +#define PathToString(path) path.native(); + +#endif // _WIN32 + } // namespace node #endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS diff --git a/test/parallel/test-non-ascii.js b/test/parallel/test-non-ascii.js new file mode 100644 index 00000000000000..6ae62b56ff571a --- /dev/null +++ b/test/parallel/test-non-ascii.js @@ -0,0 +1,20 @@ +'use strict'; + +const fs = require('node:fs'); +const common = require('../common'); +const tmpdir = require('../common/tmpdir'); +const { describe, it } = require('node:test'); +const assert = require('node:assert'); + +describe('None ASCII', () => { + it('should be able to run on a directory with non-ASCII characters', async () => { + fs.mkdirSync(tmpdir.resolve('12月'), { recursive: true }); + fs.writeFileSync( + tmpdir.resolve('12月/index.js'), + "console.log('12月');", + ); + await common.spawnPromisified(process.execPath, [tmpdir.resolve('12月/index.js')]).then(common.mustCall((result) => { + assert.strictEqual(result.stdout, '12月' + '\n'); + })); + }); +}); \ No newline at end of file From 8b20a68d12b7c1c31fc95b082a3467762cff009b Mon Sep 17 00:00:00 2001 From: Shima Ryuhei <65934663+islandryu@users.noreply.github.com> Date: Fri, 15 Nov 2024 23:26:10 +0900 Subject: [PATCH 2/8] Update test/parallel/test-non-ascii.js Co-authored-by: Aviv Keller --- test/parallel/test-non-ascii.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/parallel/test-non-ascii.js b/test/parallel/test-non-ascii.js index 6ae62b56ff571a..37220a11650115 100644 --- a/test/parallel/test-non-ascii.js +++ b/test/parallel/test-non-ascii.js @@ -6,8 +6,7 @@ const tmpdir = require('../common/tmpdir'); const { describe, it } = require('node:test'); const assert = require('node:assert'); -describe('None ASCII', () => { - it('should be able to run on a directory with non-ASCII characters', async () => { +test('Running from a directory with non-ASCII characters', async () => { fs.mkdirSync(tmpdir.resolve('12月'), { recursive: true }); fs.writeFileSync( tmpdir.resolve('12月/index.js'), From de6fba1519367fbffcbbc31362b527c936c4def6 Mon Sep 17 00:00:00 2001 From: Shima Ryuhei <65934663+islandryu@users.noreply.github.com> Date: Fri, 15 Nov 2024 23:26:22 +0900 Subject: [PATCH 3/8] Update test/parallel/test-non-ascii.js Co-authored-by: Aviv Keller --- test/parallel/test-non-ascii.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-non-ascii.js b/test/parallel/test-non-ascii.js index 37220a11650115..2d43a8c7d89450 100644 --- a/test/parallel/test-non-ascii.js +++ b/test/parallel/test-non-ascii.js @@ -3,7 +3,7 @@ const fs = require('node:fs'); const common = require('../common'); const tmpdir = require('../common/tmpdir'); -const { describe, it } = require('node:test'); +const { test } = require('node:test'); const assert = require('node:assert'); test('Running from a directory with non-ASCII characters', async () => { From dd8ceacce3fe3cdbec649cf3b28e379c0f6913cd Mon Sep 17 00:00:00 2001 From: Shima Ryuhei <65934663+islandryu@users.noreply.github.com> Date: Fri, 15 Nov 2024 23:26:32 +0900 Subject: [PATCH 4/8] Update test/parallel/test-non-ascii.js Co-authored-by: Aviv Keller --- test/parallel/test-non-ascii.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/test/parallel/test-non-ascii.js b/test/parallel/test-non-ascii.js index 2d43a8c7d89450..f7ec127a3321f8 100644 --- a/test/parallel/test-non-ascii.js +++ b/test/parallel/test-non-ascii.js @@ -12,8 +12,7 @@ test('Running from a directory with non-ASCII characters', async () => { tmpdir.resolve('12月/index.js'), "console.log('12月');", ); - await common.spawnPromisified(process.execPath, [tmpdir.resolve('12月/index.js')]).then(common.mustCall((result) => { - assert.strictEqual(result.stdout, '12月' + '\n'); - })); + const { stdout } = await common.spawnPromisified(process.execPath, [tmpdir.resolve('12月/index.js')]); + assert.strictEqual(stdout, '12月\n'); }); }); \ No newline at end of file From a4f683035ff09ceb849885c5c91299b6816ffb89 Mon Sep 17 00:00:00 2001 From: Shima Ryuhei <65934663+islandryu@users.noreply.github.com> Date: Fri, 15 Nov 2024 23:26:44 +0900 Subject: [PATCH 5/8] Update test/parallel/test-non-ascii.js Co-authored-by: Aviv Keller --- test/parallel/test-non-ascii.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-non-ascii.js b/test/parallel/test-non-ascii.js index f7ec127a3321f8..91bb75d7b9d27f 100644 --- a/test/parallel/test-non-ascii.js +++ b/test/parallel/test-non-ascii.js @@ -15,4 +15,4 @@ test('Running from a directory with non-ASCII characters', async () => { const { stdout } = await common.spawnPromisified(process.execPath, [tmpdir.resolve('12月/index.js')]); assert.strictEqual(stdout, '12月\n'); }); -}); \ No newline at end of file +}); From fd7a3f57ec9df34cb3a0c3543518a5a88c32647b Mon Sep 17 00:00:00 2001 From: islandryu Date: Fri, 15 Nov 2024 23:47:17 +0900 Subject: [PATCH 6/8] move test file to fixtures --- src/node_modules.cc | 3 +-- .../index.js" | 1 + test/parallel/test-non-ascii.js | 14 ++++---------- 3 files changed, 6 insertions(+), 12 deletions(-) create mode 100644 "test/fixtures/\345\205\250\350\247\222\346\226\207\345\255\227/index.js" diff --git a/src/node_modules.cc b/src/node_modules.cc index 4678061a9321e3..f539002a51bd08 100644 --- a/src/node_modules.cc +++ b/src/node_modules.cc @@ -342,8 +342,7 @@ void BindingData::GetNearestParentPackageJSON( path += kPathSeparator; } - auto package_json = - TraverseParent(realm, path); + auto package_json = TraverseParent(realm, path); if (package_json != nullptr) { args.GetReturnValue().Set(package_json->Serialize(realm)); diff --git "a/test/fixtures/\345\205\250\350\247\222\346\226\207\345\255\227/index.js" "b/test/fixtures/\345\205\250\350\247\222\346\226\207\345\255\227/index.js" new file mode 100644 index 00000000000000..a30f442385c691 --- /dev/null +++ "b/test/fixtures/\345\205\250\350\247\222\346\226\207\345\255\227/index.js" @@ -0,0 +1 @@ +console.log('check non-ascii'); \ No newline at end of file diff --git a/test/parallel/test-non-ascii.js b/test/parallel/test-non-ascii.js index 91bb75d7b9d27f..0cd17321d74de4 100644 --- a/test/parallel/test-non-ascii.js +++ b/test/parallel/test-non-ascii.js @@ -1,18 +1,12 @@ 'use strict'; -const fs = require('node:fs'); const common = require('../common'); -const tmpdir = require('../common/tmpdir'); +const path = require('path'); const { test } = require('node:test'); const assert = require('node:assert'); test('Running from a directory with non-ASCII characters', async () => { - fs.mkdirSync(tmpdir.resolve('12月'), { recursive: true }); - fs.writeFileSync( - tmpdir.resolve('12月/index.js'), - "console.log('12月');", - ); - const { stdout } = await common.spawnPromisified(process.execPath, [tmpdir.resolve('12月/index.js')]); - assert.strictEqual(stdout, '12月\n'); - }); + const nonAsciiPath = path.resolve(__dirname, '../fixtures/全角文字/index.js'); + const { stdout } = await common.spawnPromisified(process.execPath, [nonAsciiPath]); + assert.strictEqual(stdout, 'check non-ascii\n'); }); From 2ad8502c6fd578361c1d9b71655298facf7e99f5 Mon Sep 17 00:00:00 2001 From: islandryu Date: Fri, 15 Nov 2024 23:50:23 +0900 Subject: [PATCH 7/8] add new line to fixture --- .../\345\205\250\350\247\222\346\226\207\345\255\227/index.js" | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git "a/test/fixtures/\345\205\250\350\247\222\346\226\207\345\255\227/index.js" "b/test/fixtures/\345\205\250\350\247\222\346\226\207\345\255\227/index.js" index a30f442385c691..358b2e2e2e8713 100644 --- "a/test/fixtures/\345\205\250\350\247\222\346\226\207\345\255\227/index.js" +++ "b/test/fixtures/\345\205\250\350\247\222\346\226\207\345\255\227/index.js" @@ -1 +1 @@ -console.log('check non-ascii'); \ No newline at end of file +console.log('check non-ascii'); From c2958186574ab40c89b1f0cd548ad0431251fd9f Mon Sep 17 00:00:00 2001 From: islandryu Date: Mon, 18 Nov 2024 10:33:32 +0900 Subject: [PATCH 8/8] use string_view for convert to path --- src/node_file.cc | 4 ++-- src/node_modules.cc | 4 ++-- src/util.cc | 9 ++++----- src/util.h | 19 +++++++++++++------ 4 files changed, 21 insertions(+), 15 deletions(-) diff --git a/src/node_file.cc b/src/node_file.cc index f77cc88fcb4246..7eb2aefa0babea 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -3137,7 +3137,7 @@ static void CpSyncCheckPaths(const FunctionCallbackInfo& args) { THROW_IF_INSUFFICIENT_PERMISSIONS( env, permission::PermissionScope::kFileSystemRead, src.ToStringView()); - auto src_path = BufferValueToPath(src); + auto src_path = StringViewToPath(src.ToStringView()); BufferValue dest(isolate, args[1]); CHECK_NOT_NULL(*dest); @@ -3145,7 +3145,7 @@ static void CpSyncCheckPaths(const FunctionCallbackInfo& args) { THROW_IF_INSUFFICIENT_PERMISSIONS( env, permission::PermissionScope::kFileSystemWrite, dest.ToStringView()); - auto dest_path = BufferValueToPath(dest); + auto dest_path = StringViewToPath(dest.ToStringView()); bool dereference = args[2]->IsTrue(); bool recursive = args[3]->IsTrue(); diff --git a/src/node_modules.cc b/src/node_modules.cc index f539002a51bd08..4a3e7f59eb7efe 100644 --- a/src/node_modules.cc +++ b/src/node_modules.cc @@ -337,7 +337,7 @@ void BindingData::GetNearestParentPackageJSON( bool slashCheck = path_value.ToStringView().ends_with(kPathSeparator); ToNamespacedPath(realm->env(), &path_value); - std::filesystem::path path = BufferValueToPath(path_value); + std::filesystem::path path = StringViewToPath(path_value.ToStringView()); if (slashCheck) { path += kPathSeparator; } @@ -361,7 +361,7 @@ void BindingData::GetNearestParentPackageJSONType( bool slashCheck = path_value.ToStringView().ends_with(kPathSeparator); ToNamespacedPath(realm->env(), &path_value); - std::filesystem::path path = BufferValueToPath(path_value); + std::filesystem::path path = StringViewToPath(path_value.ToStringView()); if (slashCheck) { path += kPathSeparator; } diff --git a/src/util.cc b/src/util.cc index 531a5330c2cb74..cd16c39d29b52a 100644 --- a/src/util.cc +++ b/src/util.cc @@ -886,14 +886,14 @@ v8::Maybe GetValidFileMode(Environment* env, } #ifdef _WIN32 -std::wstring ConvertToWideString(const std::string& str) { +std::wstring ConvertToWideString(std::string_view strView) { int size_needed = MultiByteToWideChar( - CP_UTF8, 0, &str[0], static_cast(str.size()), nullptr, 0); + CP_UTF8, 0, strView.data(), static_cast(strView.size()), nullptr, 0); std::wstring wstrTo(size_needed, 0); MultiByteToWideChar(CP_UTF8, 0, - &str[0], - static_cast(str.size()), + strView.data(), + static_cast(strView.size()), &wstrTo[0], size_needed); return wstrTo; @@ -921,7 +921,6 @@ std::string ConvertWideToUTF8(const std::wstring& wstr) { nullptr); return strTo; } - #endif // _WIN32 } // namespace node diff --git a/src/util.h b/src/util.h index d32c71cd70aee8..54d1aefe2313e8 100644 --- a/src/util.h +++ b/src/util.h @@ -1019,19 +1019,26 @@ v8::Maybe GetValidFileMode(Environment* env, inline bool IsWindowsBatchFile(const char* filename); #ifdef _WIN32 -std::wstring ConvertToWideString(const std::string& str); +std::wstring ConvertToWideString(const std::string_view str); -#define BufferValueToPath(str) \ - std::filesystem::path(ConvertToWideString(str.ToString())) +inline std::filesystem::path StringViewToPath(std::string_view str) { + return std::filesystem::path(ConvertToWideString(str)); +} std::string ConvertWideToUTF8(const std::wstring& wstr); -#define PathToString(path) ConvertWideToUTF8(path.wstring()); +inline std::string PathToString(std::filesystem::path path) { + return ConvertWideToUTF8(path.wstring()); +} #else // _WIN32 -#define BufferValueToPath(str) std::filesystem::path(str.ToStringView()); -#define PathToString(path) path.native(); +inline std::filesystem::path StringViewToPath(std::string_view str) { + return std::filesystem::path(str); +} +inline std::string PathToString(std::filesystem::path path) { + return path.native(); +} #endif // _WIN32