From a981c9060cbb063d1f4809aa01719f5909625c08 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Sun, 6 Sep 2026 12:11:29 +0200 Subject: [PATCH] fix(html): keep the host's locale out of the css we write MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `html::color` built its `rgba(…)` through an un-imbued `std::stringstream`, so the alpha picked up whatever global locale the host had set. Under a german one a translucent colour came out as `rgba(0,0,0,0,501961)` — four commas, not a colour, and the declaration is dropped. `util::number::to_string_significant` already knew this and imbued the classic locale by hand; three other places did not. All of them format through `fmt` now, which ignores the global locale unless asked. `std::format` would have done as well, but is unusable here: libc++ reaches it through a floating-point `std::to_chars` marked unavailable before macOS 13.3 / iOS 16.3, and the apple profiles deploy to macOS 12 / iOS 15, so one call anywhere in `src/` fails the framework build whatever the argument types are. Output is unchanged everywhere else, and deliberately so — the reference html is byte for byte what it was. `{:.{}f}` is `std::fixed` + `setprecision`, `{:.4g}` is bare `setprecision(4)`, `{:g}` is a default-formatted double and `{:06x}` is `setw(6)` + `setfill('0')` + `hex`; each was checked against a stream over 1.2M values before being used. `pdf_object` stops leaving `setprecision(4)` on a stream it shares while it is at it. The hot html element and style writers keep their `"…" + std::to_string(n)` concatenations — `fmt` is no faster there, and this is not a perf change: a render calls `to_string_significant` a few thousand times, not a few million. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01QoTh7BEUSL2z9BBThgsEi7 --- AGENTS.md | 7 +++++++ CHANGELOG.md | 4 ++++ CMakeLists.txt | 2 ++ conan.lock | 1 + conanfile.py | 1 + src/odr/internal/html/common.cpp | 21 ++++++++----------- src/odr/internal/html/filesystem.cpp | 9 +++----- src/odr/internal/pdf/pdf_object.cpp | 6 ++++-- src/odr/internal/util/number_util.cpp | 19 ++++++----------- src/odr/internal/util/number_util.hpp | 2 +- src/odr/internal/util/string_util.cpp | 8 +++---- test/src/internal/html/common_test.cpp | 29 ++++++++++++++++++++++++++ 12 files changed, 70 insertions(+), 39 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 12c0b49ba..dead739d4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -200,6 +200,13 @@ Dispatch `release.yml` against main, publish the draft that appears — input (`std::runtime_error` or the typed exceptions in `src/odr/exceptions.hpp`) rather than silently degrading. Only pass through (return empty / skip) values that are genuinely *optional* or *not yet modelled*. +- **Format numbers with `fmt`, never a stream**: a stream carries the global + locale the host sets, and a german one writes `1,5` into a css length. + `util::number::to_string_significant` is the css/svg spelling. **Not + `std::format`** — libc++ reaches it through a floating-point `std::to_chars` + unavailable before macOS 13.3 / iOS 16.3, and the apple slices deploy to + macOS 12 / iOS 15, so one call anywhere in `src/` fails the framework build. + Only `-mmacosx-version-min=12.0` shows it. - **Fixed-width integer types — always**: prefer `` types (`std::int32_t`, `std::uint8_t`, …) over `int` / `unsigned` / `long` / `unsigned char`. Reserve the built-in types for genuinely index/size-like values (`std::size_t`) or where diff --git a/CHANGELOG.md b/CHANGELOG.md index 89faee8dd..9f94fc779 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,10 @@ The release run heads these entries with the version and opens a fresh ## Unreleased +- Fixed: a semi-transparent colour wrote `rgba(0,0,0,0,501961)` where the host + had set a global locale with a comma decimal separator. Adds a `fmt` + dependency. + - New `Sheet::page_layout()`: the paper an ods states for a sheet, read from the master page its table style names. Mirrored in the Python, JNI and Apple bindings. Empty for xlsx, xls, numbers and csv. diff --git a/CMakeLists.txt b/CMakeLists.txt index ddf8fc430..e15011c27 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -63,6 +63,7 @@ set_property(TARGET pugixml::pugixml APPEND PROPERTY find_package(md4c REQUIRED) find_package(miniz REQUIRED) find_package(cryptopp REQUIRED) +find_package(fmt REQUIRED) find_package(nlohmann_json REQUIRED) find_package(OpenJPEG REQUIRED) find_package(uchardet REQUIRED) @@ -318,6 +319,7 @@ target_link_libraries(odr md4c::md4c miniz::miniz cryptopp::cryptopp + fmt::fmt nlohmann_json::nlohmann_json openjp2 uchardet::uchardet diff --git a/conan.lock b/conan.lock index a691f8295..b06f154b5 100644 --- a/conan.lock +++ b/conan.lock @@ -13,6 +13,7 @@ "miniz/3.1.1#ae48ea4c4ee773386b496b9722e3b66a%1778764964.632", "md4c/0.5.2#3d7106721e458f9f799b87d4d50d02e0%1746796758.933", "gtest/1.14.0#f8f0757a574a8dd747d16af62d6eb1b7%1743410807.169", + "fmt/11.2.0#15c2efeeecd75ba2fb9dafbec8dfa4cc%1785754557.243", "cryptopp/8.9.0#7a51e0038756b21bc3a6b82d681d5906%1758206597.119", "cpp-httplib/0.47.0#add6673ff352c26898ed2650453e706e%1784539639.401", "bzip2/1.0.8#c470882369c2d95c5c77e970c0c7e321%1762886692.465" diff --git a/conanfile.py b/conanfile.py index 04d005b63..bbd418fcb 100644 --- a/conanfile.py +++ b/conanfile.py @@ -53,6 +53,7 @@ def config_options(self): def requirements(self): self.requires("pugixml/1.15") self.requires("cryptopp/8.9.0") + self.requires("fmt/11.2.0") self.requires("md4c/0.5.2") self.requires("miniz/3.1.1") self.requires("nlohmann_json/3.12.0") diff --git a/src/odr/internal/html/common.cpp b/src/odr/internal/html/common.cpp index 46736a960..a5a3a7e38 100644 --- a/src/odr/internal/html/common.cpp +++ b/src/odr/internal/html/common.cpp @@ -16,9 +16,9 @@ #include #include #include + +#include #include -#include -#include #include #include #include @@ -320,17 +320,14 @@ html::fill_path_variables(const std::string &path, std::string html::color(const Color &color) { if (color.alpha != 255) { - std::stringstream ss; - ss << "rgba(" << static_cast(color.red) << "," - << static_cast(color.green) << "," - << static_cast(color.blue) << "," - << (static_cast(color.alpha) / 255.0) << ")"; - return ss.str(); + // `{:g}` is the six significant digits the stream this replaced wrote. + return fmt::format("rgba({},{},{},{:g})", + static_cast(color.red), + static_cast(color.green), + static_cast(color.blue), + static_cast(color.alpha) / 255.0); } - std::stringstream ss; - ss << "#"; - ss << std::setw(6) << std::setfill('0') << std::hex << color.rgb(); - return ss.str(); + return fmt::format("#{:06x}", color.rgb()); } std::string html::file_to_url(const std::string &file, diff --git a/src/odr/internal/html/filesystem.cpp b/src/odr/internal/html/filesystem.cpp index 834abd1e9..259b01d4b 100644 --- a/src/odr/internal/html/filesystem.cpp +++ b/src/odr/internal/html/filesystem.cpp @@ -15,9 +15,9 @@ #include #include -#include + +#include #include -#include namespace odr::internal::html { namespace { @@ -36,10 +36,7 @@ std::string human_size(const std::size_t size) { ++unit; } - std::ostringstream result; - result << std::fixed << std::setprecision(unit == 0 ? 0 : 1) << value << " " - << units.at(unit); - return result.str(); + return fmt::format("{:.{}f} {}", value, unit == 0 ? 0 : 1, units.at(unit)); } /// From the extension, not the bytes: sniffing every entry would read the whole diff --git a/src/odr/internal/pdf/pdf_object.cpp b/src/odr/internal/pdf/pdf_object.cpp index 356ce0e17..962827d7c 100644 --- a/src/odr/internal/pdf/pdf_object.cpp +++ b/src/odr/internal/pdf/pdf_object.cpp @@ -3,10 +3,11 @@ #include #include -#include #include #include #include + +#include #include namespace odr::internal::pdf { @@ -135,7 +136,8 @@ void Object::to_stream(std::ostream &out) const { } else if (is_integer()) { out << as_integer(); } else if (is_real()) { - out << std::setprecision(4) << as_real(); + // not `setprecision`, which would stick to the stream + out << fmt::format("{:.4g}", as_real()); } else if (is_standard_string()) { as().to_stream(out); } else if (is_hex_string()) { diff --git a/src/odr/internal/util/number_util.cpp b/src/odr/internal/util/number_util.cpp index 10226ad06..f439d4411 100644 --- a/src/odr/internal/util/number_util.cpp +++ b/src/odr/internal/util/number_util.cpp @@ -2,23 +2,19 @@ #include #include -#include -#include -#include + +#include namespace odr::internal::util { std::string number::to_string_significant(const double value, const int significant_digits) { if (!std::isfinite(value)) { - std::ostringstream ss; - ss.imbue(std::locale::classic()); - ss << value; - return ss.str(); + return fmt::format("{}", value); } - // `std::fixed` counts decimals, not significant digits, so shift by the - // integer part; clamped because a denormal or a huge value would blow up + // `{:.Nf}` counts decimals, not significant digits, so shift by the integer + // part; clamped because a denormal or a huge value would blow up int integer_digits = 1; if (value != 0.0) { integer_digits = @@ -26,10 +22,7 @@ std::string number::to_string_significant(const double value, } const int decimals = std::clamp(significant_digits - integer_digits, 0, 15); - std::ostringstream ss; - ss.imbue(std::locale::classic()); - ss << std::fixed << std::setprecision(decimals) << value; - std::string result = ss.str(); + std::string result = fmt::format("{:.{}f}", value, decimals); if (result.find('.') != std::string::npos) { result.erase(result.find_last_not_of('0') + 1); diff --git a/src/odr/internal/util/number_util.hpp b/src/odr/internal/util/number_util.hpp index 89f17c817..81f105632 100644 --- a/src/odr/internal/util/number_util.hpp +++ b/src/odr/internal/util/number_util.hpp @@ -6,7 +6,7 @@ namespace odr::internal::util::number { /// Renders @p value with @p significant_digits significant digits, without /// trailing zeros, never in scientific notation, which CSS and SVG lengths do -/// not accept, and in the classic locale, where a german one would write +/// not accept, and never in the host's locale, where a german one would write /// `1,5`. Asking for more digits than the source has shows its noise: a /// `float` carries about 7, beyond that `68.55` becomes `68.550003`. std::string to_string_significant(double value, int significant_digits); diff --git a/src/odr/internal/util/string_util.cpp b/src/odr/internal/util/string_util.cpp index 85c52faa8..21a3611d1 100644 --- a/src/odr/internal/util/string_util.cpp +++ b/src/odr/internal/util/string_util.cpp @@ -3,11 +3,11 @@ #include #include #include -#include #include -#include #include +#include + #include #include #include @@ -167,9 +167,7 @@ std::vector string::split(const std::string &string, } std::string string::to_string(const double d, const int precision) { - std::stringstream stream; - stream << std::fixed << std::setprecision(precision) << d; - return stream.str(); + return fmt::format("{:.{}f}", d, precision); } std::size_t string::utf8_length(const std::string &string) { diff --git a/test/src/internal/html/common_test.cpp b/test/src/internal/html/common_test.cpp index 9caedee8c..01f235f0f 100644 --- a/test/src/internal/html/common_test.cpp +++ b/test/src/internal/html/common_test.cpp @@ -7,6 +7,7 @@ #include #include +#include #include #include #include @@ -355,3 +356,31 @@ TEST(html_common, whitespace_inside_a_scheme_does_not_hide_it) { ihtml::is_safe_uri(std::string("javascript") + '\0' + ":alert(1)")); EXPECT_FALSE(ihtml::is_safe_uri("\x01javascript:alert(1)")); } + +namespace { + +class LocaleGuard final { +public: + ~LocaleGuard() { std::locale::global(m_previous); } + +private: + std::locale m_previous{std::locale()}; +}; + +} // namespace + +/// The alpha is the only number `color` prints, so it is the only way the +/// host's locale could reach the css. `rgba(0,0,0,0,501961)` is not a colour. +TEST(html_common, a_global_locale_does_not_reach_the_css) { + const Color translucent{0, 0, 0, 128}; + ASSERT_EQ(ihtml::color(translucent), "rgba(0,0,0,0.501961)"); + + const LocaleGuard guard; + try { + std::locale::global(std::locale("de_DE.UTF-8")); + } catch (const std::runtime_error &) { + GTEST_SKIP() << "de_DE.UTF-8 is not installed"; + } + + EXPECT_EQ(ihtml::color(translucent), "rgba(0,0,0,0.501961)"); +}