fix(pdf): stop embedded fonts rendering as tofu and wrong glyphs

src/odr/internal/font/cff_transform.cpp

@@ -5,9 +5,12 @@
 #include <odr/internal/util/byte_string.hpp>
 
 #include <algorithm>
+#include <cstddef>
 #include <cstdint>
 #include <map>
+#include <stdexcept>
 #include <string>
+#include <string_view>
 #include <utility>
 #include <vector>
 
@@ -89,6 +92,41 @@ std::string serialize_hmtx(const CffFont &font) {
   return hmtx;
 }
 
+/// A byte the OpenType Sanitizer rejects in a CFF font name.
+bool bad_name_byte(const char c) {
+  constexpr std::string_view delimiters = "[](){}<>/%";
+  const auto byte = static_cast<std::uint8_t>(c);
+  return byte <= 32 || byte >= 127 ||
+         delimiters.find(c) != std::string_view::npos;
+}
+
+/// Replace the bytes OTS rejects in the Name INDEX entry — it discards the
+/// whole `CFF ` table over them. Same-length and in place: the Top DICT's
+/// offsets are absolute.
+void sanitize_name_index(std::string &cff) {
+  const std::string_view d{cff};
+  const std::size_t header = bs::read_u8(d.substr(2)); // hdrSize
+  const std::uint16_t count = bs::read_u16_be(d.substr(header));
+  if (count == 0) {
+    return;
+  }
+  const std::size_t off_size = bs::read_u8(d.substr(header + 2));
+  const std::size_t offsets = header + 3;
+  const std::size_t data_base = offsets + (count + 1) * off_size - 1;
+  const std::size_t begin =
+      data_base + bs::read_uint_be(d.substr(offsets), off_size);
+  const std::size_t end =
+      data_base + bs::read_uint_be(d.substr(offsets + off_size), off_size);
+  if (begin > end || end > cff.size()) {
+    throw std::runtime_error("cff: bad Name INDEX offsets");
+  }
+  for (std::size_t i = begin; i < end; ++i) {
+    if (bad_name_byte(cff[i])) {
+      cff[i] = '-';
+    }
+  }
+}
+
 } // namespace
 
 } // namespace odr::internal::font::cff
