From 8c3812499cc15ee0673159bf81c8a6c5ad64cd85 Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Thu, 22 Oct 2026 18:26:21 +0800 Subject: [PATCH] M10: read a font's post names once for the 256 codes (#3962) CodeRabbit on #3969 (nitpick): Post::glyph_name walks a version 2.0 table's name data from its start for each glyph, and symbolic_glyph_texts asks for up to 256, so an untrusted font program (up to 16 MiB) could make opening a document slow. The names the table spells out are now read once (post_custom_names) and looked up by index (post_name), giving what Post::glyph_name gives; a test checks that for every glyph of Inter's version 2.0 table. Co-Authored-By: Claude Opus 5.5 --- crates/fonts/src/pdf.rs | 30 ++++++++++++++++++++++++++++-- crates/fonts/src/pdf/tests.rs | 16 ++++++++++++++++ 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/crates/fonts/src/pdf.rs b/crates/fonts/src/pdf.rs index a81f1b341e..5d42ca67d7 100644 --- a/crates/fonts/src/pdf.rs +++ b/crates/fonts/src/pdf.rs @@ -1260,6 +1260,7 @@ fn symbolic_glyph_texts(doc: &Document, font: &Dict) -> Vec<(u32, String)> { None => face.post(), }; let Ok(post) = post else { return Vec::new() }; + let custom = post_custom_names(&post); (0..=255u32) .filter_map(|code| { // The records in order, as the renderer's `map_code` reads a symbolic font: a Windows @@ -1274,12 +1275,37 @@ fn symbolic_glyph_texts(doc: &Document, font: &Dict) -> Vec<(u32, String)> { let subtable = record.subtable(cmap.offset_data()).ok()?; offsets.iter().find_map(|o| subtable.map_codepoint(code + o).filter(|g| *g != GlyphId::NOTDEF)) })?; - let glyph = skrifa::raw::types::GlyphId16::new(u16::try_from(glyph.to_u32()).ok()?); - Some((code, glyph_text(post.glyph_name(glyph)?)?)) + let glyph = u16::try_from(glyph.to_u32()).ok()?; + Some((code, glyph_text(post_name(&post, &custom, glyph)?)?)) }) .collect() } +/// The names a version 2.0 `post` table spells out, in order, read once. `Post::glyph_name` walks +/// this data from its start for each glyph it is asked about, which over a font program of up to +/// 16 MiB, asked about 256 codes, is work an untrusted file can make large. Reading stops at the +/// first name that can't be read, as each lookup would. +fn post_custom_names<'a>(post: &skrifa::raw::tables::post::Post<'a>) -> Vec<&'a str> { + post.string_data().map(|names| names.iter().map_while(|name| name.ok().map(|n| n.as_str())).collect()).unwrap_or_default() +} + +/// `glyph`'s name in `post`, as `Post::glyph_name` gives it, with the table's own names from +/// [`post_custom_names`]. +fn post_name<'a>(post: &skrifa::raw::tables::post::Post<'a>, custom: &[&'a str], glyph: u16) -> Option<&'a str> { + use skrifa::raw::{tables::post::DEFAULT_GLYPH_NAMES, types::Version16Dot16}; + match post.version() { + Version16Dot16::VERSION_1_0 => DEFAULT_GLYPH_NAMES.get(usize::from(glyph)).copied(), + Version16Dot16::VERSION_2_0 => { + let index = usize::from(post.glyph_name_index()?.get(usize::from(glyph))?.get()); + match index.checked_sub(DEFAULT_GLYPH_NAMES.len()) { + None => DEFAULT_GLYPH_NAMES.get(index).copied(), + Some(i) => custom.get(i).copied(), + } + } + _ => None, + } +} + /// Code → Unicode for a font: its ToUnicode CMap (for a composite font, also the name /// `/Identity-H` or `/Identity-V`, read as code = Unicode), else (simple fonts) its encoding. fn unicode_map(doc: &Document, font: &Dict, composite: bool) -> Unicode { diff --git a/crates/fonts/src/pdf/tests.rs b/crates/fonts/src/pdf/tests.rs index 83db09047a..d1cff9fa5c 100644 --- a/crates/fonts/src/pdf/tests.rs +++ b/crates/fonts/src/pdf/tests.rs @@ -190,6 +190,22 @@ fn mac_subset_cmap_uses_pdf_unicode_without_changing_source_program() { assert!(append_editor_cmap(&mut broken, &std::collections::BTreeMap::from([(65, 1)])).is_none()); } +/// The names read once give what `Post::glyph_name` gives, for every glyph of a font with its +/// own names (Inter's version 2.0 table), and none past the last glyph. +#[test] +fn post_names_read_once_match_the_table() { + use skrifa::raw::TableProvider as _; + let face = skrifa::FontRef::new(include_bytes!("../../../../assets/fonts/Inter-Regular.ttf")).unwrap(); + let post = face.post().unwrap(); + let custom = post_custom_names(&post); + assert!(custom.len() > 100, "Inter names glyphs of its own: {}", custom.len()); + let glyphs = face.maxp().unwrap().num_glyphs(); + for g in 0..glyphs { + assert_eq!(post_name(&post, &custom, g), post.glyph_name(skrifa::raw::types::GlyphId16::new(g)), "glyph {g}"); + } + assert_eq!(post_name(&post, &custom, glyphs), None); +} + /// #3962: a symbolic TrueType subset with no `/Encoding` whose codes are glyph positions in /// the Macintosh order, 29 below ASCII, mapped by a Mac (1,0) cmap. Its text is what the /// `post` table names the glyphs its codes select, and typing encodes back to those codes.