Jens Geyer created THRIFT-6194:
----------------------------------
Summary: ToStringTest leaves the global locale set for the rest of
the UnitTests binary
Key: THRIFT-6194
URL: https://issues.apache.org/jira/browse/THRIFT-6194
Project: Thrift
Issue Type: Bug
Components: C++ - Library
Reporter: Jens Geyer
h2. What happens
{{lib/cpp/test/ToStringTest.cpp}} has two cases that call
{{std::locale::global()}} and never
restore it:
{code:cpp}
BOOST_AUTO_TEST_CASE(locale_en_US_int_to_string) {
std::locale::global(std::locale("en_US.UTF-8"));
BOOST_CHECK_EQUAL(to_string(1000000), "1000000");
}
BOOST_AUTO_TEST_CASE(locale_de_DE_floating_point_to_string) {
std::locale::global(std::locale("de_DE.UTF-8"));
...
}
{code}
Setting the global locale is the point of these cases -- they prove
{{to_string()}} does _not_
follow it, which is why {{TToString.h}} imbues a default locale on the streams
it builds.
Leaving it set afterwards is not the point. Every stream constructed later in
the same
{{UnitTests}} binary picks it up, in suites that have nothing to do with this
one, and numbers
formatted through {{std::ostringstream}} then come out with digit grouping.
h2. Why it is worth fixing
It has already cost one debugging session. A test in {{THttpBufferBoundTest}}
formats a chunk
size through an {{ostringstream}}:
{code:cpp}
std::ostringstream size;
size << std::hex << 4096; // expected "1000"
{code}
which produced {{1.000}} instead. That is not a hex chunk size, so the HTTP
transport under test
read a completely different stream and the case passed or failed depending on
whether it was run
alone or as part of a full run -- {{ToStringTest}} is disabled by default and
only runs when the
whole binary does.
An order-dependent test result whose cause lives in an unrelated file is
expensive to chase, and
the next test in this binary that formats a number will hit it again.
h2. Fix
A small RAII guard in the test file that saves the previous global locale and
restores it on
scope exit, applied to both cases. The test intent is unchanged -- the locale
is still set for
the duration of each case.
h2. Verified
The fix was checked by removing the defensive {{imbue(std::locale::classic())}}
that
{{THttpBufferBoundTest}} had to add for its own protection, and confirming its
chunked case then
passes a full run anyway. Before the fix it failed there and passed in
isolation.
Test-only; no shipped code is affected, which is why this is filed as Minor.
_Filed with AI assistance (Claude Opus 5); reviewed and filed by Jens Geyer._
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
