From f853c7c98e564bd5852c07c1bc8db5a17acbd4df Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Thu, 20 Aug 2026 20:17:51 +0200 Subject: [PATCH] fix(pdf): end an operator at every delimiter, and say when a font is dropped `read_operator_name` stopped at four of the nine delimiters of 7.2.2. `(` was not among them, so `Tm(text)Tj` let the name run on and swallow the string, and the show was dropped as an unknown operator. The whole set now ends a bareword; a stray closing one, which opens no token for any reader above to consume, is eaten so the caller's loop still makes progress. `font_is_usable` swallowed the re-encode failure with a bare `catch (...)`, though failing there swaps in a substitute and the page shows it. It now warns through the `Logger`. No corpus font takes that path, so no reference output moves. --- CHANGELOG.md | 4 ++ src/odr/internal/html/pdf_file.cpp | 47 ++++++++++++------- .../pdf/pdf_graphics_operator_parser.cpp | 25 ++++++++-- src/odr/internal/pdf/pdf_object_parser.cpp | 5 ++ src/odr/internal/pdf/pdf_object_parser.hpp | 2 + test/src/internal/pdf/pdf_page_extractor.cpp | 19 ++++++++ 6 files changed, 81 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 97692822..3620977f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,6 +45,10 @@ The release run heads these entries with the version and opens a fresh narrow boxes beside them. Runs one CSS transform can place share a selection block and carry the PDF's advances, and a whitespace-only run hands its advance on instead of dropping it — a space per word short on every line. +- A pdf that writes `Tm(text)Tj`, with nothing between an operator and the + string after it, renders that text instead of dropping it. +- An embedded font that will not re-encode says so in the log rather than being + swapped for a substitute in silence. ## v6.9.0 - 2026-08-18 diff --git a/src/odr/internal/html/pdf_file.cpp b/src/odr/internal/html/pdf_file.cpp index d5e97b20..6230dba8 100644 --- a/src/odr/internal/html/pdf_file.cpp +++ b/src/odr/internal/html/pdf_file.cpp @@ -1348,10 +1348,11 @@ class HtmlServiceImpl final : public HtmlService { std::unordered_map family_index; const auto font_family = [&](const pdf::Font *font) { - return intern_font(family_index, family_count, font, [&](std::uint32_t) { - accepted_fonts.push_back(font); - font_class_used.push_back({false, false}); - }); + return intern_font(family_index, family_count, font, m_logger, + [&](std::uint32_t) { + accepted_fonts.push_back(font); + font_class_used.push_back({false, false}); + }); }; const auto add_class = [&styles](std::string &classes, @@ -1921,12 +1922,13 @@ class HtmlServiceImpl final : public HtmlService { std::unordered_map family_index; const auto font_family = [&](pdf::Font *font) { - return intern_font(family_index, family_count, font, [&](std::uint32_t) { - accepted_fonts.push_back(font); - glyph_freq.emplace_back(); - used_unicode.emplace_back(); - font_class_used.push_back({false, false}); - }); + return intern_font(family_index, family_count, font, m_logger, + [&](std::uint32_t) { + accepted_fonts.push_back(font); + glyph_freq.emplace_back(); + used_unicode.emplace_back(); + font_class_used.push_back({false, false}); + }); }; AtomicStyles styles; @@ -2500,13 +2502,13 @@ class HtmlServiceImpl final : public HtmlService { template static std::uint32_t intern_font( std::unordered_map &family_index, - std::uint32_t &family_count, const pdf::Font *font, + std::uint32_t &family_count, const pdf::Font *font, const Logger &logger, OnAccept &&on_accept) { const auto [it, inserted] = family_index.try_emplace(font, 0); if (!inserted) { return it->second; } - if (!font_is_usable(*font)) { + if (!font_is_usable(*font, logger)) { return 0; } const std::uint32_t index = ++family_count; @@ -2649,15 +2651,26 @@ class HtmlServiceImpl final : public HtmlService { } /// Whether `font`'s embedded program re-encodes without throwing. Probes the - /// real encode path so failures surface here, not in the post-pass. - static bool font_is_usable(const pdf::Font &font) { + /// real encode path so failures surface here, not in the post-pass. Failing + /// swaps in a substitute, which the page shows, so say so. + static bool font_is_usable(const pdf::Font &font, const Logger &logger) { + const auto dropped = [&](const std::string &why) { + ODR_WARNING(logger, "pdf: rendering '" + << font.embedded_font->name() + << "' with a substitute, its embedded program " + "does not re-encode: " + << why); + return false; + }; if (const auto sfnt = std::dynamic_pointer_cast( font.embedded_font)) { try { (void)write_sfnt_pua(*sfnt, {}); return true; + } catch (const std::exception &e) { + return dropped(e.what()); } catch (...) { - return false; + return dropped("sfnt re-encode failed"); } } if (const auto cff = @@ -2665,8 +2678,10 @@ class HtmlServiceImpl final : public HtmlService { try { (void)font::cff::wrap_to_otf(*cff); return true; + } catch (const std::exception &e) { + return dropped(e.what()); } catch (...) { - return false; + return dropped("cff wrap failed"); } } return false; diff --git a/src/odr/internal/pdf/pdf_graphics_operator_parser.cpp b/src/odr/internal/pdf/pdf_graphics_operator_parser.cpp index 1fab7f07..998520b0 100644 --- a/src/odr/internal/pdf/pdf_graphics_operator_parser.cpp +++ b/src/odr/internal/pdf/pdf_graphics_operator_parser.cpp @@ -209,11 +209,10 @@ std::string GraphicsOperatorParser::read_operator_name() { if (c == eof) { return result; } - // Any white-space (7.2.2, incl. `\r` in CRLF streams) or the start of a - // following token ends the bareword. `%` is a delimiter too (7.2.2), so a - // comment may follow an operator with nothing in between. - if (ObjectParser::is_whitespace(static_cast(c)) || c == '/' || - c == '<' || c == '[' || c == '%') { + // White space or a delimiter ends the bareword (7.2.2): producers write + // `Tm(text)Tj` with nothing in between. + if (ObjectParser::is_whitespace(static_cast(c)) || + ObjectParser::is_delimiter(static_cast(c))) { return result; } @@ -252,6 +251,22 @@ GraphicsOperator GraphicsOperatorParser::read_operator() { } else if (operator_name == "false") { result.arguments.emplace_back(Boolean(false)); } else { + // A closing delimiter opens nothing, so no reader above consumes it and + // an unmatched one would stall the caller's loop. Eat it and go on. + if (operator_name.empty()) { + if (const int_type c = m_parser.geti(); + c != eof && + ObjectParser::is_delimiter(static_cast(c)) && + !m_parser.peek_name() && !m_parser.peek_string() && + !m_parser.peek_array() && !m_parser.peek_dictionary()) { + ODR_DEBUG(m_logger, "pdf: skipping stray delimiter '" + + std::string(1, static_cast(c)) + + "' in a content stream"); + m_parser.bumpc(); + m_parser.skip_whitespace_and_comments(); + continue; + } + } break; } } diff --git a/src/odr/internal/pdf/pdf_object_parser.cpp b/src/odr/internal/pdf/pdf_object_parser.cpp index 9859d1f1..0e56a4f7 100644 --- a/src/odr/internal/pdf/pdf_object_parser.cpp +++ b/src/odr/internal/pdf/pdf_object_parser.cpp @@ -115,6 +115,11 @@ bool ObjectParser::is_whitespace(const char c) { c == ' '; } +bool ObjectParser::is_delimiter(const char c) { + return c == '(' || c == ')' || c == '<' || c == '>' || c == '[' || c == ']' || + c == '{' || c == '}' || c == '/' || c == '%'; +} + bool ObjectParser::peek_whitespace() { const int_type c = geti(); return c != eof && is_whitespace(static_cast(c)); diff --git a/src/odr/internal/pdf/pdf_object_parser.hpp b/src/odr/internal/pdf/pdf_object_parser.hpp index a3126413..9ca15428 100644 --- a/src/odr/internal/pdf/pdf_object_parser.hpp +++ b/src/odr/internal/pdf/pdf_object_parser.hpp @@ -42,6 +42,8 @@ class ObjectParser { char_type third); static bool is_whitespace(char c); + /// The delimiters of 7.2.2, each of which opens a token of its own. + static bool is_delimiter(char c); [[nodiscard]] bool peek_whitespace(); void skip_whitespace(); /// White space plus the comments (`%` to the end of the line, 7.2.4) it may diff --git a/test/src/internal/pdf/pdf_page_extractor.cpp b/test/src/internal/pdf/pdf_page_extractor.cpp index ea63cf1b..b63bbacf 100644 --- a/test/src/internal/pdf/pdf_page_extractor.cpp +++ b/test/src/internal/pdf/pdf_page_extractor.cpp @@ -144,6 +144,25 @@ TEST(PdfPageExtractor, comment_directly_after_operator) { EXPECT_EQ(texts[0].codes, "Hi"); } +// A closing delimiter opens no token, so an unmatched one has to be eaten or +// the operator loop makes no progress. +TEST(PdfPageExtractor, stray_closing_delimiter_does_not_stall) { + const auto texts = run("BT /F1 12 Tf 1 0 0 1 5 5 Tm ) ] > } (Hi) Tj ET"); + ASSERT_EQ(texts.size(), 1); + EXPECT_EQ(texts[0].codes, "Hi"); +} + +// Nothing need separate an operator from the token after it (7.2.2). Without +// `(` ending the name, it ran on and swallowed the string behind it. +TEST(PdfPageExtractor, operator_directly_followed_by_a_string) { + const auto texts = run("BT /F1 12 Tf 1 0 0 1 100 700 Tm(Hi)Tj(there)Tj ET"); + ASSERT_EQ(texts.size(), 2); + EXPECT_DOUBLE_EQ(texts[0].transform.e, 100); + EXPECT_DOUBLE_EQ(texts[0].transform.f, 700); + EXPECT_EQ(texts[0].codes, "Hi"); + EXPECT_EQ(texts[1].codes, "there"); +} + // `Tm` sets the text matrix outright, scaling and all. TEST(PdfPageExtractor, tm_scaling) { const auto texts = run("BT /F1 10 Tf 2 0 0 2 50 60 Tm (X) Tj ET");