From da623c65e07d3e0e1e56f3b96c8f4cf8a158b5d2 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Mon, 5 Oct 2026 19:23:35 +0200 Subject: [PATCH 1/2] refactor: centralize UTF operations behind string utilities --- AGENTS.md | 2 ++ src/odr/internal/encoding/transcode.cpp | 27 +++++++++---------- src/odr/internal/html/pdf_file.cpp | 27 +++++++------------ src/odr/internal/iwork/iwork_table.cpp | 4 +-- src/odr/internal/iwork/iwork_text.cpp | 4 +-- .../internal/ooxml/text/ooxml_text_list.cpp | 6 ++--- src/odr/internal/svm/svm_to_svg.cpp | 10 +++---- src/odr/internal/util/string_util.cpp | 19 +++++++++++++ src/odr/internal/util/string_util.hpp | 7 +++++ test/src/internal/util/string_util_test.cpp | 24 +++++++++++++++++ 10 files changed, 84 insertions(+), 46 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 45eb3d96e..b83c47f3c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -161,6 +161,8 @@ cmake --build cmake-build-relwithdebinfo --target translate # CLI: file to HTML - Format numbers with `fmt`, never a stream. A stream carries the host locale, and a German one writes `1,5` into a css length. `util::number::to_string_significant` is the css and svg spelling. +- UTF operations go through `internal/util/string_util.hpp`; only its + implementation includes utfcpp. - Use fixed-width integer types from ``. Use `int` and friends only for index-like values (`std::size_t`) or where an API forces them. - Use `std::array`, never a C array. diff --git a/src/odr/internal/encoding/transcode.cpp b/src/odr/internal/encoding/transcode.cpp index a514cf6ca..87ce01ccb 100644 --- a/src/odr/internal/encoding/transcode.cpp +++ b/src/odr/internal/encoding/transcode.cpp @@ -2,14 +2,11 @@ #include #include +#include #include -#include #include -#include -#include - namespace odr::internal { namespace { @@ -127,7 +124,7 @@ std::string decode_single_byte(const std::string_view bytes, std::string result; result.reserve(bytes.size()); for (std::size_t i = 0; i < bytes.size(); ++i) { - utf8::append(table[byte_at(bytes, i)], std::back_inserter(result)); + util::string::append_c32(table[byte_at(bytes, i)], result); } return result; } @@ -147,27 +144,27 @@ std::string decode_utf16(const std::string_view bytes, const bool little) { const char32_t first = unit(i); if (first < 0xd800 || first > 0xdfff) { - utf8::append(first, std::back_inserter(result)); + util::string::append_c32(first, result); continue; } // a low surrogate first, or a high one with nothing to pair with if (first > 0xdbff || i + 3 >= bytes.size()) { - utf8::append(replacement, std::back_inserter(result)); + util::string::append_c32(replacement, result); continue; } const char32_t second = unit(i + 2); if (second < 0xdc00 || second > 0xdfff) { - utf8::append(replacement, std::back_inserter(result)); + util::string::append_c32(replacement, result); continue; } - utf8::append(0x10000 + ((first - 0xd800) << 10) + (second - 0xdc00), - std::back_inserter(result)); + util::string::append_c32( + 0x10000 + ((first - 0xd800) << 10) + (second - 0xdc00), result); i += 2; } // a trailing odd byte cannot form a unit if (i < bytes.size()) { - utf8::append(replacement, std::back_inserter(result)); + util::string::append_c32(replacement, result); } return result; } @@ -182,12 +179,12 @@ std::string decode_utf32(const std::string_view bytes, const bool little) { for (std::size_t b = 0; b < 4; ++b) { code_point = code_point << 8 | byte_at(bytes, i + (little ? 3 - b : b)); } - utf8::append(is_code_point_valid(code_point) ? code_point : replacement, - std::back_inserter(result)); + util::string::append_c32( + is_code_point_valid(code_point) ? code_point : replacement, result); } if (i < bytes.size()) { - utf8::append(replacement, std::back_inserter(result)); + util::string::append_c32(replacement, result); } return result; } @@ -207,7 +204,7 @@ std::string encoding::to_utf8(const std::string_view bytes, switch (encoding) { case TextEncoding::utf8: - return utf8::replace_invalid(body); + return util::string::replace_invalid_utf8(body); case TextEncoding::utf16le: return decode_utf16(body, true); case TextEncoding::utf16be: diff --git a/src/odr/internal/html/pdf_file.cpp b/src/odr/internal/html/pdf_file.cpp index 5a706a7e7..82f82d0de 100644 --- a/src/odr/internal/html/pdf_file.cpp +++ b/src/odr/internal/html/pdf_file.cpp @@ -30,8 +30,6 @@ #include #include -#include - #include #include #include @@ -2082,7 +2080,7 @@ class HtmlServiceImpl final : public HtmlService { }; // A leading inferred space carries no character code or advance, so the 1:1 - // codes-to-text alignment starts after it — as `core_text_begin` views it. + // codes-to-text alignment starts after it — as `core_text_view` views it. const auto core_char_count = [](const pdf::TextElement &t) { return util::string::utf8_length(t.text) - (t.leading_space_inferred ? 1u : 0u); @@ -2155,9 +2153,9 @@ class HtmlServiceImpl final : public HtmlService { if (core_char_count(*text) != text->advances.size()) { continue; } - auto cp = core_text_begin(*text); + auto remaining = core_text_view(*text); for (const std::uint32_t code : text->font->codes(text->codes)) { - const char32_t uchar = utf8::unchecked::next(cp); + const char32_t uchar = util::string::next_utf8(remaining); if (!collapsible_unicode(uchar)) { continue; } @@ -2269,9 +2267,9 @@ class HtmlServiceImpl final : public HtmlService { if (collapse) { const std::map &won = used_unicode[font - 1]; - auto cp = core_text_begin(text); + auto remaining = core_text_view(text); for (const std::uint32_t code : text.font->codes(text.codes)) { - const char32_t uchar = utf8::unchecked::next(cp); + const char32_t uchar = util::string::next_utf8(remaining); const std::uint16_t glyph = text.font->glyph_for_code(code); const auto it = won.find(uchar); if (!collapsible_unicode(uchar) || it == won.end() || @@ -2288,8 +2286,7 @@ class HtmlServiceImpl final : public HtmlService { // gap as its width (pdf2htmlEX's model: one real space that is both // the copyable character and the advance), else it is zero-width. run.lead_space = text.leading_space_inferred; - run.text = escape_markup( - std::string(core_text_begin(text), text.text.end())); + run.text = escape_markup(core_text(text)); } else { run.glyph_data = glyph_run_str(*text.font, text.codes); run.text = escape_markup(text.text); // overlay (empty=no_unicode) @@ -3069,17 +3066,13 @@ class HtmlServiceImpl final : public HtmlService { } /// `text.text` past an inferred leading space, which backs no advance. - static std::string::const_iterator - core_text_begin(const pdf::TextElement &text) { - auto cp = text.text.begin(); - if (text.leading_space_inferred) { - utf8::unchecked::next(cp); // skip the one-byte U+0020 - } - return cp; + static std::string_view core_text_view(const pdf::TextElement &text) { + return std::string_view(text.text).substr(text.leading_space_inferred ? 1 + : 0); } static std::string core_text(const pdf::TextElement &text) { - return std::string(core_text_begin(text), text.text.end()); + return std::string(core_text_view(text)); } /// Escapes only the three markup-significant characters. Deliberately *not* diff --git a/src/odr/internal/iwork/iwork_table.cpp b/src/odr/internal/iwork/iwork_table.cpp index 22489f72e..190693692 100644 --- a/src/odr/internal/iwork/iwork_table.cpp +++ b/src/odr/internal/iwork/iwork_table.cpp @@ -5,6 +5,7 @@ #include #include #include +#include #include #include @@ -19,7 +20,6 @@ #include #include -#include namespace odr::internal::iwork { @@ -190,7 +190,7 @@ read_cell(const std::string_view record, const std::uint32_t row, } result.text = std::string(it->second.bytes_field(data_list_entry::string) .value_or(std::string_view())); - if (!utf8::is_valid(result.text.begin(), result.text.end())) { + if (!util::string::is_valid_utf8(result.text)) { throw std::runtime_error("iwork: cell text is not UTF-8"); } result.value_type = ValueType::string; diff --git a/src/odr/internal/iwork/iwork_text.cpp b/src/odr/internal/iwork/iwork_text.cpp index 279d1235d..33f8be59f 100644 --- a/src/odr/internal/iwork/iwork_text.cpp +++ b/src/odr/internal/iwork/iwork_text.cpp @@ -18,8 +18,6 @@ #include #include -#include - namespace odr::internal::iwork { namespace { @@ -229,7 +227,7 @@ void iwork::parse_storage(const Context &context, text += part.bytes; } - if (!utf8::is_valid(text.begin(), text.end())) { + if (!util::string::is_valid_utf8(text)) { throw std::runtime_error("iwork: text storage is not UTF-8"); } diff --git a/src/odr/internal/ooxml/text/ooxml_text_list.cpp b/src/odr/internal/ooxml/text/ooxml_text_list.cpp index 858624430..4b27bd678 100644 --- a/src/odr/internal/ooxml/text/ooxml_text_list.cpp +++ b/src/odr/internal/ooxml/text/ooxml_text_list.cpp @@ -10,8 +10,6 @@ #include #include -#include - namespace odr::internal::ooxml::text { namespace { @@ -74,8 +72,8 @@ std::string resolve_bullet(const std::string &text, const std::uint32_t level) { return defaults[level % defaults.size()]; } - auto it = std::begin(text); - const char32_t first = utf8::next(it, text.end()); + std::string_view remaining = text; + const char32_t first = util::string::next_utf8(remaining); if (first < 0xE000 || first > 0xF8FF) { return text; } diff --git a/src/odr/internal/svm/svm_to_svg.cpp b/src/odr/internal/svm/svm_to_svg.cpp index 27fdff6e9..11afe8a00 100644 --- a/src/odr/internal/svm/svm_to_svg.cpp +++ b/src/odr/internal/svm/svm_to_svg.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include @@ -16,11 +17,10 @@ #include #include #include +#include #include #include -#include - namespace odr::internal::svm { namespace { @@ -734,8 +734,8 @@ get_x_list_string(const IntPair &point, const std::string &text, const Context &context) { std::string result; std::size_t unit = 0; - auto position = text.begin(); - while (position != text.end()) { + std::string_view remaining = text; + while (!remaining.empty()) { if (unit > dx_array.size()) { return std::nullopt; } @@ -744,7 +744,7 @@ get_x_list_string(const IntPair &point, const std::string &text, result += " "; } result += svg::format_number(transform_x(point.x + advance, context)); - unit += utf8::next(position, text.end()) >= 0x10000 ? 2 : 1; + unit += util::string::next_utf8(remaining) >= 0x10000 ? 2 : 1; } return unit == dx_array.size() ? std::optional(std::move(result)) : std::nullopt; diff --git a/src/odr/internal/util/string_util.cpp b/src/odr/internal/util/string_util.cpp index 90c5e20c6..8fb1dfcb6 100644 --- a/src/odr/internal/util/string_util.cpp +++ b/src/odr/internal/util/string_util.cpp @@ -215,6 +215,25 @@ std::size_t string::utf8_length(const std::string &string) { return utf8::distance(string.begin(), string.end()); } +bool string::is_valid_utf8(const std::string_view string) { + return string.empty() || utf8::is_valid(string.begin(), string.end()); +} + +std::string string::replace_invalid_utf8(const std::string_view string) { + return string.empty() ? std::string{} : utf8::replace_invalid(string); +} + +char32_t string::next_utf8(std::string_view &remaining) { + if (remaining.empty()) { + throw std::out_of_range("empty UTF-8 input"); + } + auto position = remaining.begin(); + const char32_t result = utf8::next(position, remaining.end()); + remaining.remove_prefix( + static_cast(position - remaining.begin())); + return result; +} + std::vector string::utf16_offsets(const std::string_view string, const std::span indices) { diff --git a/src/odr/internal/util/string_util.hpp b/src/odr/internal/util/string_util.hpp index bd1b6e1ec..87566c2dd 100644 --- a/src/odr/internal/util/string_util.hpp +++ b/src/odr/internal/util/string_util.hpp @@ -83,6 +83,13 @@ std::string to_string(double d, int precision); std::size_t utf8_length(const std::string &string); +bool is_valid_utf8(std::string_view string); +/// Replace malformed UTF-8 sequences with U+FFFD. +std::string replace_invalid_utf8(std::string_view string); +/// Consume one code point; empty or invalid input throws without consuming +/// bytes. +char32_t next_utf8(std::string_view &remaining); + /// The byte offsets into @p string of the ascending UTF-16 code unit /// @p indices — the indexing a format that counts UTF-16 uses over UTF-8 text. /// Throws when @p string is not UTF-8 or an index is past its end. diff --git a/test/src/internal/util/string_util_test.cpp b/test/src/internal/util/string_util_test.cpp index 48d86d527..2a7a49b8e 100644 --- a/test/src/internal/util/string_util_test.cpp +++ b/test/src/internal/util/string_util_test.cpp @@ -2,6 +2,7 @@ #include +#include #include #include #include @@ -232,6 +233,29 @@ TEST(string_util, utf8_length_checks_incomplete_sequences) { EXPECT_ANY_THROW(utf8_length("\x80")); } +TEST(string_util, utf8_iteration_consumes_only_complete_code_points) { + using namespace std::string_view_literals; + std::string_view remaining = "A\0¢€😀"sv; + for (const char32_t expected : std::array{U'A', U'\0', U'¢', U'€', U'😀'}) { + EXPECT_EQ(next_utf8(remaining), expected); + } + EXPECT_TRUE(remaining.empty()); + EXPECT_THROW(next_utf8(remaining), std::out_of_range); + + for (const std::string_view invalid : + std::array{"\x80", "\xc0\xaf", "\xed\xa0\x80", + "\xf4\x90\x80\x80", "\xf0\x9f"}) { + remaining = invalid; + EXPECT_ANY_THROW(next_utf8(remaining)); + EXPECT_EQ(remaining, invalid); + } + + remaining = {}; + EXPECT_THROW(next_utf8(remaining), std::out_of_range); + EXPECT_TRUE(is_valid_utf8(remaining)); + EXPECT_TRUE(replace_invalid_utf8(remaining).empty()); +} + namespace { /// `utf16_offsets` over a braced index list, which its `std::span` does not From 3581f3b318c68acfa6d28c371cc2a91633523859 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Tue, 6 Oct 2026 21:56:33 +0200 Subject: [PATCH 2/2] refactor: route the newer utfcpp calls through the string utilities Main gained two utfcpp calls after this branch was cut: the bullet check in the OOXML list, which no longer built once this change dropped the include, and the font-name repair in the SFNT transform. Both use is_valid_utf8 and replace_invalid_utf8 now, so only string_util.cpp includes utfcpp, as AGENTS.md states. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01MxyTMutqSUJRGfxA8CyzMc --- src/odr/internal/font/sfnt_transform.cpp | 5 ++--- src/odr/internal/ooxml/text/ooxml_text_list.cpp | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/odr/internal/font/sfnt_transform.cpp b/src/odr/internal/font/sfnt_transform.cpp index 9e41864fc..86b9cf431 100644 --- a/src/odr/internal/font/sfnt_transform.cpp +++ b/src/odr/internal/font/sfnt_transform.cpp @@ -15,8 +15,6 @@ #include #include -#include - namespace odr::internal::font { namespace { @@ -292,7 +290,8 @@ std::string font::serialize_cmap(const std::map &map) { std::string font::serialize_name(const std::string &font_name) { // A CFF name may hold any bytes, and the UTF-16 conversion throws on them. const std::string family = - font_name.empty() ? "ODR Font" : utf8::replace_invalid(font_name); + font_name.empty() ? "ODR Font" + : util::string::replace_invalid_utf8(font_name); const auto utf16be = [](const std::string &value) { std::string out; for (const char16_t c : util::string::string_to_u16string(value)) { diff --git a/src/odr/internal/ooxml/text/ooxml_text_list.cpp b/src/odr/internal/ooxml/text/ooxml_text_list.cpp index 4b27bd678..dc57fa0ac 100644 --- a/src/odr/internal/ooxml/text/ooxml_text_list.cpp +++ b/src/odr/internal/ooxml/text/ooxml_text_list.cpp @@ -68,7 +68,7 @@ std::uint32_t read_number( std::string resolve_bullet(const std::string &text, const std::uint32_t level) { static constexpr std::array defaults{"•", "◦", "▪"}; - if (text.empty() || !utf8::is_valid(text.begin(), text.end())) { + if (text.empty() || !util::string::is_valid_utf8(text)) { return defaults[level % defaults.size()]; }