Skip to content

Commit

Permalink
Fix ToString and ToWString on large strings (#5930)
Browse files Browse the repository at this point in the history
## Summary of changes

Fix ToString and ToWString on large strings

## Reason for change

Those two methods have a fast-path, where we try to fit the result in a
pre-allocated buffer. But when the string is too long, we incorrectly
assumed that `WideCharToMultiByte`/`MultiByteToWideChar` would return
the needed size. In truth, [it only does so when `cchWideChar` is
0](https://learn.microsoft.com/en-us/windows/win32/api/stringapiset/nf-stringapiset-multibytetowidechar#return-value).
Because of that mistake, we were returning an empty string.

## Implementation details

Added a few minor changes while I was at it:
 - For arrays, directly use the array instead of `&array[0]`
 - For strings, use `str.data()` instead of `&str[0]`
 - Don't zero the buffer since we're going to only use what is filled

## Test coverage

Added a unit test for long strings.
  • Loading branch information
kevingosse authored Aug 23, 2024
1 parent 4531f1b commit e0d9add
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 13 deletions.
33 changes: 20 additions & 13 deletions shared/src/native-src/string.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,16 +32,19 @@ namespace shared {
#ifdef _WIN32
if (nbChars == 0) return std::string();

char tmpStr[tmp_buffer_size] = {0};
int size_needed =
WideCharToMultiByte(CP_UTF8, 0, &wstr[0], (int) nbChars, &tmpStr[0], tmp_buffer_size, nullptr, nullptr);
if (size_needed < tmp_buffer_size)
char tmpStr[tmp_buffer_size];
int charsWritten =
WideCharToMultiByte(CP_UTF8, 0, wstr, (int)nbChars, tmpStr, tmp_buffer_size, nullptr, nullptr);
if (charsWritten < tmp_buffer_size && charsWritten != 0)
{
return std::string(tmpStr, size_needed);
return std::string(tmpStr, charsWritten);
}

std::string strTo(size_needed, 0);
WideCharToMultiByte(CP_UTF8, 0, &wstr[0], (int) nbChars, &strTo[0], size_needed, nullptr, nullptr);
// Get the actual size needed
auto sizeNeeded = WideCharToMultiByte(CP_UTF8, 0, wstr, (int)nbChars, nullptr, 0, nullptr, nullptr);

std::string strTo(sizeNeeded, 0);
WideCharToMultiByte(CP_UTF8, 0, wstr, (int)nbChars, strTo.data(), sizeNeeded, nullptr, nullptr);
return strTo;
#else
std::u16string ustr(reinterpret_cast<const char16_t*>(wstr), nbChars);
Expand All @@ -68,14 +71,18 @@ namespace shared {
#ifdef _WIN32
if (str.empty()) return std::wstring();

wchar_t tmpStr[tmp_buffer_size] = {0};
int size_needed = MultiByteToWideChar(CP_UTF8, 0, &str[0], (int)str.size(), &tmpStr[0], tmp_buffer_size);
if (size_needed < tmp_buffer_size) {
return std::wstring(tmpStr, size_needed);
wchar_t tmpStr[tmp_buffer_size];
int charsWritten = MultiByteToWideChar(CP_UTF8, 0, str.data(), (int)str.size(), tmpStr, tmp_buffer_size);
if (charsWritten < tmp_buffer_size && charsWritten != 0)
{
return std::wstring(tmpStr, charsWritten);
}

std::wstring wstrTo(size_needed, 0);
MultiByteToWideChar(CP_UTF8, 0, &str[0], (int)str.size(), &wstrTo[0], size_needed);
// Get the actual size needed
auto sizeNeeded = MultiByteToWideChar(CP_UTF8, 0, str.data(), (int)str.size(), nullptr, 0);

std::wstring wstrTo(sizeNeeded, 0);
MultiByteToWideChar(CP_UTF8, 0, str.data(), (int)str.size(), wstrTo.data(), sizeNeeded);
return wstrTo;
#else
auto ustr = miniutf::to_utf16(str);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ TEST(string, ToString)
EXPECT_TRUE("Wide String" == ToString(WStr("Wide String")));
EXPECT_TRUE("\tWide String\0" == ToString(WStr("\tWide String\0")));

EXPECT_TRUE(std::string(1000, 'a') == ToString(WSTRING(1000, L'a')));

EXPECT_TRUE("42" == ToString(42));

#ifndef LINUX
Expand All @@ -32,6 +34,7 @@ TEST(string, ToWSTRING)
{
EXPECT_TRUE(WStr("Normal String") == ToWSTRING(std::string("Normal String")));
EXPECT_TRUE(WStr("\tNormal String\0") == ToWSTRING(std::string("\tNormal String\0")));
EXPECT_TRUE(WSTRING(1000, 'a') == ToWSTRING(std::string(1000, 'a')));

EXPECT_EQ(WStr("42"), ToWSTRING(42));
}
Expand Down

0 comments on commit e0d9add

Please sign in to comment.