From c1e4ca43be6cb033bebb0d42f208bc3326baca99 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Mon, 5 Oct 2026 00:42:00 +0200 Subject: [PATCH 1/3] fix(font): preserve Type1 charstring widths and control flow --- CHANGELOG.md | 3 + src/odr/internal/font/type1_charstring.cpp | 130 +++++++++++++------- src/odr/internal/font/type1_charstring.hpp | 26 +--- src/odr/internal/font/type1_transform.cpp | 3 +- test/src/internal/font/type1_charstring.cpp | 77 +++++++++--- 5 files changed, 158 insertions(+), 81 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3afbdd6ab..e551e7e2d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,9 @@ The release run heads these entries with the version and opens a fresh ## Unreleased +- Type1 conversion honors subroutine returns, fractional widths and vertical + side bearings, and drops hint commands that Type2 cannot take in order. + - Type1 fonts avoid signed overflow when they decrypt, keep binary ciphertext bytes, and support unencrypted charstrings. diff --git a/src/odr/internal/font/type1_charstring.cpp b/src/odr/internal/font/type1_charstring.cpp index 43ae69b78..08e82a024 100644 --- a/src/odr/internal/font/type1_charstring.cpp +++ b/src/odr/internal/font/type1_charstring.cpp @@ -4,6 +4,7 @@ #include #include #include +#include #include #include #include @@ -78,7 +79,13 @@ void emit_num(std::string &out, const double v) { emit_int(out, static_cast(v)); return; } - const auto fixed = static_cast(std::lround(v * 65536.0)); + const double scaled = std::round(v * 65536.0); + if (!std::isfinite(scaled) || + scaled < std::numeric_limits::min() || + scaled > std::numeric_limits::max()) { + throw std::runtime_error("type1: operand exceeds Type2 fixed-point range"); + } + const auto fixed = static_cast(scaled); out += static_cast(255); out += static_cast((fixed >> 24) & 0xff); out += static_cast((fixed >> 16) & 0xff); @@ -86,6 +93,16 @@ void emit_num(std::string &out, const double v) { out += static_cast(fixed & 0xff); } +std::int32_t integer_operand(const double value) { + if (!std::isfinite(value) || + value < std::numeric_limits::min() || + value > std::numeric_limits::max() || + std::trunc(value) != value) { + throw std::runtime_error("type1: invalid integer operand"); + } + return static_cast(value); +} + /// The translation state machine. Walks the Type1 charstring (recursing through /// `callsubr`), emitting a Type2 charstring. class Translator { @@ -93,21 +110,29 @@ class Translator { explicit Translator(const std::span subrs) : m_subrs(subrs) {} - Type2Charstring run(const std::string_view charstring) { + std::string run(const std::string_view charstring) { execute(charstring, 0); if (!m_ended) { - m_out += static_cast(t1_endchar); + m_stack.clear(); + emit_op(t1_endchar); } - return {std::move(m_out), m_width, m_has_width}; + return std::move(m_out); } private: + void push(const double value) { + if (!std::isfinite(value) || m_stack.size() == 24) { + throw std::runtime_error("type1: invalid or overflowing operand stack"); + } + m_stack.push_back(value); + } + // Emit the pending width (once) ahead of the first stem/move/endchar's // operands, as the Type2 width does. nominalWidthX is 0 in the built CFF, so // the width is the absolute advance. void emit_width() { if (m_width_pending) { - emit_int(m_out, m_width); + emit_num(m_out, m_width); m_width_pending = false; } } @@ -127,7 +152,10 @@ class Translator { } void execute(const std::string_view cs, const std::int32_t depth) { - if (depth > 16 || m_ended) { + if (depth > 10) { + throw std::runtime_error("type1: subroutine nesting exceeds ten levels"); + } + if (m_ended) { return; } std::size_t p = 0; @@ -155,7 +183,7 @@ class Translator { byte_at(cs, p + 4)); p += 5; } - m_stack.push_back(value); + push(value); continue; } std::int32_t op = b; @@ -164,6 +192,9 @@ class Translator { op = 1200 + byte_at(cs, p); ++p; } + if (op == t1_return) { + return; + } handle(op, depth); } } @@ -173,20 +204,20 @@ class Translator { case t1_hsbw: if (m_stack.size() >= 2) { m_sbx = m_stack[0]; - m_width = static_cast(m_stack[1]); - m_has_width = true; + m_width = m_stack[1]; + m_sby = 0; m_width_pending = true; - m_sbx_pending = true; + m_bearing_pending = true; } m_stack.clear(); break; case t1_sbw: if (m_stack.size() >= 4) { m_sbx = m_stack[0]; - m_width = static_cast(m_stack[2]); - m_has_width = true; + m_width = m_stack[2]; + m_sby = m_stack[1]; m_width_pending = true; - m_sbx_pending = true; + m_bearing_pending = true; } m_stack.clear(); break; @@ -195,9 +226,10 @@ class Translator { if (m_flex_active) { collect_flex_point(); } else { - if (m_sbx_pending && !m_stack.empty()) { + if (m_bearing_pending && m_stack.size() >= 2) { m_stack[0] += m_sbx; - m_sbx_pending = false; + m_stack[1] += m_sby; + m_bearing_pending = false; } emit_op(t1_rmoveto); } @@ -206,32 +238,38 @@ class Translator { if (m_flex_active) { collect_flex_point(); } else { - // hmoveto has no y; the side bearing adds an x, so keep it hmoveto. - if (m_sbx_pending && !m_stack.empty()) { - m_stack[0] += m_sbx; + const double dx = m_stack.empty() ? 0 : m_stack[0]; + if (m_bearing_pending && m_sby != 0) { + m_stack = {dx + m_sbx, m_sby}; + emit_op(t1_rmoveto); + } else { + if (m_bearing_pending && !m_stack.empty()) { + m_stack[0] += m_sbx; + } + emit_op(t1_hmoveto); } - m_sbx_pending = false; - emit_op(t1_hmoveto); + m_bearing_pending = false; } break; case t1_vmoveto: if (m_flex_active) { collect_flex_point(); - } else if (m_sbx_pending && m_sbx != 0.0) { + } else if (m_bearing_pending && m_sbx != 0.0) { // A side bearing adds an x offset, which vmoveto cannot carry: promote // to rmoveto(sbx, dy). const double dy = m_stack.empty() ? 0.0 : m_stack[0]; - m_stack = {m_sbx, dy}; - m_sbx_pending = false; + m_stack = {m_sbx, dy + m_sby}; + m_bearing_pending = false; emit_op(t1_rmoveto); } else { - m_sbx_pending = false; + if (m_bearing_pending && !m_stack.empty()) { + m_stack[0] += m_sby; + } + m_bearing_pending = false; emit_op(t1_vmoveto); } break; - case t1_hstem: - case t1_vstem: case t1_rlineto: case t1_hlineto: case t1_vlineto: @@ -242,6 +280,8 @@ class Translator { break; case t1_closepath: + case t1_hstem: + case t1_vstem: case t1_dotsection: case t1_vstem3: case t1_hstem3: @@ -255,7 +295,10 @@ class Translator { m_stack.pop_back(); const double a = m_stack.back(); m_stack.pop_back(); - m_stack.push_back(b != 0.0 ? a / b : 0.0); + if (b == 0.0) { + throw std::runtime_error("type1: division by zero"); + } + push(a / b); } break; @@ -263,15 +306,14 @@ class Translator { if (m_stack.empty()) { break; } - const auto index = static_cast(m_stack.back()); + const auto index = integer_operand(m_stack.back()); m_stack.pop_back(); - if (index >= 0 && index < static_cast(m_subrs.size())) { - execute(m_subrs[index], depth + 1); + if (index < 0 || static_cast(index) >= m_subrs.size()) { + throw std::runtime_error("type1: invalid subroutine index"); } + execute(m_subrs[index], depth + 1); break; } - case t1_return: - break; // end of the current subr case t1_callothersubr: handle_othersubr(); @@ -279,10 +321,10 @@ class Translator { case t1_pop: // Push the value the matching callothersubr left on the PS stack. if (!m_ps_stack.empty()) { - m_stack.push_back(m_ps_stack.back()); + push(m_ps_stack.back()); m_ps_stack.pop_back(); } else { - m_stack.push_back(0.0); + push(0.0); } break; @@ -310,13 +352,16 @@ class Translator { m_stack.clear(); return; } - const auto othersubr = static_cast(m_stack.back()); + const auto othersubr = integer_operand(m_stack.back()); m_stack.pop_back(); - const auto argc = static_cast(m_stack.back()); + const auto argc = integer_operand(m_stack.back()); m_stack.pop_back(); + if (argc < 0 || static_cast(argc) > m_stack.size()) { + throw std::runtime_error("type1: invalid OtherSubr argument count"); + } std::vector args; - for (std::int32_t i = 0; i < argc && !m_stack.empty(); ++i) { + for (std::int32_t i = 0; i < argc; ++i) { args.push_back(m_stack.back()); m_stack.pop_back(); } @@ -417,11 +462,11 @@ class Translator { std::vector m_stack; std::vector m_ps_stack; - std::int32_t m_width{}; - bool m_has_width{}; + double m_width{}; bool m_width_pending{}; double m_sbx{}; - bool m_sbx_pending{}; + double m_sby{}; + bool m_bearing_pending{}; bool m_ended{}; bool m_flex_active{}; @@ -434,9 +479,8 @@ class Translator { namespace odr::internal::font { -type1::Type2Charstring -type1::to_type2(const std::string_view type1, - const std::span subrs) { +std::string type1::to_type2(const std::string_view type1, + const std::span subrs) { return Translator(subrs).run(type1); } diff --git a/src/odr/internal/font/type1_charstring.hpp b/src/odr/internal/font/type1_charstring.hpp index b54aad770..bb925ee30 100644 --- a/src/odr/internal/font/type1_charstring.hpp +++ b/src/odr/internal/font/type1_charstring.hpp @@ -1,32 +1,14 @@ #pragma once -#include #include #include #include namespace odr::internal::font::type1 { -/// The result of translating a Type1 charstring to Type2 (CFF). -struct Type2Charstring { - std::string charstring; ///< the Type2 charstring (no leading width) - std::int32_t width{}; ///< advance width from `hsbw`/`sbw`, in glyph units - bool has_width{}; ///< whether an `hsbw`/`sbw` set the width -}; - -/// Translate one **decrypted** Type1 charstring to a Type2 (CFF) charstring. -/// -/// Type1 and Type2 share most path operators; this flattens `callsubr` -/// (inlining @p subrs), folds `div`, lifts the `hsbw` side bearing into the -/// first move, drops Type1-only hint operators (`dotsection`, `*stem3`, hint -/// replacement) and translates the flex and `seac` OtherSubr mechanisms. The -/// advance width (`hsbw`) is returned separately rather than baked into the -/// charstring, so the caller emits it against the CFF `nominalWidthX`. -/// -/// Best-effort and display-oriented: hints are dropped (they affect rendering -/// quality, not glyph shape), and unknown operators are skipped. Throws -/// `std::runtime_error` on a charstring that ends mid-operand. -[[nodiscard]] Type2Charstring to_type2(std::string_view type1, - std::span subrs); +/// Convert Type1 outlines to Type2, flattening subroutines and dropping hints. +/// Preserves the width; throws on invalid operands or excessive recursion. +[[nodiscard]] std::string to_type2(std::string_view type1, + std::span subrs); } // namespace odr::internal::font::type1 diff --git a/src/odr/internal/font/type1_transform.cpp b/src/odr/internal/font/type1_transform.cpp index a519d0f39..1a4ce321b 100644 --- a/src/odr/internal/font/type1_transform.cpp +++ b/src/odr/internal/font/type1_transform.cpp @@ -19,8 +19,7 @@ std::string type1::to_cff(const Type1Font &font) { glyphs.reserve(font.glyphs().size() + 1); const auto translate = [&](const Glyph &glyph) { - Type2Charstring t2 = to_type2(glyph.charstring, font.subrs()); - glyphs.push_back({glyph.name, std::move(t2.charstring)}); + glyphs.push_back({glyph.name, to_type2(glyph.charstring, font.subrs())}); }; // .notdef first. diff --git a/test/src/internal/font/type1_charstring.cpp b/test/src/internal/font/type1_charstring.cpp index 840343242..677fa946d 100644 --- a/test/src/internal/font/type1_charstring.cpp +++ b/test/src/internal/font/type1_charstring.cpp @@ -3,6 +3,7 @@ #include #include +#include #include #include @@ -12,21 +13,21 @@ namespace { /// Encode an integer in the Type1/Type2 shared number forms (no 28/255 needed /// for the small values used here). -void num(std::string &s, const int v) { +void num(std::string &s, const std::int32_t v) { if (v >= -107 && v <= 107) { s += static_cast(v + 139); } else if (v >= 108 && v <= 1131) { - const int u = v - 108; + const std::int32_t u = v - 108; s += static_cast((u >> 8) + 247); s += static_cast(u & 0xff); } else if (v >= -1131 && v <= -108) { - const int u = -v - 108; + const std::int32_t u = -v - 108; s += static_cast((u >> 8) + 251); s += static_cast(u & 0xff); } } -void op(std::string &s, const int o) { s += static_cast(o); } +void op(std::string &s, const std::int32_t o) { s += static_cast(o); } } // namespace @@ -44,9 +45,7 @@ TEST(Type1CharstringTest, HsbwWidthAndSideBearing) { op(t1, 5); // rlineto op(t1, 14); // endchar - const Type2Charstring out = to_type2(t1, {}); - EXPECT_TRUE(out.has_width); - EXPECT_EQ(out.width, 200); + const std::string out = to_type2(t1, {}); // Type2: [width 200][dx 100+sbx 10 = 110][dy 0] rmoveto [50][50] rlineto // endchar. @@ -59,7 +58,7 @@ TEST(Type1CharstringTest, HsbwWidthAndSideBearing) { num(expected, 50); op(expected, 5); // rlineto op(expected, 14); // endchar - EXPECT_EQ(out.charstring, expected); + EXPECT_EQ(out, expected); } TEST(Type1CharstringTest, FlattensCallSubr) { @@ -67,8 +66,9 @@ TEST(Type1CharstringTest, FlattensCallSubr) { std::string subr0; num(subr0, 50); num(subr0, 50); - op(subr0, 5); // rlineto - op(subr0, 11); // return + op(subr0, 5); // rlineto + op(subr0, 11); // return + op(subr0, 255); // Unreachable truncated operand. // 0 0 hsbw 0 0 rmoveto 0 callsubr endchar std::string t1; @@ -83,7 +83,7 @@ TEST(Type1CharstringTest, FlattensCallSubr) { op(t1, 14); // endchar const std::array subrs = {subr0}; - const Type2Charstring out = to_type2(t1, subrs); + const std::string out = to_type2(t1, subrs); // The subr's rlineto is inlined; expect width(0) rmoveto, then rlineto, then // endchar. @@ -96,7 +96,7 @@ TEST(Type1CharstringTest, FlattensCallSubr) { num(expected, 50); op(expected, 5); // rlineto (from subr) op(expected, 14); // endchar - EXPECT_EQ(out.charstring, expected); + EXPECT_EQ(out, expected); } TEST(Type1CharstringTest, FoldsDiv) { @@ -113,12 +113,61 @@ TEST(Type1CharstringTest, FoldsDiv) { op(t1, 21); // rmoveto op(t1, 14); // endchar - const Type2Charstring out = to_type2(t1, {}); + const std::string out = to_type2(t1, {}); std::string expected; num(expected, 0); // width num(expected, 300); // 600 / 2 num(expected, 0); op(expected, 21); // rmoveto op(expected, 14); // endchar - EXPECT_EQ(out.charstring, expected); + EXPECT_EQ(out, expected); +} + +TEST(Type1CharstringTest, PreservesFractionalWidthAndBothSideBearings) { + for (const std::int32_t move : {21, 22, 4}) { + std::string input; + for (const std::int32_t value : {10, 20, 201, 2}) { + num(input, value); + } + input += std::string("\x0c\x0c", 2); // width = 201 / 2 + num(input, 0); + input += std::string("\x0c\x07", 2); // sbw + num(input, 30); + if (move == 21) { + num(input, 40); + } + op(input, move); + num(input, 5); + num(input, 10); + op(input, + 1); // A Type1 stem after a path cannot be copied as a Type2 hstem. + op(input, 14); + std::string expected("\xff\0\x64\x80\0", 5); // 100.5 in 16.16 + num(expected, move == 4 ? 10 : 40); + num(expected, move == 4 ? 50 : move == 22 ? 20 : 60); + op(expected, 21); + op(expected, 14); + EXPECT_EQ(to_type2(input, {}), expected); + } + std::string incomplete; + num(incomplete, 0); + num(incomplete, 200); + op(incomplete, 13); + std::string expected; + num(expected, 200); + op(expected, 14); + EXPECT_EQ(to_type2(incomplete, {}), expected); +} + +TEST(Type1CharstringTest, RejectsInvalidArithmeticStacksAndSubroutines) { + EXPECT_THROW((void)to_type2(std::string(25, static_cast(139)), {}), + std::runtime_error); + EXPECT_THROW((void)to_type2(std::string("\x8c\x8b\x0c\x0c", 4), {}), + std::runtime_error); + EXPECT_THROW((void)to_type2(std::string("\xff\0\0\x9c\x40\x16", 6), {}), + std::runtime_error); + const std::array subrs{std::string("\x8b\x0a", 2)}; + EXPECT_THROW((void)to_type2(subrs[0], subrs), std::runtime_error); + EXPECT_THROW((void)to_type2(std::string("\x8c\x8d\x0c\x0c\x0a", 5), subrs), + std::runtime_error); } From 87186bf140e2c98e9253e12bb32a9bc619adf284 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Mon, 5 Oct 2026 14:44:24 +0200 Subject: [PATCH 2/3] fix(font): keep a broken Type1 glyph from refusing the font to_type2 now throws on a division by zero, an invalid subroutine index or an overflowing stack, and to_cff let the error refuse the whole font. A PDF then falls back to a substitute font and shows the wrong glyphs. to_cff now turns a charstring that does not translate into an empty glyph, and the other glyphs stay. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01Gb1fLafqqzehpqPuU6uBfn --- src/odr/internal/font/type1_transform.cpp | 10 +++++++++- test/src/internal/font/type1_font.cpp | 17 +++++++++++++---- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/src/odr/internal/font/type1_transform.cpp b/src/odr/internal/font/type1_transform.cpp index 1a4ce321b..3df2e2952 100644 --- a/src/odr/internal/font/type1_transform.cpp +++ b/src/odr/internal/font/type1_transform.cpp @@ -5,6 +5,7 @@ #include #include +#include #include #include #include @@ -19,7 +20,14 @@ std::string type1::to_cff(const Type1Font &font) { glyphs.reserve(font.glyphs().size() + 1); const auto translate = [&](const Glyph &glyph) { - glyphs.push_back({glyph.name, to_type2(glyph.charstring, font.subrs())}); + // 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(14)); + try { + charstring = to_type2(glyph.charstring, font.subrs()); + } catch (const std::runtime_error &) { + } + glyphs.push_back({glyph.name, std::move(charstring)}); }; // .notdef first. diff --git a/test/src/internal/font/type1_font.cpp b/test/src/internal/font/type1_font.cpp index 05553a8d3..db9ea44a8 100644 --- a/test/src/internal/font/type1_font.cpp +++ b/test/src/internal/font/type1_font.cpp @@ -33,8 +33,9 @@ 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") { +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 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" @@ -67,8 +68,7 @@ std::string build_type1(const std::int32_t len_iv = 4, // 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", std::string("\xf0\x0d\x0e", 3), len_iv); + private_section += charstring_entry("B", glyph_b, len_iv); private_section += "end\nend\n"; std::string program = clear; @@ -140,6 +140,15 @@ TEST(Type1FontTest, ConvertsToLoadableCff) { EXPECT_TRUE(sfnt::SfntFont::is_sfnt(cff::wrap_to_otf(font))); } +TEST(Type1FontTest, BrokenGlyphBecomesEmptyInCff) { + // 1 0 div: the translation throws on the division by zero. + const Type1Font font{ + build_type1(4, "wxyz", std::string("\x8c\x8b\x0c\x0c\x0e", 5))}; + const odr::internal::font::cff::CffFont cff(to_cff(font)); + EXPECT_EQ(cff.glyph_count(), 3); + EXPECT_EQ(cff.glyph_name(2), "B"); +} + TEST(Type1FontTest, ReadsInvalidNumbersAsZeroAndRejectsBadPfbSegments) { for (const std::string_view number : {"nan", "inf", "1oops"}) { std::string program = build_type1(); From 21d9ff0e289c31f57a19be8f5e7712e7d5fbd611 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Mon, 5 Oct 2026 14:54:58 +0200 Subject: [PATCH 3/3] refactor(font): share the checked double to integer conversion Type1 and CFF each checked by hand that a double is a finite whole value in the range of an integer type. util::number::to_integer now does this, and the three call sites throw their own errors on an empty result. The bound is 2^digits, which is exact as a double, so the check is also correct for 64-bit types, where max() rounds up to 2^63. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01Gb1fLafqqzehpqPuU6uBfn --- src/odr/internal/font/cff_font.cpp | 9 +++---- src/odr/internal/font/type1_charstring.cpp | 22 ++++++++-------- src/odr/internal/util/number_util.hpp | 17 +++++++++++++ test/src/internal/util/number_util_test.cpp | 28 +++++++++++++++++++++ 4 files changed, 60 insertions(+), 16 deletions(-) diff --git a/src/odr/internal/font/cff_font.cpp b/src/odr/internal/font/cff_font.cpp index 72853b4b4..077782690 100644 --- a/src/odr/internal/font/cff_font.cpp +++ b/src/odr/internal/font/cff_font.cpp @@ -9,7 +9,6 @@ #include #include #include -#include #include #include #include @@ -109,12 +108,12 @@ std::string_view checked_slice(const std::string_view bytes, } std::uint32_t dict_offset(const double value) { - if (!std::isfinite(value) || value < 0 || - value > std::numeric_limits::max() || - std::trunc(value) != value) { + const std::optional result = + util::number::to_integer(value); + if (!result) { throw std::runtime_error("cff: invalid DICT offset or length"); } - return static_cast(value); + return *result; } /// A parsed DICT: operator -> operands. Reals are decoded to double; integers diff --git a/src/odr/internal/font/type1_charstring.cpp b/src/odr/internal/font/type1_charstring.cpp index 08e82a024..e6c0a4a55 100644 --- a/src/odr/internal/font/type1_charstring.cpp +++ b/src/odr/internal/font/type1_charstring.cpp @@ -1,10 +1,12 @@ #include +#include + #include #include #include #include -#include +#include #include #include #include @@ -79,13 +81,12 @@ void emit_num(std::string &out, const double v) { emit_int(out, static_cast(v)); return; } - const double scaled = std::round(v * 65536.0); - if (!std::isfinite(scaled) || - scaled < std::numeric_limits::min() || - scaled > std::numeric_limits::max()) { + const std::optional scaled = + util::number::to_integer(std::round(v * 65536.0)); + if (!scaled) { throw std::runtime_error("type1: operand exceeds Type2 fixed-point range"); } - const auto fixed = static_cast(scaled); + const std::int32_t fixed = *scaled; out += static_cast(255); out += static_cast((fixed >> 24) & 0xff); out += static_cast((fixed >> 16) & 0xff); @@ -94,13 +95,12 @@ void emit_num(std::string &out, const double v) { } std::int32_t integer_operand(const double value) { - if (!std::isfinite(value) || - value < std::numeric_limits::min() || - value > std::numeric_limits::max() || - std::trunc(value) != value) { + const std::optional result = + util::number::to_integer(value); + if (!result) { throw std::runtime_error("type1: invalid integer operand"); } - return static_cast(value); + return *result; } /// The translation state machine. Walks the Type1 charstring (recursing through diff --git a/src/odr/internal/util/number_util.hpp b/src/odr/internal/util/number_util.hpp index 9bdf2495e..96f8219a0 100644 --- a/src/odr/internal/util/number_util.hpp +++ b/src/odr/internal/util/number_util.hpp @@ -1,9 +1,13 @@ #pragma once +#include +#include #include +#include #include #include #include +#include namespace odr::internal::util::number { @@ -17,4 +21,17 @@ namespace odr::internal::util::number { std::string to_string_significant(double value, std::int32_t significant_digits); +/// @p value as a `T`, or nothing if it is not finite, has a fraction or does +/// not fit. +template +[[nodiscard]] std::optional to_integer(const double value) { + // 2^digits is exact as a double, where `max()` may round up for 64 bits. + const double limit = std::ldexp(1.0, std::numeric_limits::digits); + if (!std::isfinite(value) || std::trunc(value) != value || value >= limit || + value < (std::is_signed_v ? -limit : 0.0)) { + return std::nullopt; + } + return static_cast(value); +} + } // namespace odr::internal::util::number diff --git a/test/src/internal/util/number_util_test.cpp b/test/src/internal/util/number_util_test.cpp index 5ed38f712..3683fe6f1 100644 --- a/test/src/internal/util/number_util_test.cpp +++ b/test/src/internal/util/number_util_test.cpp @@ -1,6 +1,7 @@ #include #include +#include #include #include @@ -73,3 +74,30 @@ TEST(ToStringSignificant, bounds_extreme_precision) { to_string_significant(1234, std::numeric_limits::min()), "1234"); } + +TEST(ToInteger, takes_only_whole_finite_values) { + EXPECT_EQ(to_integer(-12.0), std::optional(-12)); + EXPECT_EQ(to_integer(1.5), std::nullopt); + EXPECT_EQ(to_integer(std::nan("")), std::nullopt); + EXPECT_EQ(to_integer(std::numeric_limits::infinity()), + std::nullopt); +} + +TEST(ToInteger, takes_exactly_the_range_of_the_type) { + EXPECT_EQ(to_integer(-2147483648.0), + std::numeric_limits::min()); + EXPECT_EQ(to_integer(2147483647.0), + std::numeric_limits::max()); + EXPECT_EQ(to_integer(-2147483649.0), std::nullopt); + EXPECT_EQ(to_integer(2147483648.0), std::nullopt); + + EXPECT_EQ(to_integer(4294967295.0), + std::numeric_limits::max()); + EXPECT_EQ(to_integer(4294967296.0), std::nullopt); + EXPECT_EQ(to_integer(-1.0), std::nullopt); + + // 2^63 is where `max()` rounds to as a double, so a `max()` bound takes it. + EXPECT_EQ(to_integer(std::ldexp(1.0, 63)), std::nullopt); + EXPECT_EQ(to_integer(-std::ldexp(1.0, 63)), + std::numeric_limits::min()); +}