fix(odf): parse bounded decimal tokens independently of locale

CHANGELOG.md

@@ -16,6 +16,9 @@ The release run heads these entries with the version and opens a fresh
 
 ## Unreleased
 
+- ODF geometry reads decimal numbers independently of the host locale and
+  bounds numeric scans to the current token instead of copying expression tails.
+
 - ODF geometry parsing terminates on invalid text after a closed path and
   rejects excessive polygon/arc expansion, deep formulas and nonfinite results.
 

src/odr/internal/odf/odf_value_cursor.hpp

@@ -1,12 +1,11 @@
 #pragma once
 
 #include <odr/internal/common/text_cursor.hpp>
+#include <odr/internal/util/number_util.hpp>
 #include <odr/internal/util/string_util.hpp>
 
 #include <cstddef>
-#include <cstdlib>
 #include <optional>
-#include <string>
 
 namespace odr::internal::odf {
 
@@ -25,32 +24,46 @@ class ValueCursor : public TextCursor {
     }
   }
 
-  /// `std::strtod` wants a terminator, which the view does not promise, so the
-  /// run it bounds is copied out.
+  /// Reads one decimal token, leaving following operators and commands.
   [[nodiscard]] std::optional<double> read_number() {
     skip_separators();
-    const std::string number(peek_while(is_number_char));
-    char *end = nullptr;
-    const double value = std::strtod(number.c_str(), &end);
-    if (end == number.c_str()) {
+    std::size_t end = peek() == '+' || peek() == '-' ? 1 : 0;
+    const auto digits = [&] {
+      const std::size_t begin = end;
+      while (str::is_ascii_digit(peek(end))) {
+        ++end;
+      }
+      return end != begin;
+    };
+    bool has_digits = digits();
+    if (peek(end) == '.') {
+      ++end;
+      has_digits = digits() || has_digits;
+    }
+    if (!has_digits) {
       return {};
     }
-    // `strtod` may stop short of the run, on a trailing `e` say
-    advance(static_cast<std::size_t>(end - number.c_str()));
+    if (peek(end) == 'e' || peek(end) == 'E') {
+      const std::size_t exponent = end++;
+      if (peek(end) == '+' || peek(end) == '-') {
+        ++end;
+      }
+      if (!digits()) {
+        end = exponent;
+      }
+    }
+    const std::optional<double> value =
+        util::number::parse(rest().substr(0, end));
+    if (value) {
+      advance(end);
+    }
     return value;
   }
 
   [[nodiscard]] bool starts_number() const {
     const char c = peek();
     return c == '-' || c == '+' || c == '.' || str::is_ascii_digit(c);
   }
-
-private:
-  /// A superset of a number's characters, to bound the run `std::strtod` reads.
-  static bool is_number_char(const char c) {
-    return str::is_ascii_digit(c) || c == '+' || c == '-' || c == '.' ||
-           c == 'e' || c == 'E';
-  }
 };
 
 } // namespace odr::internal::odf

test/src/internal/font/cff_font.cpp

@@ -7,6 +7,8 @@
 #include <odr/internal/font/sfnt_transform.hpp>
 #include <odr/internal/util/byte_string.hpp>
 
+#include <locale_util.hpp>
+
 #include <gtest/gtest.h>
 
 #include <array>
