fix(font): validate CFF output and preserve fractional widths

CHANGELOG.md

@@ -16,6 +16,9 @@ The release run heads these entries with the version and opens a fresh
 
 ## Unreleased
 
+- CFF output validates glyph names and offset limits, reuses standard glyph-name
+  identifiers, and preserves fractional width operands until the final advance.
+
 - Type1 conversion honors subroutine returns, fractional widths and vertical
   side bearings, and drops hint commands that Type2 cannot take in order.
 

src/odr/internal/font/cff_builder.cpp

@@ -1,12 +1,23 @@
 #include <odr/internal/font/cff_builder.hpp>
 
+#include <odr/internal/font/cff_standard_strings.hpp>
 #include <odr/internal/util/byte_string.hpp>
+#include <odr/internal/util/number_util.hpp>
 
+#include <algorithm>
+#include <cmath>
 #include <cstdint>
+#include <limits>
+#include <optional>
 #include <ranges>
+#include <stdexcept>
 #include <string>
+#include <unordered_set>
+#include <utility>
 #include <vector>
 
+#include <fmt/format.h>
+
 namespace odr::internal::font::cff {
 
 namespace {
@@ -35,6 +46,43 @@ void dict_int(std::string &s, const std::int32_t v) {
   }
 }
 
+void dict_number(std::string &out, const double value) {
+  if (!std::isfinite(value)) {
+    throw std::runtime_error("cff: non-finite DICT operand");
+  }
+  if (const std::optional<std::int32_t> integer =
+          util::number::to_integer<std::int32_t>(value)) {
+    dict_int(out, *integer);
+    return;
+  }
+  const std::string text = fmt::format("{:.17g}", value);
+  std::vector<std::uint8_t> nibbles;
+  for (std::size_t i = 0; i < text.size(); ++i) {
+    const char c = text[i];
+    if (c == '.') {
+      nibbles.push_back(0xA);
+    } else if (c == '-') {
+      nibbles.push_back(0xE);
+    } else if (c == 'e') {
+      const bool negative = text[i + 1] == '-';
+      nibbles.push_back(negative ? 0xC : 0xB);
+      if (negative || text[i + 1] == '+') {
+        ++i;
+      }
+    } else {
+      nibbles.push_back(static_cast<std::uint8_t>(c - '0'));
+    }
+  }
+  nibbles.push_back(0xF);
+  if (nibbles.size() % 2 != 0) {
+    nibbles.push_back(0xF);
+  }
+  out += static_cast<char>(30);
+  for (std::size_t i = 0; i < nibbles.size(); i += 2) {
+    out += static_cast<char>((nibbles[i] << 4) | nibbles[i + 1]);
+  }
+}
+
 /// A CFF DICT integer in the fixed 5-byte form (`29 + int32`), so an operand
 /// whose value (an offset) is not yet known can be sized before it is filled.
 void dict_int_fixed(std::string &s, const std::int32_t v) {
@@ -56,6 +104,9 @@ void dict_operator(std::string &s, const std::int32_t op) {
 
 /// Serialize a CFF INDEX from its members.
 std::string build_index(const std::vector<std::string> &members) {
+  if (!std::in_range<std::uint16_t>(members.size())) {
+    throw std::runtime_error("cff: too many INDEX members");
+  }
   std::string out;
   util::byte_string::put_u16_be(out,
                                 static_cast<std::uint16_t>(members.size()));
@@ -64,6 +115,9 @@ std::string build_index(const std::vector<std::string> &members) {
   }
   std::uint32_t total = 1;
   for (const std::string &m : members) {
+    if (m.size() > std::numeric_limits<std::uint32_t>::max() - total) {
+      throw std::runtime_error("cff: INDEX exceeds 32-bit offsets");
+    }
     total += static_cast<std::uint32_t>(m.size());
   }
   const std::uint8_t off_size = total <= 0xff       ? 1
@@ -98,6 +152,10 @@ std::string cff::build_cff(const std::string_view name,
                            const std::span<const BuilderGlyph> glyphs,
                            const double default_width,
                            const double nominal_width, const FontBBox bbox) {
+  if (glyphs.empty() || glyphs.size() > 65000 ||
+      glyphs.front().name != ".notdef") {
+    throw std::runtime_error("cff: invalid name-keyed glyph set");
+  }
   // CharStrings INDEX (one Type2 charstring per glyph).
   std::vector<std::string> charstrings;
   charstrings.reserve(glyphs.size());
@@ -106,27 +164,32 @@ std::string cff::build_cff(const std::string_view name,
   }
   const std::string charstrings_index = build_index(charstrings);
 
-  // String INDEX: every glyph name gets a custom SID (391 + position). Glyph 0
-  // is the implicit `.notdef` (SID 0), so its name is not stored; the charset
-  // lists SIDs for glyphs 1..n-1.
-  const std::string string_index =
-      build_index(glyphs | std::views::drop(1) |
-                  std::views::transform(&BuilderGlyph::name) |
-                  std::ranges::to<std::vector<std::string>>());
-
-  // Format-0 charset: SID per glyph 1..n-1.
-  std::string charset;
-  charset += static_cast<char>(0); // format 0
-  for (std::size_t i = 1; i < glyphs.size(); ++i) {
-    util::byte_string::put_u16_be(charset,
-                                  static_cast<std::uint16_t>(391 + (i - 1)));
+  std::vector<std::string> strings;
+  std::unordered_set<std::string_view> names{".notdef"};
+  std::string charset(1, '\0'); // format 0, with implicit .notdef
+  for (const BuilderGlyph &glyph : glyphs | std::views::drop(1)) {
+    if (!names.insert(glyph.name).second) {
+      throw std::runtime_error("cff: duplicate glyph name");
+    }
+    const auto standard = std::ranges::find(cff_standard_strings, glyph.name);
+    std::size_t sid =
+        static_cast<std::size_t>(standard - cff_standard_strings.begin());
+    if (standard == cff_standard_strings.end()) {
+      sid = cff_standard_strings_size + strings.size();
+      if (sid >= 65000) {
+        throw std::runtime_error("cff: too many custom strings");
+      }
+      strings.push_back(glyph.name);
+    }
+    util::byte_string::put_u16_be(charset, static_cast<std::uint16_t>(sid));
   }
+  const std::string string_index = build_index(strings);
 
   // Private DICT: defaultWidthX (20), nominalWidthX (21).
   std::string private_dict;
-  dict_int(private_dict, static_cast<std::int32_t>(default_width));
+  dict_number(private_dict, default_width);
   dict_operator(private_dict, 20);
-  dict_int(private_dict, static_cast<std::int32_t>(nominal_width));
+  dict_number(private_dict, nominal_width);
   dict_operator(private_dict, 21);
 
   const std::string name_index =
@@ -155,16 +218,19 @@ std::string cff::build_cff(const std::string_view name,
   };
 
   const std::string top_dict_probe = build_index({top_dict(0, 0, 0)});
-  constexpr std::uint32_t header_size = 4;
-  const auto prefix = static_cast<std::uint32_t>(
-      header_size + name_index.size() + top_dict_probe.size() +
-      string_index.size() + global_subrs.size());
-  // Layout after the prefix: CharStrings, charset, Private.
-  const std::uint32_t charstrings_off = prefix;
-  const std::uint32_t charset_off =
-      charstrings_off + static_cast<std::uint32_t>(charstrings_index.size());
-  const std::uint32_t private_off =
-      charset_off + static_cast<std::uint32_t>(charset.size());
+  constexpr std::uint64_t header_size = 4;
+  const std::uint64_t prefix = header_size + name_index.size() +
+                               top_dict_probe.size() + string_index.size() +
+                               global_subrs.size();
+  const std::uint64_t charset_start = prefix + charstrings_index.size();
+  const std::uint64_t private_start = charset_start + charset.size();
+  if (private_start + private_dict.size() >
+      std::numeric_limits<std::int32_t>::max()) {
+    throw std::runtime_error("cff: output exceeds signed DICT offsets");
+  }
+  const auto charstrings_off = static_cast<std::uint32_t>(prefix);
+  const auto charset_off = static_cast<std::uint32_t>(charset_start);
+  const auto private_off = static_cast<std::uint32_t>(private_start);
 
   const std::string top_dict_index =
       build_index({top_dict(charset_off, charstrings_off, private_off)});

src/odr/internal/font/cff_builder.hpp

@@ -15,18 +15,8 @@ struct BuilderGlyph {
   std::string charstring;
 };
 
-/// Serialize a name-keyed CFF font from Type2 charstrings.
-///
-/// Assembles the minimal CFF a `CffFont` reader (and, after wrapping, a
-/// browser) needs: Header, Name INDEX, Top DICT (FontBBox +
-/// charset/CharStrings/Private offsets), String INDEX (every glyph name, SID
-/// 391+), an empty Global Subr INDEX, the CharStrings INDEX, a format-0 charset
-/// and a Private DICT (`defaultWidthX`/`nominalWidthX`). The caller orders
-/// @p glyphs so glyph 0 is the implicit `.notdef`.
-///
-/// No `FontMatrix` is emitted, so the font is 1000 units/em (the Type1
-/// default); a non-default matrix is a follow-up. Top DICT offsets use the
-/// fixed-width 5-byte integer form so the layout resolves in a single pass.
+/// Serialize Type2 glyphs into a name-keyed CFF at 1000 units/em.
+/// Glyph 0 must be .notdef; names must be unique and fit the CFF SID space.
 [[nodiscard]] std::string build_cff(std::string_view name,
                                     std::span<const BuilderGlyph> glyphs,
                                     double default_width, double nominal_width,

src/odr/internal/font/cff_font.cpp

@@ -527,7 +527,7 @@ CffFont::sid_for_string(const std::string_view string) const {
   return std::nullopt;
 }
 
-std::optional<std::int32_t>
+std::optional<double>
 CffFont::charstring_width(const std::uint16_t glyph) const {
   if (glyph >= m_charstrings.size()) {
     return std::nullopt;
@@ -598,9 +598,7 @@ CffFont::charstring_width(const std::uint16_t glyph) const {
         // Any other operator before a width-bearing one: no explicit width.
         return std::nullopt;
       }
-      return width_possible
-                 ? std::optional<std::int32_t>(static_cast<std::int32_t>(first))
-                 : std::nullopt;
+      return width_possible ? std::optional<double>(first) : std::nullopt;
     }
   }
   return std::nullopt;
@@ -626,7 +624,7 @@ FontBBox CffFont::bounding_box() const noexcept { return m_bbox; }
 
 std::uint16_t CffFont::advance_width(const std::uint16_t glyph) const {
   const Widths &widths = widths_for_glyph(glyph);
-  const std::optional<std::int32_t> width = charstring_width(glyph);
+  const std::optional<double> width = charstring_width(glyph);
   const double advance =
       width.has_value() ? widths.nominal_width + *width : widths.default_width;
   // An advance is a uFWord; clamping keeps a hostile Private DICT out of the

src/odr/internal/font/cff_font.hpp

@@ -105,7 +105,7 @@ class CffFont final : public abstract::Font {
   sid_for_string(std::string_view string) const;
   /// Extract the optional leading width from glyph @p glyph's Type2 charstring,
   /// in design units; `nullopt` when the charstring carries no explicit width.
-  [[nodiscard]] std::optional<std::int32_t>
+  [[nodiscard]] std::optional<double>
   charstring_width(std::uint16_t glyph) const;
 
   std::string m_data;

src/odr/internal/font/type1_transform.cpp

@@ -7,6 +7,8 @@
 #include <cstddef>
 #include <stdexcept>
 #include <string>
+#include <string_view>
+#include <unordered_set>
 #include <utility>
 #include <vector>
 
@@ -19,7 +21,13 @@ std::string type1::to_cff(const Type1Font &font) {
   std::vector<cff::BuilderGlyph> glyphs;
   glyphs.reserve(font.glyphs().size() + 1);
 
+  // CFF glyph names must be unique; a repeated Type1 name keeps its first
+  // charstring.
+  std::unordered_set<std::string_view> names;
   const auto translate = [&](const Glyph &glyph) {
+    if (!names.insert(glyph.name).second) {
+      return;
+    }
     // A charstring that does not translate becomes an empty glyph, so one
     // broken glyph does not refuse the font.
     std::string charstring(1, static_cast<char>(14));
@@ -41,6 +49,7 @@ std::string type1::to_cff(const Type1Font &font) {
   if (notdef < font.glyphs().size()) {
     translate(font.glyphs()[notdef]);
   } else {
+    names.insert(".notdef");
     glyphs.push_back({".notdef", std::string(1, static_cast<char>(14))});
   }
   for (std::size_t i = 0; i < font.glyphs().size(); ++i) {

test/src/internal/font/cff_font.cpp

@@ -12,6 +12,7 @@
 #include <array>
 #include <clocale>
 #include <cstdint>
+#include <limits>
 #include <stdexcept>
 #include <string>
 #include <string_view>
@@ -620,3 +621,53 @@ TEST(CffFontTest, FontDictionarySelectionToleratesQuirkyRanges) {
   EXPECT_EQ(CffFont(font_bytes(std::string("\0\0\x01\0", 4))).advance_width(1),
             0);
 }
+
+TEST(CffFontTest, BuilderPreservesFractionalWidthsAndReusesStandardNames) {
+  const std::vector<BuilderGlyph> glyphs{
+      {".notdef", "\x0e"},
+      {"A", std::string("\xff\0\0\x80\0\x0e", 6)}}; // explicit width 0.5
+  const std::string bytes =
+      odr::internal::font::cff::build_cff("Widths", glyphs, 25.75, 100.75, {});
+  const CffFont font(bytes);
+  EXPECT_EQ(font.advance_width(0), 25);
+  EXPECT_EQ(font.advance_width(1), 101);
+  EXPECT_EQ(font.glyph_name(1), "A");
+  const std::size_t name_index_end = 4 + build_index({"Widths"}).size();
+  const auto offset_size = static_cast<std::uint8_t>(bytes[name_index_end + 2]);
+  const std::size_t top_dict_end =
+      name_index_end + 3 + 2 * offset_size - 1 +
+      bs::read_uint_be(
+          std::string_view(bytes).substr(name_index_end + 3 + offset_size),
+          offset_size);
+  EXPECT_EQ(bs::read_u16_be(std::string_view(bytes).substr(top_dict_end)),
+            0); // no custom names
+  for (const auto &[width, expected] :
+       std::array<std::pair<double, std::uint16_t>, 3>{
+           {{1e20, 65535}, {1e-20, 0}, {-0.5, 0}}}) {
+    const CffFont sized(
+        odr::internal::font::cff::build_cff("Widths", glyphs, width, 0, {}));
+    EXPECT_EQ(sized.advance_width(0), expected);
+  }
+  for (const double invalid : {std::numeric_limits<double>::infinity(),
+                               std::numeric_limits<double>::quiet_NaN()}) {
+    EXPECT_THROW((void)odr::internal::font::cff::build_cff("Widths", glyphs,
+                                                           invalid, 0, {}),
+                 std::runtime_error);
+  }
+}
+
+TEST(CffFontTest, BuilderRejectsInvalidGlyphSets) {
+  using odr::internal::font::cff::build_cff;
+  EXPECT_THROW((void)build_cff("Empty", {}, 0, 0, {}), std::runtime_error);
+  const std::vector<BuilderGlyph> missing{{"A", "\x0e"}};
+  EXPECT_THROW((void)build_cff("Missing", missing, 0, 0, {}),
+               std::runtime_error);
+  const std::vector<BuilderGlyph> duplicate{
+      {".notdef", "\x0e"}, {"A", "\x0e"}, {"A", "\x0e"}};
+  EXPECT_THROW((void)build_cff("Duplicate", duplicate, 0, 0, {}),
+               std::runtime_error);
+  std::vector<BuilderGlyph> too_many(65001);
+  too_many.front().name = ".notdef";
+  EXPECT_THROW((void)build_cff("TooMany", too_many, 0, 0, {}),
+               std::runtime_error);
+}

test/src/internal/font/type1_font.cpp

@@ -35,7 +35,8 @@ std::string charstring_entry(const std::string &name,
 /// Type1 fixture with two glyphs and one subroutine.
 std::string
 build_type1(const std::int32_t len_iv = 4, const std::string &prefix = "wxyz",
-            const std::string &glyph_b = std::string("\xf0\x0d\x0e", 3)) {
+            const std::string &glyph_b = std::string("\xf0\x0d\x0e", 3),
+            const std::string &name_b = "B") {
   const std::string clear = "%!PS-AdobeFont-1.0: TestType1 001.000\n"
                             "/FontName /TestType1 def\n"
                             "/FontMatrix [0.001 0 0 0.001 0 0] readonly def\n"
@@ -68,7 +69,7 @@ build_type1(const std::int32_t len_iv = 4, const std::string &prefix = "wxyz",
   // parser does not interpret them, it only extracts them.
   private_section +=
       charstring_entry("A", std::string("\x8b\x8b\x0d\x0e", 4), len_iv);
-  private_section += charstring_entry("B", glyph_b, len_iv);
+  private_section += charstring_entry(name_b, glyph_b, len_iv);
   private_section += "end\nend\n";
 
   std::string program = clear;
@@ -149,6 +150,14 @@ TEST(Type1FontTest, BrokenGlyphBecomesEmptyInCff) {
   EXPECT_EQ(cff.glyph_name(2), "B");
 }
 
+TEST(Type1FontTest, RepeatedGlyphNameKeepsTheFirstInCff) {
+  const std::string program =
+      build_type1(4, "wxyz", std::string("\xf0\x0d\x0e", 3), "A");
+  const odr::internal::font::cff::CffFont cff(to_cff(Type1Font{program}));
+  EXPECT_EQ(cff.glyph_count(), 2);
+  EXPECT_EQ(cff.glyph_name(1), "A");
+}
+
 TEST(Type1FontTest, ReadsInvalidNumbersAsZeroAndRejectsBadPfbSegments) {
   for (const std::string_view number : {"nan", "inf", "1oops"}) {
     std::string program = build_type1();