@@ -114,8 +152,11 @@ std::string cff::wrap_to_otf(const CffFont &font,
   const char32_t first = pua.empty() ? 0 : pua.begin()->first;
   const char32_t last = pua.empty() ? 0 : pua.rbegin()->first;
 
+  std::string cff_table{font.data()};
+  sanitize_name_index(cff_table);
+
   std::vector<std::pair<std::string, std::string>> tables;
-  tables.emplace_back("CFF ", std::string(font.data()));
+  tables.emplace_back("CFF ", std::move(cff_table));
   tables.emplace_back("head", serialize_head(font.units_per_em(), bbox));
   tables.emplace_back("hhea", serialize_hhea(bbox, advance_width_max, glyphs));
   tables.emplace_back("maxp", serialize_maxp(glyphs));

src/odr/internal/font/sfnt_font.cpp

@@ -5,6 +5,7 @@
 #include <odr/internal/util/string_util.hpp>
 
 #include <algorithm>
+#include <array>
 #include <cstdint>
 #include <stdexcept>
 #include <string>
@@ -162,6 +163,22 @@ struct NameEntry {
   return 0;
 }
 
+/// The tables `write()` keeps. One malformed table — routine in a PDF subset —
+/// makes the OpenType Sanitizer reject the whole font, and the dropped ones buy
+/// nothing: the caller positions every glyph, so layout never applies.
+bool is_kept_table(const std::string_view tag) {
+  constexpr std::array<std::string_view, 22> kept = {
+      // required / metrics
+      "head", "hhea", "hmtx", "maxp", "cmap", "name", "OS/2", "post",
+      // outlines
+      "glyf", "loca", "CFF ", "CFF2",
+      // hinting
+      "cvt ", "fpgm", "prep", "gasp",
+      // color
+      "COLR", "CPAL", "CBDT", "CBLC", "sbix", "SVG "};
+  return std::ranges::find(kept, tag) != kept.end();
+}
+
 } // namespace
 
 bool SfntFont::is_sfnt(const std::string_view data) {
@@ -490,8 +507,9 @@ std::string SfntFont::write() const {
   std::vector<std::pair<std::string, std::string>> tables;
   tables.reserve(m_tables.size() + 1);
   for (const auto &[tag, location] : m_tables) {
-    if (tag == "cmap") {
-      continue; // rebuilt from the cmap() model below
+    // `cmap` is rebuilt from the cmap() model below.
+    if (tag == "cmap" || !is_kept_table(tag)) {
+      continue;
     }
     tables.emplace_back(tag, m_data.substr(location.offset, location.length));
   }

src/odr/internal/html/pdf_file.cpp

@@ -2404,23 +2404,35 @@ class HtmlServiceImpl final : public HtmlService {
     add_class(classes, "m", std::move(t).str());
   }
 
+  /// Serializes `sfnt` re-encoded to the PUA, restoring the cmap
+  /// `reencode_to_pua` overwrites — the `SfntFont` is shared by every page, and
+  /// a left-behind PUA cmap makes the next `glyph_for_code` miss.
+  static std::string
+  write_sfnt_pua(font::sfnt::SfntFont &sfnt,
+                 const std::map<char32_t, std::uint16_t> &extra_unicode) {
+    std::map<char32_t, std::uint16_t> original_cmap = sfnt.cmap();
+    try {
+      font::reencode_to_pua(sfnt, extra_unicode);
+      std::string reencoded = sfnt.write();
+      sfnt.set_cmap(std::move(original_cmap));
+      return reencoded;
+    } catch (...) {
+      sfnt.set_cmap(std::move(original_cmap));
+      throw;
+    }
+  }
+
   /// Whether `font`'s embedded program re-encodes without throwing. Probes the
-  /// real encode path so failures surface here, not in the post-pass, and
-  /// restores the SFNT cmap it mutates.
+  /// real encode path so failures surface here, not in the post-pass.
   static bool font_is_usable(const pdf::Font &font) {
     if (const auto sfnt = std::dynamic_pointer_cast<font::sfnt::SfntFont>(
             font.embedded_font)) {
-      std::map<char32_t, std::uint16_t> original_cmap = sfnt->cmap();
-      bool usable = false;
       try {
-        font::reencode_to_pua(*sfnt);
-        (void)sfnt->write();
-        usable = true;
+        (void)write_sfnt_pua(*sfnt, {});
+        return true;
       } catch (...) {
-        usable = false;
+        return false;
       }
-      sfnt->set_cmap(std::move(original_cmap));
-      return usable;
     }
     if (const auto cff =
             std::dynamic_pointer_cast<font::cff::CffFont>(font.embedded_font)) {
@@ -2454,8 +2466,7 @@ class HtmlServiceImpl final : public HtmlService {
     std::string reencoded;
     if (const auto sfnt = std::dynamic_pointer_cast<font::sfnt::SfntFont>(
             font.embedded_font)) {
-      font::reencode_to_pua(*sfnt, extra_unicode);
-      reencoded = sfnt->write();
+      reencoded = write_sfnt_pua(*sfnt, extra_unicode);
     } else if (const auto cff = std::dynamic_pointer_cast<font::cff::CffFont>(
                    font.embedded_font)) {
       reencoded = font::cff::wrap_to_otf(*cff, extra_unicode);

test/data.cmake

@@ -22,4 +22,4 @@ odr_test_data(
 odr_test_data(
         PATH "reference-output/odr-private"
         URL "https://github.com/opendocument-app/OpenDocument.test-private.output.git"
-        REVISION "27367bf7cdfe73ac40a01afed92f325627652755")
+        REVISION "7b9a508e84e3f281dfb9808e0dd03e2e70fd2cff")

test/src/internal/font/cff_font.cpp

@@ -11,6 +11,7 @@
 
 #include <cstdint>
 #include <string>
+#include <string_view>
 #include <vector>
 
 using namespace odr;
@@ -61,11 +62,26 @@ std::string build_index(const std::vector<std::string> &members) {
   return out;
 }
 
+/// The bytes of the @p tag table within @p sfnt, via its table directory.
+std::string sfnt_table(const std::string &sfnt, const std::string_view tag) {
+  const std::string_view d{sfnt};
+  const std::uint16_t count = bs::read_u16_be(d.substr(4));
+  for (std::uint16_t i = 0; i < count; ++i) {
+    const std::string_view entry = d.substr(12 + i * 16);
+    if (entry.substr(0, 4) == tag) {
+      return sfnt.substr(bs::read_u32_be(entry.substr(8)),
+                         bs::read_u32_be(entry.substr(12)));
+    }
+  }
+  return {};
+}
+
 /// Build a minimal name-keyed CFF: two glyphs (.notdef + one named glyph). When
 /// @p glyph1_sid is a custom SID (>= 391) the name comes from the String INDEX
 /// ("myglyph"); a standard SID (< 391) names the glyph via the CFF standard
 /// strings (no String INDEX entry needed).
-std::string build_cff(const std::uint16_t glyph1_sid = 391) {
+std::string build_cff(const std::uint16_t glyph1_sid = 391,
+                      const std::string &font_name = "TestFont") {
   // Charstrings (Type2): glyph 0 = endchar; glyph 1 = width-operand 50,
   // endchar. operand 50 -> single byte 50 + 139 = 189; endchar = 14.
   const std::string cs_notdef(1, static_cast<char>(14));
@@ -86,7 +102,7 @@ std::string build_cff(const std::uint16_t glyph1_sid = 391) {
   dict_int(private_dict, 200);
   private_dict += static_cast<char>(21);
 
-  const std::string name_index = build_index({"TestFont"});
+  const std::string name_index = build_index({font_name});
   const std::string string_index =
       glyph1_sid >= 391 ? build_index({"myglyph"}) : build_index({});
   const std::string global_subrs = build_index({});
@@ -440,6 +456,25 @@ TEST(CffFontTest, WrapsToLoadableOtf) {
   EXPECT_EQ(wrapped.advance_width(1), 250);
 }
 
+TEST(CffFontTest, WrapSanitizesTheFontName) {
+  using namespace odr::internal::font;
+  // PDF producers emit names with spaces and other PostScript delimiters; the
+  // OpenType Sanitizer discards the whole `CFF ` table over them.
+  const CffFont cff{build_cff(391, "*Test Font (3111)")};
+  ASSERT_EQ(cff.name(), "*Test Font (3111)");
+
+  const std::string otf = cff::wrap_to_otf(cff);
+  ASSERT_TRUE(sfnt::SfntFont::is_sfnt(otf));
+
+  // Same-length patch: the pass-through CFF still parses against its own
+  // absolute offsets.
+  const CffFont patched{sfnt_table(otf, "CFF ")};
+  EXPECT_EQ(patched.name(), "*Test-Font--3111-");
+  EXPECT_EQ(patched.glyph_count(), cff.glyph_count());
+  EXPECT_EQ(patched.glyph_name(1), cff.glyph_name(1));
+  EXPECT_EQ(patched.advance_width(1), cff.advance_width(1));
+}
+
 TEST(CffFontTest, WrapDropsExtraEntriesPastGlyphCount) {
   using namespace odr::internal::font;
   const CffFont cff{build_cff()}; // 2 glyphs: valid ids 0..1

test/src/internal/font/sfnt_transform.cpp

@@ -304,6 +304,24 @@ TEST(SfntTransform, write_keeps_existing_post) {
   EXPECT_EQ(table(out, "post"), original_post);
 }
 
+TEST(SfntTransform, write_drops_tables_it_does_not_need) {
+  // A malformed layout table costs the whole font at the sanitizer, and nothing
+  // here needs one; the outlines and their hinting must survive.
+  const std::string font = build_sfnt(0x00010000, {{"head", head_table()},
+                                                   {"maxp", maxp_table(3)},
+                                                   {"hhea", hhea_table(0)},
+                                                   {"prep", "hint"},
+                                                   {"GSUB", "junk"},
+                                                   {"DSIG", "junk"},
+                                                   {"FFTM", "junk"}});
+
+  const std::string out = reencoded(font);
+  EXPECT_EQ(table(out, "prep"), "hint");
+  EXPECT_FALSE(table(out, "GSUB").has_value());
+  EXPECT_FALSE(table(out, "DSIG").has_value());
+  EXPECT_FALSE(table(out, "FFTM").has_value());
+}
+
 TEST(SfntTransform, write_synthesizes_name_when_absent) {
   // OTS requires `name` and TrueType subsets often drop it; the writer must
   // synthesize a minimal one (empty source name falls back to "ODR Font").