@@ -561,10 +563,7 @@ TEST(CffFontTest, DictOperandsStayWithinTheirDeclaredRange) {
 }
 
 TEST(CffFontTest, RealOperandsIgnoreNumericLocale) {
-  struct LocaleGuard final {
-    std::string previous{std::setlocale(LC_NUMERIC, nullptr)};
-    ~LocaleGuard() { std::setlocale(LC_NUMERIC, previous.c_str()); }
-  } guard;
+  const odr::test::LocaleGuard guard;
   if (std::setlocale(LC_NUMERIC, "de_DE.UTF-8") == nullptr &&
       std::setlocale(LC_NUMERIC, "de_DE.utf8") == nullptr) {
     GTEST_SKIP() << "German locale unavailable";

test/src/internal/font/type1_font.cpp

@@ -7,6 +7,7 @@
 #include <odr/internal/util/byte_string.hpp>
 
 #include <internal/font/type1_test_util.hpp>
+#include <locale_util.hpp>
 
 #include <gtest/gtest.h>
 
@@ -88,10 +89,7 @@ TEST(Type1FontTest, IsType1Magic) {
 }
 
 TEST(Type1FontTest, ParsesHeaderAndEncoding) {
-  struct LocaleGuard final {
-    std::string previous{std::setlocale(LC_NUMERIC, nullptr)};
-    ~LocaleGuard() { std::setlocale(LC_NUMERIC, previous.c_str()); }
-  } guard;
+  const odr::test::LocaleGuard guard;
   if (std::setlocale(LC_NUMERIC, "de_DE.UTF-8") == nullptr) {
     std::setlocale(LC_NUMERIC, "de_DE.utf8");
   }

test/src/internal/odf/odf_geometry_test.cpp

@@ -1,11 +1,15 @@
 #include <odr/internal/odf/odf_geometry.hpp>
+#include <odr/internal/odf/odf_value_cursor.hpp>
 
 #include <odr/document_element.hpp>
 
+#include <locale_util.hpp>
+
 #include <pugixml.hpp>
 
 #include <gtest/gtest.h>
 
+#include <clocale>
 #include <cmath>
 #include <cstdint>
 #include <string>
@@ -337,3 +341,30 @@ TEST(OdfShape, generated_geometry_has_bounded_expansion_and_equation_depth) {
       R"(<draw:equation draw:name="f500" draw:formula="0"/></draw:enhanced-geometry></draw:custom-shape>)";
   EXPECT_FALSE(read_path(parse_shape(document, xml)));
 }
+
+TEST(OdfTransform, decimal_tokens_ignore_the_numeric_locale) {
+  const odr::test::LocaleGuard guard;
+  if (std::setlocale(LC_NUMERIC, "de_DE.UTF-8") == nullptr &&
+      std::setlocale(LC_NUMERIC, "de_DE.utf8") == nullptr) {
+    GTEST_SKIP() << "German locale unavailable";
+  }
+  const auto transform = parse_transform("scale(1.5,2.25)");
+  ASSERT_TRUE(transform);
+  expect_linear(*transform, 1.5, 0, 0, 2.25);
+  const auto path = parse_path_data("M.5 .25 L1.25 2.5");
+  ASSERT_TRUE(path);
+  EXPECT_EQ(path->data, "M 0.5 0.25 L 1.25 2.5");
+}
+
+TEST(OdfValueCursor, a_number_stops_at_the_next_token) {
+  ValueCursor cursor("+1.5e-2-3.25+.5E");
+  EXPECT_EQ(cursor.read_number(), 0.015);
+  EXPECT_EQ(cursor.read_number(), -3.25);
+  EXPECT_EQ(cursor.read_number(), 0.5);
+  EXPECT_EQ(cursor.rest(), "E");
+  for (const std::string_view invalid : {".", "+", "-", "1e999"}) {
+    ValueCursor bad(invalid);
+    EXPECT_FALSE(bad.read_number());
+    EXPECT_EQ(bad.rest(), invalid);
+  }
+}

test/src/internal/oldms/xls_test.cpp

@@ -1,3 +1,4 @@
+#include <locale_util.hpp>
 #include <test_util.hpp>
 
 #include <gtest/gtest.h>
@@ -465,10 +466,7 @@ TEST(OldMs, xls_mulrk_takes_its_cells_from_the_body_size) {
 }
 
 TEST(OldMs, xls_number_format_ignores_numeric_locale) {
-  struct LocaleGuard final {
-    std::string previous{std::setlocale(LC_NUMERIC, nullptr)};
-    ~LocaleGuard() { std::setlocale(LC_NUMERIC, previous.c_str()); }
-  } guard;
+  const odr::test::LocaleGuard guard;
   if (std::setlocale(LC_NUMERIC, "de_DE.UTF-8") == nullptr &&
       std::setlocale(LC_NUMERIC, "de_DE.utf8") == nullptr) {
     GTEST_SKIP() << "German locale unavailable";

test/src/locale_util.hpp

@@ -0,0 +1,26 @@
+#pragma once
+
+#include <clocale>
+#include <locale>
+#include <string>
+
+namespace odr::test {
+
+/// Restores C and C++ locales, including after a failed assertion or skip.
+class LocaleGuard final {
+public:
+  LocaleGuard() : m_c(std::setlocale(LC_ALL, nullptr)) {}
+  ~LocaleGuard() {
+    std::locale::global(m_cpp);
+    std::setlocale(LC_ALL, m_c.c_str());
+  }
+
+  LocaleGuard(const LocaleGuard &) = delete;
+  LocaleGuard &operator=(const LocaleGuard &) = delete;
+
+private:
+  const std::locale m_cpp;
+  const std::string m_c;
+};
+
+} // namespace odr::test

test/src/quantity_test.cpp

@@ -1,5 +1,7 @@
 #include <odr/quantity.hpp>
 
+#include <locale_util.hpp>
+
 #include <gtest/gtest.h>
 
 #include <clocale>
@@ -45,23 +47,10 @@ class GroupedNumbers final : public std::numpunct<char> {
   std::string do_grouping() const override { return "\3"; }
 };
 
-class LocaleGuard final {
-public:
-  LocaleGuard() : m_cpp(), m_c(std::setlocale(LC_NUMERIC, nullptr)) {}
-  ~LocaleGuard() {
-    std::locale::global(m_cpp);
-    std::setlocale(LC_NUMERIC, m_c.c_str());
-  }
-
-private:
-  std::locale m_cpp;
-  std::string m_c;
-};
-
 } // namespace
 
 TEST(Quantity, ignores_numeric_locale) {
-  const LocaleGuard guard;
+  const odr::test::LocaleGuard guard;
   std::locale::global(std::locale(std::locale::classic(), new GroupedNumbers));
   if (std::setlocale(LC_NUMERIC, "de_DE.UTF-8") == nullptr) {
     std::setlocale(LC_NUMERIC, "de_DE.utf8");