Protect numeric conversion against user's locale (fixes #459)

9 files changed, 53 insertions, 1 deletions

diff --git a/ChangeLog b/ChangeLog
index 8d772d91..c04ec131 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,8 @@
 2020-10-21  Jay Berkenbilt  <ejb@ql.org>
 
+	* Ensure that numeric conversion is not affected by the user's
+	global locale setting. Fixes #459.
+
 	* Add qpdf-<version>-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 <limits>
 #include <sstream>
 #include <cassert>
+#include <locale>
 #include <type_traits>
 
 // This namespace provides safe integer conversion that detects
@@ -67,6 +68,7 @@ namespace QIntC // QIntC = qpdf Integer Conversion
             if (i > std::numeric_limits<To>::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<To>::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<To>::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<qpdf_offset_t>::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<int>::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 <string.h>
 #include <fcntl.h>
 #include <memory>
+#include <locale>
 #ifndef QPDF_NO_WCHAR_T
 # include <cwchar>
 #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 <limits.h>
 #include <assert.h>
 #include <fstream>
+#include <locale>
 
 #ifdef _WIN32
 # include <io.h>
@@ -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