Skip to content

Commit

Permalink
Fix #7047 Safely handle setlocale (#8735)
Browse files Browse the repository at this point in the history
* fix #7074 Safely handle setlocale

`setlocale` returns a pointer to a buffer containing the current locale name.
This needs to be copied into a `std::string` or it will be overwritten by the next call.

Trying to call `setlocale` with a non-null, invalid pointer can have unpredictable results, such as
```
[ RUN      ] StringPrintfTest.Multibyte
minkernel\crts\ucrt\src\appcrt\convert\mbstowcs.cpp(246) : Assertion failed: (pwcs == nullptr && sizeInWords == 0) || (pwcs != nullptr && sizeInWords > 0)
```

`setlocale` can also return a `nullptr` if it fails, but we assert against that.

* stringprintf_unittest.cc: Replace `new char[n+1]` with `std::array`

Prefer safer alternative to naked pointers.

This is a follow-up to 1dd313c
  • Loading branch information
florin-crisan committed Jul 8, 2021
1 parent d652d80 commit 0444e07
Showing 1 changed file with 15 additions and 11 deletions.
26 changes: 15 additions & 11 deletions src/google/protobuf/stubs/stringprintf_unittest.cc
Expand Up @@ -32,6 +32,7 @@

#include <google/protobuf/stubs/stringprintf.h>

#include <array>
#include <cerrno>
#include <string>

Expand Down Expand Up @@ -91,7 +92,9 @@ TEST(StringPrintfTest, Multibyte) {
// out of memory while trying to determine destination buffer size.
// see b/4194543.

char* old_locale = setlocale(LC_CTYPE, nullptr);
char* old_locale_c = setlocale(LC_CTYPE, nullptr);
ASSERT_TRUE(old_locale_c != nullptr);
std::string old_locale = old_locale_c;
// Push locale with multibyte mode
setlocale(LC_CTYPE, "en_US.utf8");

Expand All @@ -106,24 +109,25 @@ TEST(StringPrintfTest, Multibyte) {

// Repeat with longer string, to make sure that the dynamically
// allocated path in StringAppendV is handled correctly.
int n = 2048;
char* buf = new char[n+1];
memset(buf, ' ', n-3);
memcpy(buf + n - 3, kInvalidCodePoint, 4);
value = StringPrintf("%.*s", n, buf);
const size_t n = 2048;
std::array<char, n+1> buf;
memset(&buf[0], ' ', n-3);
memcpy(&buf[0] + n - 3, kInvalidCodePoint, 4);
value = StringPrintf("%.*s", n, &buf[0]);
// See GRTEv2 vs. GRTEv3 comment above.
EXPECT_TRUE(value.empty() || value == buf);
delete[] buf;
EXPECT_TRUE(value.empty() || value == &buf[0]);

setlocale(LC_CTYPE, old_locale);
setlocale(LC_CTYPE, old_locale.c_str());
}

TEST(StringPrintfTest, NoMultibyte) {
// No multibyte handling, but the string contains funny chars.
char* old_locale = setlocale(LC_CTYPE, nullptr);
char* old_locale_c = setlocale(LC_CTYPE, nullptr);
ASSERT_TRUE(old_locale_c != nullptr);
std::string old_locale = old_locale_c;
setlocale(LC_CTYPE, "POSIX");
std::string value = StringPrintf("%.*s", 3, "\375\067s");
setlocale(LC_CTYPE, old_locale);
setlocale(LC_CTYPE, old_locale.c_str());
EXPECT_EQ("\375\067s", value);
}

Expand Down

0 comments on commit 0444e07

Please sign in to comment.