From 020e1f856be4bf222effa92dd8476501e43bd4f2 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Wed, 26 Aug 2026 21:21:05 +0200 Subject: [PATCH 1/3] fix(pdf): read a simple font's codes one byte at a time MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A simple font's character codes are one byte each (ISO 32000-1 9.10.3), but `to_unicode` split them by whatever codespace the `/ToUnicode` CMap declared. Producers write the two-byte `<0000> ` boilerplate there regardless, so the bytes were paired up and the extracted text came out as CJK — "IAN 479084_2410" as "䥁丠㐷㤰㠴弲㐱0". Display was unaffected (the glyph path splits via `Font::codes`, which already imposes the width), so this only ever showed up in selection, search and copy. `translate_string` now takes the width to split by, and `to_unicode` imposes one byte for a simple font. An imposed one-byte code is also looked up zero-padded, producers keying the entries either way; a declared mixed codespace still keeps `<20>` and `<0020>` distinct. Across the pdf corpus this moves no pixel and takes word-level extraction recall against ghostscript from 30-70% to ~100% on the five affected files. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_0169dUoxqhqfi7ZU5ei8b9ov --- CHANGELOG.md | 3 ++ src/odr/internal/pdf/AGENTS.md | 8 ++++- src/odr/internal/pdf/pdf_cmap.cpp | 19 ++++++++++-- src/odr/internal/pdf/pdf_cmap.hpp | 7 ++++- src/odr/internal/pdf/pdf_document.cpp | 8 +++-- test/src/internal/pdf/pdf_cmap.cpp | 43 +++++++++++++++++++++++++++ 6 files changed, 81 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7847b6190..6ba90ff31 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -77,6 +77,9 @@ The release run heads these entries with the version and opens a fresh missing. - An odf shape or frame that is not filled no longer paints the colour of one that was. +- Text in a pdf can be selected, searched and copied where it came out as CJK + before. A simple font's codes are one byte each, whatever codespace its + `ToUnicode` map declares, and producers routinely declare two. ## v6.10.1 - 2026-08-21 diff --git a/src/odr/internal/pdf/AGENTS.md b/src/odr/internal/pdf/AGENTS.md index 132c0757c..320b48408 100644 --- a/src/odr/internal/pdf/AGENTS.md +++ b/src/odr/internal/pdf/AGENTS.md @@ -53,7 +53,13 @@ directly; legacy CJK CMaps via `pdf_cid`/`pdf_cid_data`: code → CID → Unicod `/CIDSystemInfo` collection for `Identity-H`/embedded-CMap → embedded-font reverse map (code → glyph → `code_point_for_glyph`). Only a genuinely unmapped code yields "no Unicode" — never byte-garbage. The point of the chain is that each link -recovers a class of real-world PDF the previous one misses. +recovers a class of real-world PDF the previous one misses. A **simple font's +codes are one byte** (9.10.3), so `to_unicode` imposes that width rather than +read it off the `/ToUnicode` codespace — producers write the two-byte +`<0000> ` boilerplate there regardless, and splitting by it pairs the +bytes into CJK. The entries themselves are keyed either way, so an imposed +one-byte code is also looked up zero-padded. Composite codes keep splitting by +the codespace, as `Font::codes` splits them. **`std::any`-based object model.** `Object` holds its value in `std::any` with typed `is_*`/`as_*` accessors (mirrors `oldms/`'s `Entry`). Pro: one value type diff --git a/src/odr/internal/pdf/pdf_cmap.cpp b/src/odr/internal/pdf/pdf_cmap.cpp index aa14c682c..67a3a1271 100644 --- a/src/odr/internal/pdf/pdf_cmap.cpp +++ b/src/odr/internal/pdf/pdf_cmap.cpp @@ -4,6 +4,7 @@ #include #include +#include namespace odr::internal::pdf { @@ -62,13 +63,15 @@ std::size_t CMap::code_length(const std::string &codes, return code_width(static_cast(codes[pos])); } -std::string CMap::translate_string(const std::string &codes) const { +std::string +CMap::translate_string(const std::string &codes, + const std::optional code_width) const { std::u16string result; std::size_t pos = 0; while (pos < codes.size()) { - const std::size_t width = - std::min(code_length(codes, pos), codes.size() - pos); + const std::size_t width = std::min( + code_width.value_or(code_length(codes, pos)), codes.size() - pos); const std::string code = codes.substr(pos, width); pos += width; @@ -77,6 +80,16 @@ std::string CMap::translate_string(const std::string &codes) const { continue; } + // Only for an imposed width — a declared mixed codespace keeps `<20>` and + // `<0020>` distinct. + if (code_width.has_value() && code.size() == 1) { + if (const auto it = m_map.find(std::string(1, '\0') + code); + it != m_map.end()) { + result += it->second; + continue; + } + } + // Unknown code: fall back to its numeric value as a single UTF-16 unit // (identity for single-byte codes). These "no Unicode" runs are left for // later re-encoding. diff --git a/src/odr/internal/pdf/pdf_cmap.hpp b/src/odr/internal/pdf/pdf_cmap.hpp index fb8f788bf..e8f8e791e 100644 --- a/src/odr/internal/pdf/pdf_cmap.hpp +++ b/src/odr/internal/pdf/pdf_cmap.hpp @@ -56,7 +56,12 @@ class CMap { /// does, keeping a mixed 1-/2-byte codespace aligned across both. [[nodiscard]] std::size_t code_width(std::uint8_t first) const; - [[nodiscard]] std::string translate_string(const std::string &codes) const; + /// `code_width` overrides the codespace ranges. An imposed single-byte code + /// is also looked up zero-padded to two bytes, producers keying the entries + /// either way. + [[nodiscard]] std::string + translate_string(const std::string &codes, + std::optional code_width = {}) const; /// True when at least one `cidchar`/`cidrange` mapping was parsed (an /// embedded CID `/Encoding` CMap). When false the composite code -> CID is diff --git a/src/odr/internal/pdf/pdf_document.cpp b/src/odr/internal/pdf/pdf_document.cpp index ec8443906..f37324dc2 100644 --- a/src/odr/internal/pdf/pdf_document.cpp +++ b/src/odr/internal/pdf/pdf_document.cpp @@ -161,8 +161,12 @@ std::uint16_t Font::glyph_for_code(const std::uint32_t code) const { } std::string Font::to_unicode(const std::string &codes) const { + // A simple font's codes are one byte each (ISO 32000-1 9.10.3); its + // `ToUnicode` codespace is not to be trusted, producers writing the + // two-byte `<0000> ` boilerplate there regardless. if (!cmap.empty()) { - return cmap.translate_string(codes); + return composite ? cmap.translate_string(codes) + : cmap.translate_string(codes, 1); } if (composite) { // A composite (Type0) font with no `ToUnicode` CMap. A predefined @@ -210,7 +214,7 @@ std::string Font::to_unicode(const std::string &codes) const { !unicode.empty()) { return unicode; } - return cmap.translate_string(codes); + return cmap.translate_string(codes, 1); } } // namespace odr::internal::pdf diff --git a/test/src/internal/pdf/pdf_cmap.cpp b/test/src/internal/pdf/pdf_cmap.cpp index 137c6cec5..fecdfe877 100644 --- a/test/src/internal/pdf/pdf_cmap.cpp +++ b/test/src/internal/pdf/pdf_cmap.cpp @@ -199,3 +199,46 @@ TEST(PdfCMap, usecmap_disables_local_codespace_authority) { EXPECT_TRUE(cmap.has_cid_map()); EXPECT_EQ(cmap.cid_for_code(std::string_view("\x20", 1)), 1u); } + +TEST(PdfCMap, imposed_code_width_overrides_codespace) { + // A simple font's `ToUnicode` CMap carrying the two-byte `<0000> ` + // boilerplate over one-byte entries; splitting by it pairs the codes up. + CMap cmap = parse("1 begincodespacerange\n" + "<0000> \n" + "endcodespacerange\n" + "2 beginbfchar\n" + "<41> <0041>\n" + "<42> <0042>\n" + "endbfchar\n"); + + EXPECT_EQ(cmap.translate_string("\x41\x42"), "\xe4\x85\x82"); // U+4142 + EXPECT_EQ(cmap.translate_string("\x41\x42", 1), "AB"); +} + +TEST(PdfCMap, imposed_code_width_falls_back_to_a_padded_entry) { + CMap cmap = parse("1 begincodespacerange\n" + "<0000> \n" + "endcodespacerange\n" + "2 beginbfchar\n" + "<0041> <0041>\n" + "<0042> <0042>\n" + "endbfchar\n"); + + EXPECT_EQ(cmap.translate_string("\x41\x42", 1), "AB"); +} + +TEST(PdfCMap, imposed_code_width_keeps_a_mixed_codespace_distinct) { + // Padding is only for an imposed width; a declared mixed codespace keeps + // `<20>` and `<2120>` apart. + CMap cmap = parse("2 begincodespacerange\n" + "<00> <20>\n" + "<2100> \n" + "endcodespacerange\n" + "2 beginbfchar\n" + "<20> <0041>\n" + "<2120> <0042>\n" + "endbfchar\n"); + + EXPECT_EQ(cmap.translate_string("\x20"), "A"); + EXPECT_EQ(cmap.translate_string("\x21\x20"), "B"); +} From 6c58a02a5822168b09cd89a41fafee0ac8276c07 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Thu, 27 Aug 2026 21:22:38 +0200 Subject: [PATCH 2/3] test(pdf): pin the simple-font width dispatch, and name the flag for it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-up: the dispatch in Font::to_unicode that imposes the one-byte width was untested — the CMap tests call translate_string directly, so reverting it would have slipped through. Two Font-level tests now pin both arms. The optional code_width parameter only ever carried 1; a bool single_byte_codes says what it means and drops the dead width guard. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Hsg62JPF5kXLixJbjULmr5 --- src/odr/internal/pdf/pdf_cmap.cpp | 13 +++++++------ src/odr/internal/pdf/pdf_cmap.hpp | 9 +++++---- src/odr/internal/pdf/pdf_document.cpp | 5 ++--- test/src/internal/pdf/pdf_cmap.cpp | 6 ++++-- test/src/internal/pdf/pdf_font.cpp | 23 +++++++++++++++++++++++ 5 files changed, 41 insertions(+), 15 deletions(-) diff --git a/src/odr/internal/pdf/pdf_cmap.cpp b/src/odr/internal/pdf/pdf_cmap.cpp index 67a3a1271..63d0dff41 100644 --- a/src/odr/internal/pdf/pdf_cmap.cpp +++ b/src/odr/internal/pdf/pdf_cmap.cpp @@ -63,15 +63,16 @@ std::size_t CMap::code_length(const std::string &codes, return code_width(static_cast(codes[pos])); } -std::string -CMap::translate_string(const std::string &codes, - const std::optional code_width) const { +std::string CMap::translate_string(const std::string &codes, + const bool single_byte_codes) const { std::u16string result; std::size_t pos = 0; while (pos < codes.size()) { - const std::size_t width = std::min( - code_width.value_or(code_length(codes, pos)), codes.size() - pos); + const std::size_t width = + single_byte_codes + ? 1 + : std::min(code_length(codes, pos), codes.size() - pos); const std::string code = codes.substr(pos, width); pos += width; @@ -82,7 +83,7 @@ CMap::translate_string(const std::string &codes, // Only for an imposed width — a declared mixed codespace keeps `<20>` and // `<0020>` distinct. - if (code_width.has_value() && code.size() == 1) { + if (single_byte_codes) { if (const auto it = m_map.find(std::string(1, '\0') + code); it != m_map.end()) { result += it->second; diff --git a/src/odr/internal/pdf/pdf_cmap.hpp b/src/odr/internal/pdf/pdf_cmap.hpp index e8f8e791e..1e249d5cc 100644 --- a/src/odr/internal/pdf/pdf_cmap.hpp +++ b/src/odr/internal/pdf/pdf_cmap.hpp @@ -56,12 +56,13 @@ class CMap { /// does, keeping a mixed 1-/2-byte codespace aligned across both. [[nodiscard]] std::size_t code_width(std::uint8_t first) const; - /// `code_width` overrides the codespace ranges. An imposed single-byte code - /// is also looked up zero-padded to two bytes, producers keying the entries - /// either way. + /// `single_byte_codes` overrides the codespace ranges and splits the codes + /// one byte each (a simple font's width, ISO 32000-1 9.10.3). An imposed + /// single-byte code is also looked up zero-padded to two bytes, producers + /// keying the entries either way. [[nodiscard]] std::string translate_string(const std::string &codes, - std::optional code_width = {}) const; + bool single_byte_codes = false) const; /// True when at least one `cidchar`/`cidrange` mapping was parsed (an /// embedded CID `/Encoding` CMap). When false the composite code -> CID is diff --git a/src/odr/internal/pdf/pdf_document.cpp b/src/odr/internal/pdf/pdf_document.cpp index f37324dc2..a03fb24a8 100644 --- a/src/odr/internal/pdf/pdf_document.cpp +++ b/src/odr/internal/pdf/pdf_document.cpp @@ -165,8 +165,7 @@ std::string Font::to_unicode(const std::string &codes) const { // `ToUnicode` codespace is not to be trusted, producers writing the // two-byte `<0000> ` boilerplate there regardless. if (!cmap.empty()) { - return composite ? cmap.translate_string(codes) - : cmap.translate_string(codes, 1); + return cmap.translate_string(codes, /*single_byte_codes=*/!composite); } if (composite) { // A composite (Type0) font with no `ToUnicode` CMap. A predefined @@ -214,7 +213,7 @@ std::string Font::to_unicode(const std::string &codes) const { !unicode.empty()) { return unicode; } - return cmap.translate_string(codes, 1); + return cmap.translate_string(codes, /*single_byte_codes=*/true); } } // namespace odr::internal::pdf diff --git a/test/src/internal/pdf/pdf_cmap.cpp b/test/src/internal/pdf/pdf_cmap.cpp index fecdfe877..995c45af6 100644 --- a/test/src/internal/pdf/pdf_cmap.cpp +++ b/test/src/internal/pdf/pdf_cmap.cpp @@ -212,7 +212,8 @@ TEST(PdfCMap, imposed_code_width_overrides_codespace) { "endbfchar\n"); EXPECT_EQ(cmap.translate_string("\x41\x42"), "\xe4\x85\x82"); // U+4142 - EXPECT_EQ(cmap.translate_string("\x41\x42", 1), "AB"); + EXPECT_EQ(cmap.translate_string("\x41\x42", /*single_byte_codes=*/true), + "AB"); } TEST(PdfCMap, imposed_code_width_falls_back_to_a_padded_entry) { @@ -224,7 +225,8 @@ TEST(PdfCMap, imposed_code_width_falls_back_to_a_padded_entry) { "<0042> <0042>\n" "endbfchar\n"); - EXPECT_EQ(cmap.translate_string("\x41\x42", 1), "AB"); + EXPECT_EQ(cmap.translate_string("\x41\x42", /*single_byte_codes=*/true), + "AB"); } TEST(PdfCMap, imposed_code_width_keeps_a_mixed_codespace_distinct) { diff --git a/test/src/internal/pdf/pdf_font.cpp b/test/src/internal/pdf/pdf_font.cpp index d0f7261b1..85705edc6 100644 --- a/test/src/internal/pdf/pdf_font.cpp +++ b/test/src/internal/pdf/pdf_font.cpp @@ -184,6 +184,29 @@ TEST(PdfFont, to_unicode_prefers_cmap_over_reverse_map) { EXPECT_EQ(font.to_unicode(codes2({1})), "Z"); } +TEST(PdfFont, simple_font_to_unicode_ignores_cmap_codespace) { + // A simple font whose `ToUnicode` CMap declares the two-byte + // `<0000> ` boilerplate: the codes still split one byte each + // (splitting by the codespace would pair them into U+4142). + Font font; + font.cmap.add_codespace_range(codes2({0}), codes2({0xffff})); + font.cmap.map_single("\x41", u"A"); + font.cmap.map_single("\x42", u"B"); + + EXPECT_EQ(font.to_unicode("\x41\x42"), "AB"); +} + +TEST(PdfFont, composite_to_unicode_splits_by_cmap_codespace) { + // The same codespace on a composite font stays authoritative: the two + // bytes form one code. + Font font; + font.composite = true; + font.cmap.add_codespace_range(codes2({0}), codes2({0xffff})); + font.cmap.map_single(codes2({0x4142}), u"Z"); + + EXPECT_EQ(font.to_unicode("\x41\x42"), "Z"); +} + TEST(PdfFont, simple_font_glyph_for_code_via_cmap) { // A simple (1-byte) TrueType font: the code's Unicode reaches the glyph // through the embedded (3,1) cmap. From 34d2aebad0716ad2e8b69ad9de5ad056dd952a31 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Thu, 27 Aug 2026 21:38:43 +0200 Subject: [PATCH 3/3] style(pdf): drop narration comments around the width dispatch Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Hsg62JPF5kXLixJbjULmr5 --- src/odr/internal/pdf/pdf_cmap.hpp | 3 +-- src/odr/internal/pdf/pdf_document.cpp | 4 ++-- test/src/internal/pdf/pdf_cmap.cpp | 6 ++---- test/src/internal/pdf/pdf_font.cpp | 5 ----- 4 files changed, 5 insertions(+), 13 deletions(-) diff --git a/src/odr/internal/pdf/pdf_cmap.hpp b/src/odr/internal/pdf/pdf_cmap.hpp index 1e249d5cc..3da8e89d5 100644 --- a/src/odr/internal/pdf/pdf_cmap.hpp +++ b/src/odr/internal/pdf/pdf_cmap.hpp @@ -56,8 +56,7 @@ class CMap { /// does, keeping a mixed 1-/2-byte codespace aligned across both. [[nodiscard]] std::size_t code_width(std::uint8_t first) const; - /// `single_byte_codes` overrides the codespace ranges and splits the codes - /// one byte each (a simple font's width, ISO 32000-1 9.10.3). An imposed + /// `single_byte_codes` overrides the codespace ranges. An imposed /// single-byte code is also looked up zero-padded to two bytes, producers /// keying the entries either way. [[nodiscard]] std::string diff --git a/src/odr/internal/pdf/pdf_document.cpp b/src/odr/internal/pdf/pdf_document.cpp index a03fb24a8..46278a96d 100644 --- a/src/odr/internal/pdf/pdf_document.cpp +++ b/src/odr/internal/pdf/pdf_document.cpp @@ -165,7 +165,7 @@ std::string Font::to_unicode(const std::string &codes) const { // `ToUnicode` codespace is not to be trusted, producers writing the // two-byte `<0000> ` boilerplate there regardless. if (!cmap.empty()) { - return cmap.translate_string(codes, /*single_byte_codes=*/!composite); + return cmap.translate_string(codes, !composite); } if (composite) { // A composite (Type0) font with no `ToUnicode` CMap. A predefined @@ -213,7 +213,7 @@ std::string Font::to_unicode(const std::string &codes) const { !unicode.empty()) { return unicode; } - return cmap.translate_string(codes, /*single_byte_codes=*/true); + return cmap.translate_string(codes, true); } } // namespace odr::internal::pdf diff --git a/test/src/internal/pdf/pdf_cmap.cpp b/test/src/internal/pdf/pdf_cmap.cpp index 995c45af6..9e751cfc1 100644 --- a/test/src/internal/pdf/pdf_cmap.cpp +++ b/test/src/internal/pdf/pdf_cmap.cpp @@ -212,8 +212,7 @@ TEST(PdfCMap, imposed_code_width_overrides_codespace) { "endbfchar\n"); EXPECT_EQ(cmap.translate_string("\x41\x42"), "\xe4\x85\x82"); // U+4142 - EXPECT_EQ(cmap.translate_string("\x41\x42", /*single_byte_codes=*/true), - "AB"); + EXPECT_EQ(cmap.translate_string("\x41\x42", true), "AB"); } TEST(PdfCMap, imposed_code_width_falls_back_to_a_padded_entry) { @@ -225,8 +224,7 @@ TEST(PdfCMap, imposed_code_width_falls_back_to_a_padded_entry) { "<0042> <0042>\n" "endbfchar\n"); - EXPECT_EQ(cmap.translate_string("\x41\x42", /*single_byte_codes=*/true), - "AB"); + EXPECT_EQ(cmap.translate_string("\x41\x42", true), "AB"); } TEST(PdfCMap, imposed_code_width_keeps_a_mixed_codespace_distinct) { diff --git a/test/src/internal/pdf/pdf_font.cpp b/test/src/internal/pdf/pdf_font.cpp index 85705edc6..d7ec64cd7 100644 --- a/test/src/internal/pdf/pdf_font.cpp +++ b/test/src/internal/pdf/pdf_font.cpp @@ -185,9 +185,6 @@ TEST(PdfFont, to_unicode_prefers_cmap_over_reverse_map) { } TEST(PdfFont, simple_font_to_unicode_ignores_cmap_codespace) { - // A simple font whose `ToUnicode` CMap declares the two-byte - // `<0000> ` boilerplate: the codes still split one byte each - // (splitting by the codespace would pair them into U+4142). Font font; font.cmap.add_codespace_range(codes2({0}), codes2({0xffff})); font.cmap.map_single("\x41", u"A"); @@ -197,8 +194,6 @@ TEST(PdfFont, simple_font_to_unicode_ignores_cmap_codespace) { } TEST(PdfFont, composite_to_unicode_splits_by_cmap_codespace) { - // The same codespace on a composite font stays authoritative: the two - // bytes form one code. Font font; font.composite = true; font.cmap.add_codespace_range(codes2({0}), codes2({0xffff}));