diff --git a/ChangeLog b/ChangeLog index 8d772d91..c04ec131 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,8 @@ 2020-10-21 Jay Berkenbilt + * Ensure that numeric conversion is not affected by the user's + global locale setting. Fixes #459. + * Add qpdf--linux-x86_64.zip to the list of built distributions. This is a simple zip file that contains just the qpdf executables and the dependent shared libraries that would not diff --git a/README-maintainer b/README-maintainer index 32577195..049d4c33 100644 --- a/README-maintainer +++ b/README-maintainer @@ -83,6 +83,14 @@ CODING RULES * Use QIntC for type conversions -- see casting policy in docs. +* Remember to imbue ostringstreams with std::locale::classic() before + outputting numbers. This protects against the user's global locale + altering otherwise deterministic values. (See github issue #459.) + One could argue that error messages containing numbers should + respect the user's locale, but I think it's more important for + output to be consistent, since the messages in question are not + really targetted at the end user. + * Use QPDF_DLL on all methods that are to be exported in the shared library/DLL. Use QPDF_DLL_CLASS for all classes whose type information is needed. This is important for exception classes and diff --git a/TODO b/TODO index c26eac3f..7e4b8528 100644 --- a/TODO +++ b/TODO @@ -7,7 +7,6 @@ Candidates for upcoming release * Open "next" issues * bugs * #473: zsh completion with directories - * #459: locale-sensitivity * #449: internal error with case to reproduce (from pikepdf) * #444: concatenated stream/whitespace bug * Non-bugs diff --git a/include/qpdf/QIntC.hh b/include/qpdf/QIntC.hh index 906eff77..6f1f4b63 100644 --- a/include/qpdf/QIntC.hh +++ b/include/qpdf/QIntC.hh @@ -29,6 +29,7 @@ #include #include #include +#include #include // This namespace provides safe integer conversion that detects @@ -67,6 +68,7 @@ namespace QIntC // QIntC = qpdf Integer Conversion if (i > std::numeric_limits::max()) { std::ostringstream msg; + msg.imbue(std::locale::classic()); msg << "integer out of range converting " << i << " from a " << sizeof(From) << "-byte unsigned type to a " @@ -88,6 +90,7 @@ namespace QIntC // QIntC = qpdf Integer Conversion (i > std::numeric_limits::max())) { std::ostringstream msg; + msg.imbue(std::locale::classic()); msg << "integer out of range converting " << i << " from a " << sizeof(From) << "-byte signed type to a " @@ -111,6 +114,7 @@ namespace QIntC // QIntC = qpdf Integer Conversion if ((i < 0) || (ii > std::numeric_limits::max())) { std::ostringstream msg; + msg.imbue(std::locale::classic()); msg << "integer out of range converting " << i << " from a " << sizeof(From) << "-byte signed type to a " @@ -134,6 +138,7 @@ namespace QIntC // QIntC = qpdf Integer Conversion if (i > maxval) { std::ostringstream msg; + msg.imbue(std::locale::classic()); msg << "integer out of range converting " << i << " from a " << sizeof(From) << "-byte unsigned type to a " diff --git a/libqpdf/BufferInputSource.cc b/libqpdf/BufferInputSource.cc index 9e141510..fb4010ef 100644 --- a/libqpdf/BufferInputSource.cc +++ b/libqpdf/BufferInputSource.cc @@ -108,6 +108,7 @@ BufferInputSource::range_check(qpdf_offset_t cur, qpdf_offset_t delta) ((std::numeric_limits::max() - cur) < delta)) { std::ostringstream msg; + msg.imbue(std::locale::classic()); msg << "seeking forward from " << cur << " by " << delta << " would cause an overflow of the offset type"; diff --git a/libqpdf/OffsetInputSource.cc b/libqpdf/OffsetInputSource.cc index b6dae255..88eca4e4 100644 --- a/libqpdf/OffsetInputSource.cc +++ b/libqpdf/OffsetInputSource.cc @@ -47,6 +47,7 @@ OffsetInputSource::seek(qpdf_offset_t offset, int whence) if (offset > this->max_safe_offset) { std::ostringstream msg; + msg.imbue(std::locale::classic()); msg << "seeking to " << offset << " offset by " << global_offset << " would cause an overflow of the offset type"; diff --git a/libqpdf/QPDF.cc b/libqpdf/QPDF.cc index 2ebf88b0..1cbef133 100644 --- a/libqpdf/QPDF.cc +++ b/libqpdf/QPDF.cc @@ -1220,6 +1220,7 @@ QPDF::processXRefStream(qpdf_offset_t xref_offset, QPDFObjectHandle& xref_obj) ((std::numeric_limits::max() - obj) < chunk_count)) { std::ostringstream msg; + msg.imbue(std::locale::classic()); msg << "adding " << chunk_count << " to " << obj << " while computing index in xref stream would cause" << " an integer overflow"; diff --git a/libqpdf/QUtil.cc b/libqpdf/QUtil.cc index 072a939c..366365f1 100644 --- a/libqpdf/QUtil.cc +++ b/libqpdf/QUtil.cc @@ -21,6 +21,7 @@ #include #include #include +#include #ifndef QPDF_NO_WCHAR_T # include #endif @@ -267,6 +268,7 @@ int_to_string_base_internal(T num, int base, int length) "int_to_string_base called with unsupported base"); } std::ostringstream buf; + buf.imbue(std::locale::classic()); buf << std::setbase(base) << std::nouppercase << num; std::string result; int str_length = QIntC::to_int(buf.str().length()); @@ -318,6 +320,7 @@ QUtil::double_to_string(double num, int decimal_places) decimal_places = 6; } std::ostringstream buf; + buf.imbue(std::locale::classic()); buf << std::setprecision(decimal_places) << std::fixed << num; return buf.str(); } diff --git a/libtests/qutil.cc b/libtests/qutil.cc index 935cdfc2..abfefce0 100644 --- a/libtests/qutil.cc +++ b/libtests/qutil.cc @@ -10,6 +10,7 @@ #include #include #include +#include #ifdef _WIN32 # include @@ -80,8 +81,38 @@ void test_to_ull(char const* str, unsigned long long wanted, bool error) test_to_number(str, wanted, error, QUtil::string_to_ull); } +static void set_locale() +{ + try + { + // First try a locale known to put commas in numbers. + std::locale::global(std::locale("en_US.UTF-8")); + } + catch (std::runtime_error&) + { + try + { + // If that fails, fall back to the user's default locale. + std::locale::global(std::locale("")); + } + catch (std::runtime_error& e) + { + // Ignore this error on Windows without MSVC. We get + // enough test coverage on other platforms, and mingw + // seems to have limited locale support (as of + // 2020-10). +#if ! defined(_WIN32) || defined(_MSC_VER) + throw e; +#endif + } + } +} + void string_conversion_test() { + // Make sure the code produces consistent results even if we load + // a non-C locale. + set_locale(); std::cout << QUtil::int_to_string(16059) << std::endl << QUtil::int_to_string(16059, 7) << std::endl << QUtil::int_to_string(16059, -7) << std::endl