Skip to content

Commit 3d9eb4e

Browse files
Default value conversion error if the application locale is set (#1160)
**Short description** Application fails with `CLI::ConversionError` in some locales if the application locale is set. **Root cause** CLI11 uses `std::stringstream` to format default value and `strtol`/`strtod` to parse them back. The first method adds locale-specific thousand separators but the second method parses the number only up to the first separator. This causes an exception when the same value is converted to string representation and then parsed back. Examples: in _en_US_ locale 1234.56 is "1,234.56" and in de_DE locale 1234.56 is "1.234,56". **Reproduction** ```c++ #include <string> #include <CLI/CLI.hpp> int main(int argc, const char **argv) { std::locale::global(std::locale("")); CLI::App app{"Bug report app"}; long foo; app.add_option("FOO", foo, "Foo option") ->envname("FOO") ->default_val(1234567); CLI11_PARSE(app, argc, argv); return 0; } ``` Output: ``` $ g++ -Wall -Wextra -I CLI11/include/ cli11-bug-locale-string.cpp -o cli11-bug-locale-string $ for locale in C en_US de_DE fr_FR de_CH; do echo "locale $locale"; LC_NUMERIC=$locale.UTF-8 ./cli11-bug-locale-string && echo OK; echo; done locale C OK locale en_US terminate called after throwing an instance of 'CLI::ConversionError' what(): The value FOO is not an allowed value for given default value("1,234,567") produces an error : Could not convert: FOO = 1,234,567 Aborted (core dumped) locale de_DE terminate called after throwing an instance of 'CLI::ConversionError' what(): The value FOO is not an allowed value for given default value("1.234.567") produces an error : Could not convert: FOO = 1.234.567 Aborted (core dumped) locale fr_FR terminate called after throwing an instance of 'CLI::ConversionError' what(): The value FOO is not an allowed value for given default value("1 234 567") produces an error : Could not convert: FOO = 1 234 567 Aborted (core dumped) locale de_CH OK ``` **Fix description** The fix strips group separators from the input string. This extends a bit the logic introduced in change #968 for `_` and `'` separators. There are no unit tests for the change because it requires to have all mentioned locales to be installed in the system. --------- Co-authored-by: Philip Top <[email protected]>
1 parent 5602f2b commit 3d9eb4e

File tree

5 files changed

+69
-1
lines changed

5 files changed

+69
-1
lines changed

include/CLI/TypeTools.hpp

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -961,6 +961,13 @@ bool integral_conversion(const std::string &input, T &output) noexcept {
961961
output = (output_sll < 0) ? static_cast<T>(0) : static_cast<T>(output_sll);
962962
return (static_cast<std::int64_t>(output) == output_sll);
963963
}
964+
// remove locale-specific group separators
965+
char group_separator = std::use_facet<std::numpunct<char>>(std::locale()).thousands_sep();
966+
if(input.find_first_of(group_separator) != std::string::npos) {
967+
std::string nstring = input;
968+
nstring.erase(std::remove(nstring.begin(), nstring.end(), group_separator), nstring.end());
969+
return integral_conversion(nstring, output);
970+
}
964971
// remove separators
965972
if(input.find_first_of("_'") != std::string::npos) {
966973
std::string nstring = input;
@@ -1019,6 +1026,13 @@ bool integral_conversion(const std::string &input, T &output) noexcept {
10191026
output = static_cast<T>(1);
10201027
return true;
10211028
}
1029+
// remove locale-specific group separators
1030+
char group_separator = std::use_facet<std::numpunct<char>>(std::locale()).thousands_sep();
1031+
if(input.find_first_of(group_separator) != std::string::npos) {
1032+
std::string nstring = input;
1033+
nstring.erase(std::remove(nstring.begin(), nstring.end(), group_separator), nstring.end());
1034+
return integral_conversion(nstring, output);
1035+
}
10221036
// remove separators and trailing spaces
10231037
if(input.find_first_of("_'") != std::string::npos) {
10241038
std::string nstring = input;
@@ -1160,6 +1174,13 @@ bool lexical_cast(const std::string &input, T &output) {
11601174
}
11611175
}
11621176

1177+
// remove locale-specific group separators
1178+
char group_separator = std::use_facet<std::numpunct<char>>(std::locale()).thousands_sep();
1179+
if(input.find_first_of(group_separator) != std::string::npos) {
1180+
std::string nstring = input;
1181+
nstring.erase(std::remove(nstring.begin(), nstring.end(), group_separator), nstring.end());
1182+
return lexical_cast(nstring, output);
1183+
}
11631184
// remove separators
11641185
if(input.find_first_of("_'") != std::string::npos) {
11651186
std::string nstring = input;

tests/AppTest.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2706,7 +2706,7 @@ TEST_CASE_METHOD(TApp, "BadUserSepParse", "[app]") {
27062706
std::vector<int> vals;
27072707
app.add_option("--idx", vals);
27082708

2709-
args = {"--idx", "1,2,3"};
2709+
args = {"--idx", "1;2;3"};
27102710

27112711
CHECK_THROWS_AS(run(), CLI::ConversionError);
27122712
}

tests/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ set(CLI11_TESTS
4949
StringParseTest
5050
ComplexTypeTest
5151
TrueFalseTest
52+
localeTest
5253
OptionGroupTest
5354
EncodingTest)
5455

tests/localeTest.cpp

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
// Copyright (c) 2017-2025, University of Cincinnati, developed by Henry Schreiner
2+
// under NSF AWARD 1414736 and by the respective contributors.
3+
// All rights reserved.
4+
//
5+
// SPDX-License-Identifier: BSD-3-Clause
6+
7+
#include "app_helper.hpp"
8+
9+
#include <complex>
10+
#include <cstdint>
11+
#include <iomanip>
12+
#include <iostream>
13+
#include <locale>
14+
#include <numeric>
15+
#include <string>
16+
#include <utility>
17+
#include <vector>
18+
19+
// Custom facet for thousands separator
20+
class CustomThousandsSeparator : public std::numpunct<char> {
21+
protected:
22+
char do_thousands_sep() const override { return '|'; } // Space separator
23+
std::string do_grouping() const override { return "\2"; } // Group digits in sets of 2
24+
};
25+
26+
// derived from https://github.com/CLIUtils/CLI11/pull/1160
27+
TEST_CASE_METHOD(TApp, "locale", "[separators]") {
28+
std::locale customLocale(std::locale::classic(), new CustomThousandsSeparator);
29+
std::locale::global(customLocale); // Set as the default system-wide locale
30+
31+
// Ensure standard streams use the custom locale automatically
32+
std::cout.imbue(std::locale());
33+
std::int64_t foo;
34+
std::uint64_t bar;
35+
float qux;
36+
37+
app.add_option("FOO", foo, "Foo option")->default_val(1234567)->force_callback();
38+
app.add_option("BAR", bar, "Bar option")->default_val(2345678)->force_callback();
39+
app.add_option("QUX", qux, "QUX option")->default_val(3456.78)->force_callback();
40+
41+
CHECK_NOTHROW(run());
42+
CHECK(foo == 1234567);
43+
CHECK(bar == 2345678);
44+
CHECK_THAT(qux, Catch::Matchers::WithinAbs(3456.78, 0.01));
45+
}

tests/meson.build

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ testnames = [
5757
['StringParseTest', {}],
5858
['ComplexTypeTest', {}],
5959
['TrueFalseTest', {}],
60+
['localeTest', {}],
6061
['OptionGroupTest', {}],
6162
['EncodingTest', {}],
6263
# multi-only

0 commit comments

Comments
 (0)