Commit 98f6c00dad96d3150a9b969a0ee67addc78ac5f0
1 parent
ef127001
Protect numeric conversion against user's locale (fixes #459)
Showing
9 changed files
with
53 additions
and
1 deletions
ChangeLog
| 1 | 1 | 2020-10-21 Jay Berkenbilt <ejb@ql.org> |
| 2 | 2 | |
| 3 | + * Ensure that numeric conversion is not affected by the user's | |
| 4 | + global locale setting. Fixes #459. | |
| 5 | + | |
| 3 | 6 | * Add qpdf-<version>-linux-x86_64.zip to the list of built |
| 4 | 7 | distributions. This is a simple zip file that contains just the |
| 5 | 8 | qpdf executables and the dependent shared libraries that would not | ... | ... |
README-maintainer
| ... | ... | @@ -83,6 +83,14 @@ CODING RULES |
| 83 | 83 | |
| 84 | 84 | * Use QIntC for type conversions -- see casting policy in docs. |
| 85 | 85 | |
| 86 | +* Remember to imbue ostringstreams with std::locale::classic() before | |
| 87 | + outputting numbers. This protects against the user's global locale | |
| 88 | + altering otherwise deterministic values. (See github issue #459.) | |
| 89 | + One could argue that error messages containing numbers should | |
| 90 | + respect the user's locale, but I think it's more important for | |
| 91 | + output to be consistent, since the messages in question are not | |
| 92 | + really targetted at the end user. | |
| 93 | + | |
| 86 | 94 | * Use QPDF_DLL on all methods that are to be exported in the shared |
| 87 | 95 | library/DLL. Use QPDF_DLL_CLASS for all classes whose type |
| 88 | 96 | information is needed. This is important for exception classes and | ... | ... |
TODO
| ... | ... | @@ -7,7 +7,6 @@ Candidates for upcoming release |
| 7 | 7 | * Open "next" issues |
| 8 | 8 | * bugs |
| 9 | 9 | * #473: zsh completion with directories |
| 10 | - * #459: locale-sensitivity | |
| 11 | 10 | * #449: internal error with case to reproduce (from pikepdf) |
| 12 | 11 | * #444: concatenated stream/whitespace bug |
| 13 | 12 | * Non-bugs | ... | ... |
include/qpdf/QIntC.hh
| ... | ... | @@ -29,6 +29,7 @@ |
| 29 | 29 | #include <limits> |
| 30 | 30 | #include <sstream> |
| 31 | 31 | #include <cassert> |
| 32 | +#include <locale> | |
| 32 | 33 | #include <type_traits> |
| 33 | 34 | |
| 34 | 35 | // This namespace provides safe integer conversion that detects |
| ... | ... | @@ -67,6 +68,7 @@ namespace QIntC // QIntC = qpdf Integer Conversion |
| 67 | 68 | if (i > std::numeric_limits<To>::max()) |
| 68 | 69 | { |
| 69 | 70 | std::ostringstream msg; |
| 71 | + msg.imbue(std::locale::classic()); | |
| 70 | 72 | msg << "integer out of range converting " << i |
| 71 | 73 | << " from a " |
| 72 | 74 | << sizeof(From) << "-byte unsigned type to a " |
| ... | ... | @@ -88,6 +90,7 @@ namespace QIntC // QIntC = qpdf Integer Conversion |
| 88 | 90 | (i > std::numeric_limits<To>::max())) |
| 89 | 91 | { |
| 90 | 92 | std::ostringstream msg; |
| 93 | + msg.imbue(std::locale::classic()); | |
| 91 | 94 | msg << "integer out of range converting " << i |
| 92 | 95 | << " from a " |
| 93 | 96 | << sizeof(From) << "-byte signed type to a " |
| ... | ... | @@ -111,6 +114,7 @@ namespace QIntC // QIntC = qpdf Integer Conversion |
| 111 | 114 | if ((i < 0) || (ii > std::numeric_limits<To>::max())) |
| 112 | 115 | { |
| 113 | 116 | std::ostringstream msg; |
| 117 | + msg.imbue(std::locale::classic()); | |
| 114 | 118 | msg << "integer out of range converting " << i |
| 115 | 119 | << " from a " |
| 116 | 120 | << sizeof(From) << "-byte signed type to a " |
| ... | ... | @@ -134,6 +138,7 @@ namespace QIntC // QIntC = qpdf Integer Conversion |
| 134 | 138 | if (i > maxval) |
| 135 | 139 | { |
| 136 | 140 | std::ostringstream msg; |
| 141 | + msg.imbue(std::locale::classic()); | |
| 137 | 142 | msg << "integer out of range converting " << i |
| 138 | 143 | << " from a " |
| 139 | 144 | << sizeof(From) << "-byte unsigned type to a " | ... | ... |
libqpdf/BufferInputSource.cc
| ... | ... | @@ -108,6 +108,7 @@ BufferInputSource::range_check(qpdf_offset_t cur, qpdf_offset_t delta) |
| 108 | 108 | ((std::numeric_limits<qpdf_offset_t>::max() - cur) < delta)) |
| 109 | 109 | { |
| 110 | 110 | std::ostringstream msg; |
| 111 | + msg.imbue(std::locale::classic()); | |
| 111 | 112 | msg << "seeking forward from " << cur |
| 112 | 113 | << " by " << delta |
| 113 | 114 | << " would cause an overflow of the offset type"; | ... | ... |
libqpdf/OffsetInputSource.cc
| ... | ... | @@ -47,6 +47,7 @@ OffsetInputSource::seek(qpdf_offset_t offset, int whence) |
| 47 | 47 | if (offset > this->max_safe_offset) |
| 48 | 48 | { |
| 49 | 49 | std::ostringstream msg; |
| 50 | + msg.imbue(std::locale::classic()); | |
| 50 | 51 | msg << "seeking to " << offset |
| 51 | 52 | << " offset by " << global_offset |
| 52 | 53 | << " would cause an overflow of the offset type"; | ... | ... |
libqpdf/QPDF.cc
| ... | ... | @@ -1220,6 +1220,7 @@ QPDF::processXRefStream(qpdf_offset_t xref_offset, QPDFObjectHandle& xref_obj) |
| 1220 | 1220 | ((std::numeric_limits<int>::max() - obj) < chunk_count)) |
| 1221 | 1221 | { |
| 1222 | 1222 | std::ostringstream msg; |
| 1223 | + msg.imbue(std::locale::classic()); | |
| 1223 | 1224 | msg << "adding " << chunk_count << " to " << obj |
| 1224 | 1225 | << " while computing index in xref stream would cause" |
| 1225 | 1226 | << " an integer overflow"; | ... | ... |
libqpdf/QUtil.cc
| ... | ... | @@ -21,6 +21,7 @@ |
| 21 | 21 | #include <string.h> |
| 22 | 22 | #include <fcntl.h> |
| 23 | 23 | #include <memory> |
| 24 | +#include <locale> | |
| 24 | 25 | #ifndef QPDF_NO_WCHAR_T |
| 25 | 26 | # include <cwchar> |
| 26 | 27 | #endif |
| ... | ... | @@ -267,6 +268,7 @@ int_to_string_base_internal(T num, int base, int length) |
| 267 | 268 | "int_to_string_base called with unsupported base"); |
| 268 | 269 | } |
| 269 | 270 | std::ostringstream buf; |
| 271 | + buf.imbue(std::locale::classic()); | |
| 270 | 272 | buf << std::setbase(base) << std::nouppercase << num; |
| 271 | 273 | std::string result; |
| 272 | 274 | int str_length = QIntC::to_int(buf.str().length()); |
| ... | ... | @@ -318,6 +320,7 @@ QUtil::double_to_string(double num, int decimal_places) |
| 318 | 320 | decimal_places = 6; |
| 319 | 321 | } |
| 320 | 322 | std::ostringstream buf; |
| 323 | + buf.imbue(std::locale::classic()); | |
| 321 | 324 | buf << std::setprecision(decimal_places) << std::fixed << num; |
| 322 | 325 | return buf.str(); |
| 323 | 326 | } | ... | ... |
libtests/qutil.cc
| ... | ... | @@ -10,6 +10,7 @@ |
| 10 | 10 | #include <limits.h> |
| 11 | 11 | #include <assert.h> |
| 12 | 12 | #include <fstream> |
| 13 | +#include <locale> | |
| 13 | 14 | |
| 14 | 15 | #ifdef _WIN32 |
| 15 | 16 | # include <io.h> |
| ... | ... | @@ -80,8 +81,38 @@ void test_to_ull(char const* str, unsigned long long wanted, bool error) |
| 80 | 81 | test_to_number(str, wanted, error, QUtil::string_to_ull); |
| 81 | 82 | } |
| 82 | 83 | |
| 84 | +static void set_locale() | |
| 85 | +{ | |
| 86 | + try | |
| 87 | + { | |
| 88 | + // First try a locale known to put commas in numbers. | |
| 89 | + std::locale::global(std::locale("en_US.UTF-8")); | |
| 90 | + } | |
| 91 | + catch (std::runtime_error&) | |
| 92 | + { | |
| 93 | + try | |
| 94 | + { | |
| 95 | + // If that fails, fall back to the user's default locale. | |
| 96 | + std::locale::global(std::locale("")); | |
| 97 | + } | |
| 98 | + catch (std::runtime_error& e) | |
| 99 | + { | |
| 100 | + // Ignore this error on Windows without MSVC. We get | |
| 101 | + // enough test coverage on other platforms, and mingw | |
| 102 | + // seems to have limited locale support (as of | |
| 103 | + // 2020-10). | |
| 104 | +#if ! defined(_WIN32) || defined(_MSC_VER) | |
| 105 | + throw e; | |
| 106 | +#endif | |
| 107 | + } | |
| 108 | + } | |
| 109 | +} | |
| 110 | + | |
| 83 | 111 | void string_conversion_test() |
| 84 | 112 | { |
| 113 | + // Make sure the code produces consistent results even if we load | |
| 114 | + // a non-C locale. | |
| 115 | + set_locale(); | |
| 85 | 116 | std::cout << QUtil::int_to_string(16059) << std::endl |
| 86 | 117 | << QUtil::int_to_string(16059, 7) << std::endl |
| 87 | 118 | << QUtil::int_to_string(16059, -7) << std::endl | ... | ... |