From d74529a3833c1c6243e44660ba49bc6ed6a4c7e2 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Mon, 5 Oct 2026 00:35:22 +0200 Subject: [PATCH 1/3] fix(font): validate Type1 decoding and cipher arithmetic --- CHANGELOG.md | 3 + src/odr/internal/font/type1_crypt.cpp | 15 +-- src/odr/internal/font/type1_crypt.hpp | 13 +-- src/odr/internal/font/type1_font.cpp | 57 ++++++---- src/odr/internal/font/type1_font.hpp | 13 +-- test/src/internal/font/type1_crypt.cpp | 21 +--- test/src/internal/font/type1_font.cpp | 125 ++++++++++++--------- test/src/internal/font/type1_test_util.hpp | 23 ++++ 8 files changed, 150 insertions(+), 120 deletions(-) create mode 100644 test/src/internal/font/type1_test_util.hpp diff --git a/CHANGELOG.md b/CHANGELOG.md index 070225dee..3afbdd6ab 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 fonts avoid signed overflow when they decrypt, keep binary ciphertext + bytes, and support unencrypted charstrings. + - CFF fonts bound INDEX, dictionary and glyph reads, and check offsets before conversion. Real operands ignore the host locale. diff --git a/src/odr/internal/font/type1_crypt.cpp b/src/odr/internal/font/type1_crypt.cpp index 4a31b73c4..a77382322 100644 --- a/src/odr/internal/font/type1_crypt.cpp +++ b/src/odr/internal/font/type1_crypt.cpp @@ -1,7 +1,7 @@ #include -#include #include +#include #include namespace odr::internal::font::type1 { @@ -12,7 +12,8 @@ constexpr std::uint16_t c1 = 52845; constexpr std::uint16_t c2 = 22719; [[nodiscard]] bool is_hex_digit(const std::uint8_t c) { - return std::isxdigit(c) != 0; + return (c >= '0' && c <= '9') || (c >= 'A' && c <= 'F') || + (c >= 'a' && c <= 'f'); } /// Hex-decode @p in, skipping whitespace; stops at the first non-hex, non-space @@ -22,7 +23,7 @@ constexpr std::uint16_t c2 = 22719; std::int32_t high = -1; for (const char ch : in) { const auto c = static_cast(ch); - if (std::isspace(c) != 0) { + if (std::string_view(" \t\r\n").contains(ch)) { continue; } if (!is_hex_digit(c)) { @@ -47,7 +48,7 @@ constexpr std::uint16_t c2 = 22719; std::int32_t seen = 0; for (const char ch : eexec) { const auto c = static_cast(ch); - if (std::isspace(c) != 0) { + if (std::string_view(" \t\r\n").contains(ch)) { continue; } if (!is_hex_digit(c)) { @@ -74,10 +75,10 @@ std::string type1::decrypt(const std::string_view cipher, for (const char ch : cipher) { const auto c = static_cast(ch); out += static_cast(c ^ (r >> 8)); - r = static_cast((c + r) * c1 + c2); + r = static_cast((std::uint32_t{c} + r) * c1 + c2); } - if (skip >= out.size()) { - return {}; + if (skip > out.size()) { + throw std::runtime_error("type1: truncated encryption prefix"); } return out.substr(skip); } diff --git a/src/odr/internal/font/type1_crypt.hpp b/src/odr/internal/font/type1_crypt.hpp index 779f9b3e4..4b13a2bcb 100644 --- a/src/odr/internal/font/type1_crypt.hpp +++ b/src/odr/internal/font/type1_crypt.hpp @@ -7,23 +7,12 @@ namespace odr::internal::font::type1 { -// Type1 (Adobe Type 1 Font Format) `eexec` / charstring decryption. -// -// Both the `eexec`-encrypted portion of the font program and each individual -// charstring use the same stream cipher with different keys: a 16-bit running -// key `R`, constants c1 = 52845 / c2 = 22719, where each plaintext byte is -// `cipher ^ (R >> 8)` and `R = (cipher + R) * c1 + c2` (mod 2^16). The leading -// @p skip bytes of plaintext are random padding and discarded. - /// Decrypt @p cipher with the running-key cipher seeded at @p key, discarding /// the first @p skip plaintext bytes. [[nodiscard]] std::string decrypt(std::string_view cipher, std::uint16_t key, std::size_t skip); -/// Decrypt the `eexec` section (key 55665, 4 random bytes). Accepts either the -/// binary form (PDF `/FontFile`, the `/Length2` portion) or the ASCII-hex form -/// (PFA fonts): when the section's leading bytes are all hex digits/whitespace -/// it is hex-decoded first. +/// Decrypt binary or ASCII-hex eexec data (Adobe Type 1 Font Format 7.2). [[nodiscard]] std::string decrypt_eexec(std::string_view eexec); /// Decrypt one charstring (key 4330), discarding @p len_iv leading bytes diff --git a/src/odr/internal/font/type1_font.cpp b/src/odr/internal/font/type1_font.cpp index 8b02b6320..e8591489e 100644 --- a/src/odr/internal/font/type1_font.cpp +++ b/src/odr/internal/font/type1_font.cpp @@ -2,9 +2,11 @@ #include #include +#include #include #include +#include #include #include #include @@ -48,13 +50,11 @@ namespace { } [[nodiscard]] double parse_double(const std::string_view token) { - // std::from_chars for double is not universally available; std::stod needs a - // null-terminated copy. - try { - return std::stod(std::string(token)); - } catch (const std::exception &) { - return 0.0; + const std::optional value = util::number::parse(token); + if (!value || !std::isfinite(*value)) { + throw std::runtime_error("type1: invalid numeric operand"); } + return *value; } /// A `/FontBBox` number as an FWord. Clamping keeps an out-of-range number out @@ -71,7 +71,7 @@ parse_number_array(const std::string_view s, const std::string_view key) { if (k == std::string_view::npos) { return out; } - std::size_t open = s.find_first_of("[{", k); + const std::size_t open = s.find_first_of("[{", k); if (open == std::string_view::npos) { return out; } @@ -111,7 +111,7 @@ read_rd_binary(const std::string_view s, std::size_t &p) { return std::nullopt; } ++q; // the single delimiter space - if (q + static_cast(length) > s.size()) { + if (static_cast(length) > s.size() - q) { return std::nullopt; } const std::string_view bytes = s.substr(q, static_cast(length)); @@ -135,16 +135,22 @@ Type1Font::Type1Font(std::string_view data) { std::string unframed; if (!data.empty() && static_cast(data[0]) == 0x80) { std::size_t p = 0; - while (p + 6 <= data.size() && static_cast(data[p]) == 0x80) { + while (p < data.size()) { + if (data.size() - p < 2 || static_cast(data[p]) != 0x80) { + throw std::runtime_error("type1: invalid PFB segment"); + } const std::uint8_t type = static_cast(data[p + 1]); if (type == 3) { break; } + if ((type != 1 && type != 2) || data.size() - p < 6) { + throw std::runtime_error("type1: invalid PFB segment header"); + } const auto len = util::byte::from_little_endian(data.substr(p + 2, 4)); p += 6; - if (p + len > data.size()) { - break; + if (len > data.size() - p) { + throw std::runtime_error("type1: truncated PFB segment"); } unframed.append(data.substr(p, len)); p += len; @@ -160,7 +166,11 @@ Type1Font::Type1Font(std::string_view data) { parse_clear(data.substr(0, eexec)); // The encrypted blob begins after `eexec` and its trailing whitespace. - const std::size_t blob = skip_space(data, eexec + 5); + std::size_t blob = eexec + 5; + while (blob < data.size() && + std::string_view(" \t\r\n").contains(data[blob])) { + ++blob; + } const std::string decrypted = decrypt_eexec(data.substr(blob)); parse_private(decrypted); @@ -218,18 +228,19 @@ void Type1Font::parse_clear(const std::string_view clear) { } void Type1Font::parse_private(const std::string_view decrypted) { - std::size_t len_iv = 4; + std::int32_t len_iv = 4; if (const std::size_t k = decrypted.find("/lenIV"); k != std::string_view::npos) { std::size_t p = k + 6; - std::int32_t value = 0; - if (parse_int(read_token(decrypted, p), value)) { - if (value < 0) { - throw std::runtime_error("type1: negative /lenIV"); - } - len_iv = static_cast(value); + if (!parse_int(read_token(decrypted, p), len_iv) || len_iv < -1) { + throw std::runtime_error("type1: invalid /lenIV"); } } + const auto decode = [len_iv](const std::string_view bytes) { + return len_iv == -1 + ? std::string(bytes) + : decrypt_charstring(bytes, static_cast(len_iv)); + }; // /CharStrings starts where /Subrs ends (Subrs precede it). const std::size_t cs = decrypted.find("/CharStrings"); @@ -257,10 +268,10 @@ void Type1Font::parse_private(const std::string_view decrypted) { p += 4; continue; } - if (static_cast(m_subrs.size()) <= index) { - m_subrs.resize(index + 1); + if (m_subrs.size() <= static_cast(index)) { + m_subrs.resize(static_cast(index) + 1); } - m_subrs[index] = decrypt_charstring(*bytes, len_iv); + m_subrs[index] = decode(*bytes); p = q; } } @@ -284,7 +295,7 @@ void Type1Font::parse_private(const std::string_view decrypted) { p = slash + 1; continue; } - m_glyphs.push_back({std::move(name), decrypt_charstring(*bytes, len_iv)}); + m_glyphs.push_back({std::move(name), decode(*bytes)}); p = q; } } diff --git a/src/odr/internal/font/type1_font.hpp b/src/odr/internal/font/type1_font.hpp index 6abac2de0..623b0bc68 100644 --- a/src/odr/internal/font/type1_font.hpp +++ b/src/odr/internal/font/type1_font.hpp @@ -18,17 +18,8 @@ struct Glyph { std::string charstring; }; -/// @brief Parses an Adobe Type1 font program into its decrypted parts. -/// -/// A Type1 program has three sections: a clear-text header (font dictionary up -/// to `eexec`), an `eexec`-encrypted private portion (`/Subrs`, -/// `/CharStrings`), and a zero-padded trailer. This reads `/FontMatrix`, -/// `/FontBBox`, `/Encoding` and `/FontName` from the header, decrypts the -/// `eexec` section (`type1_crypt`) and keeps every glyph's decrypted -/// charstring plus the `/Subrs`; interpreting them is `type1_charstring`. -/// -/// Throws `std::runtime_error` when the program has no `eexec` section or no -/// `/CharStrings`. +/// Parse Type1 metadata and decrypt its charstrings and subroutines. +/// Throws on invalid numbers, framing, or missing eexec/CharStrings data. class Type1Font { public: /// Cheap magic test: the PostScript font sentinel (`%!PS-AdobeFont`, diff --git a/test/src/internal/font/type1_crypt.cpp b/test/src/internal/font/type1_crypt.cpp index 7bfbd4d84..293444a3b 100644 --- a/test/src/internal/font/type1_crypt.cpp +++ b/test/src/internal/font/type1_crypt.cpp @@ -1,3 +1,5 @@ +#include "type1_test_util.hpp" + #include #include @@ -9,23 +11,7 @@ using namespace odr::internal::font::type1; namespace { -/// Independent reference implementation of the Type1 *encryption* (the inverse -/// of `decrypt`), so the round-trip tests are not circular: this codes the -/// cipher forwards (plaintext -> ciphertext), `decrypt` codes it backwards. -std::string encrypt(const std::string &plain, std::uint16_t r, - const std::string &random_prefix) { - constexpr std::uint16_t c1 = 52845; - constexpr std::uint16_t c2 = 22719; - std::string out; - const std::string full = random_prefix + plain; - for (const char ch : full) { - const auto p = static_cast(ch); - const auto cipher = static_cast(p ^ (r >> 8)); - out += static_cast(cipher); - r = static_cast((cipher + r) * c1 + c2); - } - return out; -} +using odr::test::font::encrypt; } // namespace @@ -45,6 +31,7 @@ TEST(Type1CryptTest, CharstringHonoursLenIv) { const std::string plain = "hello"; const std::string cipher = encrypt(plain, 4330, ""); EXPECT_EQ(decrypt_charstring(cipher, 0), plain); + EXPECT_THROW((void)decrypt_charstring("x", 4), std::runtime_error); } TEST(Type1CryptTest, EexecAcceptsHexForm) { diff --git a/test/src/internal/font/type1_font.cpp b/test/src/internal/font/type1_font.cpp index 22155762a..f2a7023e5 100644 --- a/test/src/internal/font/type1_font.cpp +++ b/test/src/internal/font/type1_font.cpp @@ -1,12 +1,16 @@ +#include "type1_test_util.hpp" + #include #include #include #include #include +#include #include +#include #include #include @@ -14,67 +18,61 @@ using namespace odr::internal::font::type1; namespace { -/// Forward Type1 cipher (the inverse of `decrypt`), so the test builds a real -/// encrypted program rather than trusting the decryptor. -std::string encrypt(const std::string &plain, std::uint16_t r, - const std::string &random_prefix) { - constexpr std::uint16_t c1 = 52845; - constexpr std::uint16_t c2 = 22719; - std::string out; - for (const char ch : random_prefix + plain) { - const auto p = static_cast(ch); - const auto cipher = static_cast(p ^ (r >> 8)); - out += static_cast(cipher); - r = static_cast((cipher + r) * c1 + c2); - } - return out; -} +using odr::test::font::encrypt; -/// A `/name len RD ND` charstring entry, the charstring encrypted with -/// the charstring key (4330) and a 4-byte lenIV prefix. +/// A charstring entry encoded according to /lenIV. std::string charstring_entry(const std::string &name, - const std::string &plain_charstring) { - // The 4-byte lenIV prefix must be a real 4 NUL bytes — a "\x00\x00\x00\x00" - // string literal would be empty (the first NUL terminates it). - const std::string enc = encrypt(plain_charstring, 4330, std::string(4, '\0')); + const std::string &plain_charstring, + const std::int32_t len_iv) { + const std::string enc = + len_iv == -1 + ? plain_charstring + : encrypt(plain_charstring, 4330, + std::string(static_cast(len_iv), '\0')); return "/" + name + " " + std::to_string(enc.size()) + " RD " + enc + " ND\n"; } -/// Assemble a minimal but well-formed Type1 program: a clear header (with a -/// custom /Encoding and /FontMatrix) and an eexec-encrypted private section -/// holding two glyphs and one subr. -std::string build_type1() { - 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" - "/FontBBox {0 -200 700 800} readonly def\n" - "/Encoding 256 array\n" - "0 1 255 {1 index exch /.notdef put} for\n" - "dup 65 /A put\n" - "dup 66 /B put\n" - "readonly def\n" - "currentdict end\n" - "currentfile eexec\n"; +/// 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 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" + "/FontBBox {0 -200 700 800} readonly def\n" + "/Encoding 256 array\n" + "0 1 255 {1 index exch /.notdef put} for\n" + "dup 65 /A put\n" + "dup 66 /B put\n" + "readonly def\n" + "currentdict end\n" + "currentfile eexec\n"; std::string private_section = "dup /Private 16 dict dup begin\n" - "/lenIV 4 def\n" - "/Subrs 1 array\n"; + "/lenIV " + + std::to_string(len_iv) + + " def\n/Subrs 1 array\n"; private_section += "dup 0 "; { + const std::string plain("\x0b", 1); const std::string subr = - encrypt(std::string("\x0b", 1), 4330, std::string(4, '\0')); // return + len_iv == -1 + ? plain + : encrypt(plain, 4330, + std::string(static_cast(len_iv), '\0')); private_section += std::to_string(subr.size()) + " RD " + subr + " NP\n"; } private_section += "ND\n" "2 index /CharStrings 2 dict dup begin\n"; // .notdef-ish + two named glyphs. Charstring bytes are arbitrary here: the // parser does not interpret them, it only extracts them. - private_section += charstring_entry("A", std::string("\x8b\x8b\x0d\x0e", 4)); - private_section += charstring_entry("B", std::string("\xf0\x0d\x0e", 3)); + 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 += "end\nend\n"; std::string program = clear; - program += encrypt(private_section, 55665, "wxyz"); + program += encrypt(private_section, 55665, prefix); // Trailer (would be 512 zeros + cleartomark in a real font); the parser // tolerates trailing data, so a short stub is enough. program += std::string(8, '\0'); @@ -89,6 +87,13 @@ TEST(Type1FontTest, IsType1Magic) { } TEST(Type1FontTest, ParsesHeaderAndEncoding) { + struct LocaleGuard final { + std::string previous{std::setlocale(LC_NUMERIC, nullptr)}; + ~LocaleGuard() { std::setlocale(LC_NUMERIC, previous.c_str()); } + } guard; + if (std::setlocale(LC_NUMERIC, "de_DE.UTF-8") == nullptr) { + std::setlocale(LC_NUMERIC, "de_DE.utf8"); + } const Type1Font font{build_type1()}; EXPECT_EQ(font.name(), "TestType1"); @@ -103,16 +108,18 @@ TEST(Type1FontTest, ParsesHeaderAndEncoding) { } TEST(Type1FontTest, DecryptsCharstringsAndSubrs) { - const Type1Font font{build_type1()}; + for (const std::int32_t len_iv : {-1, 0, 4}) { + const Type1Font font{build_type1(len_iv, std::string("\xd9xyz", 4))}; - ASSERT_EQ(font.glyphs().size(), 2u); - EXPECT_EQ(font.glyphs()[0].name, "A"); - EXPECT_EQ(font.glyphs()[0].charstring, std::string("\x8b\x8b\x0d\x0e", 4)); - EXPECT_EQ(font.glyphs()[1].name, "B"); - EXPECT_EQ(font.glyphs()[1].charstring, std::string("\xf0\x0d\x0e", 3)); + ASSERT_EQ(font.glyphs().size(), 2u); + EXPECT_EQ(font.glyphs()[0].name, "A"); + EXPECT_EQ(font.glyphs()[0].charstring, std::string("\x8b\x8b\x0d\x0e", 4)); + EXPECT_EQ(font.glyphs()[1].name, "B"); + EXPECT_EQ(font.glyphs()[1].charstring, std::string("\xf0\x0d\x0e", 3)); - ASSERT_EQ(font.subrs().size(), 1u); - EXPECT_EQ(font.subrs()[0], std::string("\x0b", 1)); // return + ASSERT_EQ(font.subrs().size(), 1u); + EXPECT_EQ(font.subrs()[0], std::string("\x0b", 1)); // return + } } TEST(Type1FontTest, ConvertsToLoadableCff) { @@ -132,3 +139,21 @@ TEST(Type1FontTest, ConvertsToLoadableCff) { // The converted CFF wraps into a browser-loadable OTTO (the 3.4 path). EXPECT_TRUE(sfnt::SfntFont::is_sfnt(cff::wrap_to_otf(font))); } + +TEST(Type1FontTest, RejectsInvalidNumbersAndPfbSegments) { + for (const std::string_view number : {"nan", "inf", "1oops"}) { + std::string program = build_type1(); + program.replace(program.find("0.001"), 5, number); + EXPECT_THROW(Type1Font{program}, std::runtime_error); + } + const std::string program = build_type1(); + std::string pfb("\x80\x01", 2); + odr::internal::util::byte_string::put_u32_le( + pfb, static_cast(program.size())); + pfb += program; + EXPECT_EQ(Type1Font(pfb + std::string("\x80\x03", 2)).glyphs().size(), 2); + EXPECT_THROW(Type1Font(pfb + std::string("\x80\x02\xff\xff\xff\xff", 6)), + std::runtime_error); + pfb[1] = 4; + EXPECT_THROW(Type1Font{pfb}, std::runtime_error); +} diff --git a/test/src/internal/font/type1_test_util.hpp b/test/src/internal/font/type1_test_util.hpp new file mode 100644 index 000000000..7aabc1215 --- /dev/null +++ b/test/src/internal/font/type1_test_util.hpp @@ -0,0 +1,23 @@ +#pragma once + +#include +#include +#include + +namespace odr::test::font { + +/// Reference encryption for Type1 fixtures; the library implements decryption. +inline std::string encrypt(const std::string &plain, std::uint16_t state, + const std::string &prefix) { + std::string out; + for (const char ch : prefix + plain) { + const auto cipher = + static_cast(static_cast(ch) ^ (state >> 8)); + out += static_cast(cipher); + state = static_cast( + (std::uint32_t{cipher} + state) * 52845U + 22719U); + } + return out; +} + +} // namespace odr::test::font From d25dc82fa9557f14ef4d46fe3319a38a1f08dbab Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Mon, 5 Oct 2026 14:14:27 +0200 Subject: [PATCH 2/3] fix(font): keep small Type1 quirks from refusing the font A PDF falls back to a substitute font when its embedded font is refused, so the page shows the wrong glyphs. As before this change, an unreadable number reads as zero, an unreadable /lenIV keeps the default of 4, and a charstring shorter than lenIV decrypts to nothing. A /lenIV below -1 and bad PFB framing still throw. The tests include the shared helper by its path, like the other test utilities. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01Gb1fLafqqzehpqPuU6uBfn --- src/odr/internal/font/type1_crypt.cpp | 6 +++--- src/odr/internal/font/type1_font.cpp | 14 ++++++++------ test/src/internal/font/type1_crypt.cpp | 6 +++--- test/src/internal/font/type1_font.cpp | 8 ++++---- 4 files changed, 18 insertions(+), 16 deletions(-) diff --git a/src/odr/internal/font/type1_crypt.cpp b/src/odr/internal/font/type1_crypt.cpp index a77382322..0c3389cc8 100644 --- a/src/odr/internal/font/type1_crypt.cpp +++ b/src/odr/internal/font/type1_crypt.cpp @@ -1,7 +1,6 @@ #include #include -#include #include namespace odr::internal::font::type1 { @@ -77,8 +76,9 @@ std::string type1::decrypt(const std::string_view cipher, out += static_cast(c ^ (r >> 8)); r = static_cast((std::uint32_t{c} + r) * c1 + c2); } - if (skip > out.size()) { - throw std::runtime_error("type1: truncated encryption prefix"); + // A cipher shorter than its prefix decrypts to nothing. + if (skip >= out.size()) { + return {}; } return out.substr(skip); } diff --git a/src/odr/internal/font/type1_font.cpp b/src/odr/internal/font/type1_font.cpp index e8591489e..7082ede77 100644 --- a/src/odr/internal/font/type1_font.cpp +++ b/src/odr/internal/font/type1_font.cpp @@ -50,11 +50,9 @@ namespace { } [[nodiscard]] double parse_double(const std::string_view token) { + // An unreadable number reads as zero, so it does not refuse the font. const std::optional value = util::number::parse(token); - if (!value || !std::isfinite(*value)) { - throw std::runtime_error("type1: invalid numeric operand"); - } - return *value; + return value && std::isfinite(*value) ? *value : 0.0; } /// A `/FontBBox` number as an FWord. Clamping keeps an out-of-range number out @@ -232,8 +230,12 @@ void Type1Font::parse_private(const std::string_view decrypted) { if (const std::size_t k = decrypted.find("/lenIV"); k != std::string_view::npos) { std::size_t p = k + 6; - if (!parse_int(read_token(decrypted, p), len_iv) || len_iv < -1) { - throw std::runtime_error("type1: invalid /lenIV"); + std::int32_t value = 0; + if (parse_int(read_token(decrypted, p), value)) { + if (value < -1) { + throw std::runtime_error("type1: invalid /lenIV"); + } + len_iv = value; } } const auto decode = [len_iv](const std::string_view bytes) { diff --git a/test/src/internal/font/type1_crypt.cpp b/test/src/internal/font/type1_crypt.cpp index 293444a3b..3a0e9cabc 100644 --- a/test/src/internal/font/type1_crypt.cpp +++ b/test/src/internal/font/type1_crypt.cpp @@ -1,7 +1,7 @@ -#include "type1_test_util.hpp" - #include +#include + #include #include @@ -31,7 +31,7 @@ TEST(Type1CryptTest, CharstringHonoursLenIv) { const std::string plain = "hello"; const std::string cipher = encrypt(plain, 4330, ""); EXPECT_EQ(decrypt_charstring(cipher, 0), plain); - EXPECT_THROW((void)decrypt_charstring("x", 4), std::runtime_error); + EXPECT_EQ(decrypt_charstring("x", 4), ""); } TEST(Type1CryptTest, EexecAcceptsHexForm) { diff --git a/test/src/internal/font/type1_font.cpp b/test/src/internal/font/type1_font.cpp index f2a7023e5..05553a8d3 100644 --- a/test/src/internal/font/type1_font.cpp +++ b/test/src/internal/font/type1_font.cpp @@ -1,5 +1,3 @@ -#include "type1_test_util.hpp" - #include #include @@ -8,6 +6,8 @@ #include #include +#include + #include #include @@ -140,11 +140,11 @@ TEST(Type1FontTest, ConvertsToLoadableCff) { EXPECT_TRUE(sfnt::SfntFont::is_sfnt(cff::wrap_to_otf(font))); } -TEST(Type1FontTest, RejectsInvalidNumbersAndPfbSegments) { +TEST(Type1FontTest, ReadsInvalidNumbersAsZeroAndRejectsBadPfbSegments) { for (const std::string_view number : {"nan", "inf", "1oops"}) { std::string program = build_type1(); program.replace(program.find("0.001"), 5, number); - EXPECT_THROW(Type1Font{program}, std::runtime_error); + EXPECT_EQ(Type1Font{program}.glyphs().size(), 2); } const std::string program = build_type1(); std::string pfb("\x80\x01", 2); From fedd3202fb61aee0b5a750da33db09f34295a15c Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Mon, 5 Oct 2026 14:24:36 +0200 Subject: [PATCH 3/3] refactor: share the hex digit and PostScript white space checks Type1, RTF, PDF and the document color parser each decoded hex digits on their own. util::string now has hex_digit, and the callers use it. RTF and PDF keep their throwing wrappers. Type1 uses the PostScript white space of ObjectParser::is_whitespace in place of its own copy. The Type1 hex loops use is_ascii_whitespace, and the eexec boundary has a named predicate that does not skip NUL or form feed. The document color check no longer depends on the locale. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01Gb1fLafqqzehpqPuU6uBfn --- src/odr/document.cpp | 4 ++-- src/odr/internal/font/type1_crypt.cpp | 26 ++++++++------------- src/odr/internal/font/type1_font.cpp | 15 ++++++------ src/odr/internal/pdf/pdf_object_parser.cpp | 14 ++++------- src/odr/internal/pdf/pdf_object_parser.hpp | 1 + src/odr/internal/rtf/rtf_tokenizer.cpp | 14 ++++------- src/odr/internal/util/string_util.cpp | 13 +++++++++++ src/odr/internal/util/string_util.hpp | 3 +++ test/src/internal/util/string_util_test.cpp | 11 +++++++++ 9 files changed, 58 insertions(+), 43 deletions(-) diff --git a/src/odr/document.cpp b/src/odr/document.cpp index 96aebe43a..4e425f9d7 100644 --- a/src/odr/document.cpp +++ b/src/odr/document.cpp @@ -13,10 +13,10 @@ #include #include #include +#include #include #include -#include #include #include #include @@ -152,7 +152,7 @@ CellValue parse_cell_value(const nlohmann::json &json) { /// `#rrggbb`, as the page spells one. Color parse_color(const std::string &text) { const auto is_hex = [](const char c) { - return std::isxdigit(static_cast(c)) != 0; + return internal::util::string::hex_digit(c).has_value(); }; if (text.size() != 7 || text[0] != '#' || !std::ranges::all_of(text.substr(1), is_hex)) { diff --git a/src/odr/internal/font/type1_crypt.cpp b/src/odr/internal/font/type1_crypt.cpp index 0c3389cc8..a5764251c 100644 --- a/src/odr/internal/font/type1_crypt.cpp +++ b/src/odr/internal/font/type1_crypt.cpp @@ -1,6 +1,9 @@ #include +#include + #include +#include #include namespace odr::internal::font::type1 { @@ -10,31 +13,23 @@ namespace { constexpr std::uint16_t c1 = 52845; constexpr std::uint16_t c2 = 22719; -[[nodiscard]] bool is_hex_digit(const std::uint8_t c) { - return (c >= '0' && c <= '9') || (c >= 'A' && c <= 'F') || - (c >= 'a' && c <= 'f'); -} - /// Hex-decode @p in, skipping whitespace; stops at the first non-hex, non-space /// byte (the binary `eexec` form never reaches here). [[nodiscard]] std::string hex_decode(const std::string_view in) { std::string out; std::int32_t high = -1; for (const char ch : in) { - const auto c = static_cast(ch); - if (std::string_view(" \t\r\n").contains(ch)) { + if (util::string::is_ascii_whitespace(ch)) { continue; } - if (!is_hex_digit(c)) { + const std::optional value = util::string::hex_digit(ch); + if (!value) { break; } - const std::int32_t value = (c <= '9') ? c - '0' - : (c <= 'F') ? c - 'A' + 10 - : c - 'a' + 10; if (high < 0) { - high = value; + high = *value; } else { - out += static_cast((high << 4) | value); + out += static_cast((high << 4) | *value); high = -1; } } @@ -46,11 +41,10 @@ constexpr std::uint16_t c2 = 22719; [[nodiscard]] bool looks_like_hex(const std::string_view eexec) { std::int32_t seen = 0; for (const char ch : eexec) { - const auto c = static_cast(ch); - if (std::string_view(" \t\r\n").contains(ch)) { + if (util::string::is_ascii_whitespace(ch)) { continue; } - if (!is_hex_digit(c)) { + if (!util::string::hex_digit(ch)) { return false; } if (++seen == 4) { diff --git a/src/odr/internal/font/type1_font.cpp b/src/odr/internal/font/type1_font.cpp index 7082ede77..1de580053 100644 --- a/src/odr/internal/font/type1_font.cpp +++ b/src/odr/internal/font/type1_font.cpp @@ -1,6 +1,7 @@ #include #include +#include #include #include @@ -18,14 +19,15 @@ namespace odr::internal::font::type1 { namespace { -[[nodiscard]] bool is_ps_space(const char c) { - return c == ' ' || c == '\t' || c == '\r' || c == '\n' || c == '\f' || - c == '\0'; +/// The white space after `eexec`. NUL and form feed can start the binary +/// ciphertext, so they are not skipped here. +[[nodiscard]] bool is_eexec_space(const char c) { + return c == ' ' || c == '\t' || c == '\r' || c == '\n'; } /// Skip PostScript whitespace starting at @p p. [[nodiscard]] std::size_t skip_space(const std::string_view s, std::size_t p) { - while (p < s.size() && is_ps_space(s[p])) { + while (p < s.size() && pdf::ObjectParser::is_whitespace(s[p])) { ++p; } return p; @@ -36,7 +38,7 @@ namespace { std::size_t &p) { p = skip_space(s, p); const std::size_t begin = p; - while (p < s.size() && !is_ps_space(s[p])) { + while (p < s.size() && !pdf::ObjectParser::is_whitespace(s[p])) { ++p; } return s.substr(begin, p - begin); @@ -165,8 +167,7 @@ Type1Font::Type1Font(std::string_view data) { // The encrypted blob begins after `eexec` and its trailing whitespace. std::size_t blob = eexec + 5; - while (blob < data.size() && - std::string_view(" \t\r\n").contains(data[blob])) { + while (blob < data.size() && is_eexec_space(data[blob])) { ++blob; } const std::string decrypted = decrypt_eexec(data.substr(blob)); diff --git a/src/odr/internal/pdf/pdf_object_parser.cpp b/src/odr/internal/pdf/pdf_object_parser.cpp index 7ba7a2ef0..de0bb9e6f 100644 --- a/src/odr/internal/pdf/pdf_object_parser.cpp +++ b/src/odr/internal/pdf/pdf_object_parser.cpp @@ -1,6 +1,7 @@ #include #include +#include #include #include @@ -86,16 +87,11 @@ std::uint8_t ObjectParser::octet_char_to_int(const char_type c) { } std::uint8_t ObjectParser::hex_char_to_int(const char_type c) { - if (c >= '0' && c <= '9') { - return c - '0'; - } - if (c >= 'A' && c <= 'F') { - return c - 'A' + 10; - } - if (c >= 'a' && c <= 'f') { - return c - 'a' + 10; + const std::optional value = util::string::hex_digit(c); + if (!value) { + throw std::runtime_error("invalid character in hex_char_to_int"); } - throw std::runtime_error("invalid character in hex_char_to_int"); + return *value; } ObjectParser::char_type ObjectParser::two_hex_to_char(const char_type first, diff --git a/src/odr/internal/pdf/pdf_object_parser.hpp b/src/odr/internal/pdf/pdf_object_parser.hpp index 5f9034eba..ccee712d1 100644 --- a/src/odr/internal/pdf/pdf_object_parser.hpp +++ b/src/odr/internal/pdf/pdf_object_parser.hpp @@ -41,6 +41,7 @@ class ObjectParser { static char_type three_octet_to_char(char_type first, char_type second, char_type third); + /// PostScript white space, which PDF shares (7.2.2). 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); diff --git a/src/odr/internal/rtf/rtf_tokenizer.cpp b/src/odr/internal/rtf/rtf_tokenizer.cpp index a3f7e8e7e..88f14b017 100644 --- a/src/odr/internal/rtf/rtf_tokenizer.cpp +++ b/src/odr/internal/rtf/rtf_tokenizer.cpp @@ -6,6 +6,7 @@ #include #include #include +#include #include #include @@ -16,16 +17,11 @@ namespace str = util::string; namespace { std::uint8_t hex_char_to_int(const char c) { - if (c >= '0' && c <= '9') { - return static_cast(c - '0'); + const std::optional value = str::hex_digit(c); + if (!value) { + throw std::runtime_error("invalid hex digit"); } - if (c >= 'a' && c <= 'f') { - return static_cast(c - 'a' + 10); - } - if (c >= 'A' && c <= 'F') { - return static_cast(c - 'A' + 10); - } - throw std::runtime_error("invalid hex digit"); + return *value; } char two_hex_to_char(const char first, const char second) { diff --git a/src/odr/internal/util/string_util.cpp b/src/odr/internal/util/string_util.cpp index 23b1dc614..90c5e20c6 100644 --- a/src/odr/internal/util/string_util.cpp +++ b/src/odr/internal/util/string_util.cpp @@ -34,6 +34,19 @@ bool string::is_ascii_letter_or_digit(const char c) { return is_ascii_letter(c) || is_ascii_digit(c); } +std::optional string::hex_digit(const char c) { + if (c >= '0' && c <= '9') { + return static_cast(c - '0'); + } + if (c >= 'A' && c <= 'F') { + return static_cast(c - 'A' + 10); + } + if (c >= 'a' && c <= 'f') { + return static_cast(c - 'a' + 10); + } + return std::nullopt; +} + char string::to_lower(const char c) { return c >= 'A' && c <= 'Z' ? static_cast(c + ('a' - 'A')) : c; } diff --git a/src/odr/internal/util/string_util.hpp b/src/odr/internal/util/string_util.hpp index 135a9ef16..bd1b6e1ec 100644 --- a/src/odr/internal/util/string_util.hpp +++ b/src/odr/internal/util/string_util.hpp @@ -2,6 +2,7 @@ #include #include +#include #include #include #include @@ -23,6 +24,8 @@ bool is_ascii_whitespace(char c); bool is_ascii_digit(char c); bool is_ascii_letter(char c); bool is_ascii_letter_or_digit(char c); +/// The value of the hex digit @p c, or nothing if @p c is not 0-9, A-F or a-f. +std::optional hex_digit(char c); /// `std::tolower` / `std::toupper` for the default C locale, made safe for any /// `char` value. diff --git a/test/src/internal/util/string_util_test.cpp b/test/src/internal/util/string_util_test.cpp index 2755298af..48d86d527 100644 --- a/test/src/internal/util/string_util_test.cpp +++ b/test/src/internal/util/string_util_test.cpp @@ -6,6 +6,7 @@ #include #include #include +#include #include #include #include @@ -168,6 +169,16 @@ TEST(string_util, ascii_classes_ignore_the_host_locale) { std::setlocale(LC_CTYPE, previous.c_str()); } +TEST(string_util, hex_digit) { + EXPECT_EQ(hex_digit('0'), 0); + EXPECT_EQ(hex_digit('9'), 9); + EXPECT_EQ(hex_digit('a'), 10); + EXPECT_EQ(hex_digit('F'), 15); + for (const char c : {'g', 'G', '/', ':', '@', '`', ' ', '\0', '\xff'}) { + EXPECT_EQ(hex_digit(c), std::nullopt); + } +} + TEST(string_util, equals_ignore_case) { EXPECT_TRUE(equals_ignore_case("script", "SCRIPT")); EXPECT_TRUE(equals_ignore_case("", ""));