diff --git a/CHANGELOG.md b/CHANGELOG.md index 92faff86c..8c6050008 100644 --- a/CHANGELOG.md +++ b/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. diff --git a/src/odr/internal/odf/odf_value_cursor.hpp b/src/odr/internal/odf/odf_value_cursor.hpp index 0ad404d88..8ba77c7d0 100644 --- a/src/odr/internal/odf/odf_value_cursor.hpp +++ b/src/odr/internal/odf/odf_value_cursor.hpp @@ -1,12 +1,11 @@ #pragma once #include +#include #include #include -#include #include -#include namespace odr::internal::odf { @@ -25,18 +24,39 @@ 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 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(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 value = + util::number::parse(rest().substr(0, end)); + if (value) { + advance(end); + } return value; } @@ -44,13 +64,6 @@ class ValueCursor : public TextCursor { 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 diff --git a/test/src/internal/font/cff_font.cpp b/test/src/internal/font/cff_font.cpp index 3b9fa7f4b..98cfa2270 100644 --- a/test/src/internal/font/cff_font.cpp +++ b/test/src/internal/font/cff_font.cpp @@ -7,6 +7,8 @@ #include #include +#include + #include #include @@ -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"; diff --git a/test/src/internal/font/type1_font.cpp b/test/src/internal/font/type1_font.cpp index 88b9bad06..0559718ee 100644 --- a/test/src/internal/font/type1_font.cpp +++ b/test/src/internal/font/type1_font.cpp @@ -7,6 +7,7 @@ #include #include +#include #include @@ -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"); } diff --git a/test/src/internal/odf/odf_geometry_test.cpp b/test/src/internal/odf/odf_geometry_test.cpp index e9de4f311..3d6e5cf33 100644 --- a/test/src/internal/odf/odf_geometry_test.cpp +++ b/test/src/internal/odf/odf_geometry_test.cpp @@ -1,11 +1,15 @@ #include +#include #include +#include + #include #include +#include #include #include #include @@ -337,3 +341,30 @@ TEST(OdfShape, generated_geometry_has_bounded_expansion_and_equation_depth) { R"()"; 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); + } +} diff --git a/test/src/internal/oldms/xls_test.cpp b/test/src/internal/oldms/xls_test.cpp index 4082f4bb5..e01803a1b 100644 --- a/test/src/internal/oldms/xls_test.cpp +++ b/test/src/internal/oldms/xls_test.cpp @@ -1,3 +1,4 @@ +#include #include #include @@ -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"; diff --git a/test/src/locale_util.hpp b/test/src/locale_util.hpp new file mode 100644 index 000000000..f48cec63e --- /dev/null +++ b/test/src/locale_util.hpp @@ -0,0 +1,26 @@ +#pragma once + +#include +#include +#include + +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 diff --git a/test/src/quantity_test.cpp b/test/src/quantity_test.cpp index d856f70be..9847a7045 100644 --- a/test/src/quantity_test.cpp +++ b/test/src/quantity_test.cpp @@ -1,5 +1,7 @@ #include +#include + #include #include @@ -45,23 +47,10 @@ class GroupedNumbers final : public std::numpunct { 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");