From 9bc24aadffe6201105ecef5c718f7baeb6026dba Mon Sep 17 00:00:00 2001 From: Grzegorz Martens Date: Wed, 4 Nov 2026 20:56:22 +0100 Subject: [PATCH 01/16] M7: a font's metrics can be read within a caller's decode allowance Metrics::from_dict decodes up to four streams of a font (its Encoding and ToUnicode CMaps, its program, its /CIDToGIDMap), each within a limit of its own but afresh on every call, so a caller reading many fonts that name one large stream decodes it once per font with no total. Metrics::from_dict_within charges each of those decodes to the caller's DecodeBudget, and past_allowance says when one was left unread because the allowance was spent, so a caller can refuse rather than work from guessed widths, code lengths or meanings. A stream past its own limit is left unread as before and doesn't count as past the allowance. from_dict reads with no total, as before. embedded_sfnt_within, which the editor's font listing shares one allowance across, read the metrics that adapt a program's cmap outside that allowance: their ToUnicode and the program again. It now charges them, skips a composite font before reading its metrics (it never used them), and adapts no copy from metrics read past the allowance. Part of #5755. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AmsdeUCdTaD72jkDw2pJKY --- crates/fonts/src/pdf.rs | 89 +++++++++++++++++----- crates/fonts/src/pdf/tests.rs | 138 +++++++++++++++++++++++++++++++++- 2 files changed, 205 insertions(+), 22 deletions(-) diff --git a/crates/fonts/src/pdf.rs b/crates/fonts/src/pdf.rs index 240b1c4d55..a2f28702ab 100644 --- a/crates/fonts/src/pdf.rs +++ b/crates/fonts/src/pdf.rs @@ -14,7 +14,7 @@ use std::borrow::Cow; use std::collections::{BinaryHeap, HashMap, HashSet}; -use pdfcraft_cos::{DecodeBudget, Dict, Document, Object}; +use pdfcraft_cos::{DecodeBudget, Dict, Document, Object, Stream}; /// How the bytes of a string map to codes. #[derive(Clone, Debug, PartialEq)] @@ -67,6 +67,10 @@ pub struct Metrics { /// Subset CIDFontType2 only: one bit per CID (below 65 536) whose glyph has an outline in the /// embedded program. See [`cids_with_outlines`]. outlined_cids: Vec, + /// A stream these metrics come from (a CMap, the program, `/CIDToGIDMap`) was left unread + /// because the decode allowance [`Metrics::from_dict_within`] was given was spent: what it + /// would have decided (code lengths, CIDs, widths, meanings) is a guess. + pub past_allowance: bool, } fn nums(doc: &Document, o: Option<&Object>) -> Vec { @@ -92,14 +96,17 @@ fn legacy_indic_font(base_font: &str) -> bool { /// A source PDF's sfnt program for an in-memory editor. Never a bundled font or an export /// fallback. Type 1/Type 3 programs cannot be consumed by an sfnt text renderer. pub fn embedded_sfnt(doc: &Document, font: &Dict) -> Option> { - embedded_sfnt_within(doc, font, &mut DecodeBudget::new(2 * MAX_SFNT, 0)) + // No total: one font's two programs, and the metrics read that may adapt one, are each + // bounded by that stream's own limit. + embedded_sfnt_within(doc, font, &mut DecodeBudget::new(usize::MAX, 0)) } /// The largest embedded sfnt program read for the editor. const MAX_SFNT: usize = 16 << 20; /// [`embedded_sfnt`], decoding against `budget`, which an enumeration of a page's fonts shares: -/// each program decoded is charged whether or not it turns out to be usable. +/// each program decoded is charged whether or not it turns out to be usable, and so are the +/// streams of the metrics read that adapts a program without a Unicode cmap. pub fn embedded_sfnt_within(doc: &Document, font: &Dict, budget: &mut DecodeBudget) -> Option> { use skrifa::{MetadataProvider as _, raw::TableProvider as _}; let descendant = dict(doc, font.get(b"DescendantFonts").and_then(|o| doc.resolve(o).as_array().and_then(|a| a.first()).cloned()).as_ref()); @@ -120,8 +127,13 @@ pub fn embedded_sfnt_within(doc: &Document, font: &Dict, budget: &mut DecodeBudg // format-4/12 cmaps. Combine the PDF's Unicode meanings with the source glyph IDs; // only this in-memory copy gets a new cmap, never the working PDF or a font asset. let Ok(cmap) = face.cmap() else { continue }; - let metrics = Metrics::from_dict(doc, font); - if metrics.composite { + // A composite font's codes aren't these, so its metrics aren't read (Metrics::composite + // is this test). Meanings read past the allowance would be guesses: no copy is adapted. + if font.name(b"Subtype") == Some(b"Type0") { + continue; + } + let metrics = Metrics::from_dict_within(doc, font, budget); + if metrics.past_allowance { continue; } let mut mappings = std::collections::BTreeMap::new(); @@ -147,7 +159,7 @@ pub fn embedded_sfnt_within(doc: &Document, font: &Dict, budget: &mut DecodeBudg /// The CIDs (below 65 536) of a Type 0 font over an embedded TrueType program whose glyph has /// outline data, through `/CIDToGIDMap`; empty when the program or the map can't be read. /// Subsetters keep glyph ids and empty the glyphs they drop, so an empty glyph is a missing one. -fn cids_with_outlines(doc: &Document, font: &Dict) -> Vec { +fn cids_with_outlines(doc: &Document, font: &Dict, reads: &mut Reads<'_>) -> Vec { use skrifa::raw::TableProvider as _; let Some(descendant) = dict(doc, font.get(b"DescendantFonts").and_then(|o| doc.resolve(o).as_array().and_then(|a| a.first()).cloned()).as_ref()) else { @@ -156,13 +168,13 @@ fn cids_with_outlines(doc: &Document, font: &Dict) -> Vec { let Some(program) = dict(doc, descendant.get(b"FontDescriptor")).and_then(|d| d.get(b"FontFile2").cloned()) else { return Vec::new() }; let program = doc.resolve(&program); let Object::Stream(stream) = program.as_ref() else { return Vec::new() }; - let Ok(bytes) = stream.decoded_within(16 << 20) else { return Vec::new() }; + let Some(bytes) = reads.decode(stream, MAX_SFNT) else { return Vec::new() }; let Some(loca) = skrifa::FontRef::from_index(&bytes, 0).ok().and_then(|face| face.loca(None).ok()) else { return Vec::new() }; let outlined = |gid: u32| loca.get_raw(gid as usize).zip(loca.get_raw(gid as usize + 1)).is_some_and(|(start, end)| end > start); match descendant.get(b"CIDToGIDMap").map(|o| doc.resolve(o)).as_deref() { None | Some(Object::Name(_)) => bit_set_below_64k((0..loca.len().min(0x1_0000) as u32).filter(|&gid| outlined(gid))), Some(Object::Stream(map)) => { - let Ok(map) = map.decoded_within(1 << 17) else { return Vec::new() }; + let Some(map) = reads.decode(map, MAX_CID_TO_GID) else { return Vec::new() }; bit_set_below_64k( map.as_chunks::<2>() .0 @@ -181,7 +193,7 @@ fn cids_with_outlines(doc: &Document, font: &Dict) -> Vec { /// wider and leave the end of a line outside a redaction mark (#2130). The glyph is looked up /// the way the renderer does: the encoding's character in the Unicode cmap, else the code itself /// (as in a symbolic cmap), else `.notdef`. Only sfnt programs are read; others keep the guess. -fn embedded_widths(doc: &Document, font: &Dict, m: &mut Metrics) { +fn embedded_widths(doc: &Document, font: &Dict, m: &mut Metrics, reads: &mut Reads<'_>) { use skrifa::instance::{LocationRef, Size}; use skrifa::{GlyphId, MetadataProvider as _, raw::TableProvider as _}; let Some(descriptor) = dict(doc, font.get(b"FontDescriptor")) else { return }; @@ -189,7 +201,7 @@ fn embedded_widths(doc: &Document, font: &Dict, m: &mut Metrics) { let Some(object) = descriptor.get(key) else { continue }; let object = doc.resolve(object); let Object::Stream(stream) = object.as_ref() else { continue }; - let Ok(bytes) = stream.decoded_within(16 << 20) else { continue }; + let Some(bytes) = reads.decode(stream, MAX_SFNT) else { continue }; let Ok(face) = skrifa::FontRef::from_index(&bytes, 0) else { continue }; let upem = f64::from(face.metrics(Size::unscaled(), LocationRef::default()).units_per_em); if upem == 0.0 { @@ -287,6 +299,33 @@ const MAX_CMAP_RANGES: usize = 1 << 16; /// Largest decoded Encoding or ToUnicode CMap stream read; real CMaps stay well under it, and a larger /// one is not decoded at all. const MAX_CMAP_BYTES: usize = 16 << 20; +/// Largest decoded `/CIDToGIDMap` stream read: two bytes for each CID below 65,536. +const MAX_CID_TO_GID: usize = 1 << 17; + +/// The streams one reading of a font decodes, against the caller's allowance, and whether one was +/// left unread because that allowance was spent. +struct Reads<'a> { + budget: &'a mut DecodeBudget, + refused: bool, +} + +impl<'a> Reads<'a> { + fn new(budget: &'a mut DecodeBudget) -> Self { + Self { budget, refused: false } + } + + /// `stream` decoded within `max` and the allowance; `None` when it can't be, noting when the + /// allowance is why. A stream past `max` is unread as before allowances, not refused. + fn decode(&mut self, stream: &Stream, max: usize) -> Option> { + match self.budget.decode_within(stream, max) { + Ok(data) => Some(data), + Err(_) => { + self.refused |= self.budget.remaining() == 0; + None + } + } + } +} /// Code → Unicode of a font. Explicit mappings and sequential ranges are kept as declared, so a range /// costs one record however many codes it covers; where two overlap, the one declared later wins. @@ -628,11 +667,24 @@ impl Metrics { code_len: 1, explicit_reverse: (32..127u8).map(|c| (char::from(c), u32::from(c))).collect(), outlined_cids: Vec::new(), + past_allowance: false, } } + /// The metrics of `font`. Each stream they come from decodes within its own limit (16 MiB for + /// a CMap or a program), and again on every call: a caller that reads many fonts, or one font + /// many times, shares an allowance through [`Metrics::from_dict_within`]. pub fn from_dict(doc: &Document, font: &Dict) -> Self { - let mut m = Self::read_metrics(doc, font); + // No total, so nothing is refused: each stream stays within its own limit, as it always has. + Self::from_dict_within(doc, font, &mut DecodeBudget::new(usize::MAX, 0)) + } + + /// [`Metrics::from_dict`], decoding against `budget`, which the caller shares across the fonts + /// it reads (a page's, say): each stream pays for what it decodes, usable or not. A stream the + /// allowance can't pay for is left unread, and [`Metrics::past_allowance`] says so. + pub fn from_dict_within(doc: &Document, font: &Dict, budget: &mut DecodeBudget) -> Self { + let mut reads = Reads::new(budget); + let mut m = Self::read_metrics(doc, font, &mut reads); let base = font .name(b"BaseFont") .map(|name| String::from_utf8_lossy(name).into_owned()) @@ -649,17 +701,18 @@ impl Metrics { m.italic |= italic; m.base_font = base; if m.composite && m.subset { - m.outlined_cids = cids_with_outlines(doc, font); + m.outlined_cids = cids_with_outlines(doc, font, &mut reads); } m.code_len = match &m.codes { Codes::One => 1, Codes::Two => 2, Codes::Ranges(r) => r.last().map_or(2, |x| x.0), }; - m.unicode = unicode_map(doc, font, m.composite); + m.unicode = unicode_map(doc, font, m.composite, &mut reads); if m.std14.is_some() { - embedded_widths(doc, font, &mut m); + embedded_widths(doc, font, &mut m, &mut reads); } + m.past_allowance = reads.refused; let mut reverse = HashMap::new(); for (&code, text) in m.unicode.explicit.iter().map(|(c, (_, t))| (c, t)).chain(m.unicode.encoding.iter()) { if let Some(ch) = only_char(text) @@ -680,7 +733,7 @@ impl Metrics { m } - fn read_metrics(doc: &Document, font: &Dict) -> Self { + fn read_metrics(doc: &Document, font: &Dict, reads: &mut Reads<'_>) -> Self { let mut m = Metrics::fallback(); m.std14 = None; let subtype = font.name(b"Subtype").unwrap_or(b"Type1").to_vec(); @@ -691,7 +744,7 @@ impl Metrics { m.codes = Codes::Two; if let Some(e) = font.get(b"Encoding").map(|e| doc.resolve(e)) && let Object::Stream(s) = &*e - && let Ok(data) = s.decoded_within(MAX_CMAP_BYTES) + && let Some(data) = reads.decode(s, MAX_CMAP_BYTES) { let (spaces, cids) = parse_cmap(&data); if !spaces.is_empty() { @@ -1229,7 +1282,7 @@ pub fn glyph_unicode(name: &str) -> Option { /// 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 { +fn unicode_map(doc: &Document, font: &Dict, composite: bool, reads: &mut Reads<'_>) -> Unicode { let mut out = Unicode::default(); if !composite { let enc = font.get(b"Encoding").map(|e| doc.resolve(e)); @@ -1293,7 +1346,7 @@ fn unicode_map(doc: &Document, font: &Dict, composite: bool) -> Unicode { } match font.get(b"ToUnicode").map(|t| doc.resolve(t)).as_deref() { Some(Object::Stream(s)) => { - if let Ok(data) = s.decoded_within(MAX_CMAP_BYTES) { + if let Some(data) = reads.decode(s, MAX_CMAP_BYTES) { parse_to_unicode(&data, &mut out); } } diff --git a/crates/fonts/src/pdf/tests.rs b/crates/fonts/src/pdf/tests.rs index c2acef191c..798c619018 100644 --- a/crates/fonts/src/pdf/tests.rs +++ b/crates/fonts/src/pdf/tests.rs @@ -154,9 +154,10 @@ fn a_font_without_widths_measures_with_its_embedded_program() { assert!(m.width(u32::from('i')) < 0.4, "not Courier's 0.6"); } -#[test] -fn mac_subset_cmap_uses_pdf_unicode_without_changing_source_program() { - use skrifa::{MetadataProvider as _, raw::TableProvider as _}; +/// Inter with its cmap replaced by a Mac format-6 one mapping code 65 to Ω's glyph, which egui +/// can't use; and that glyph. +fn mac_subset() -> (Vec, skrifa::GlyphId) { + use skrifa::MetadataProvider as _; let original = include_bytes!("../../../../assets/fonts/Inter-Regular.ttf"); let face = skrifa::FontRef::new(original).unwrap(); let expected = face.charmap().map('Ω').unwrap(); @@ -170,6 +171,13 @@ fn mac_subset_cmap_uses_pdf_unicode_without_changing_source_program() { subset[entry + 8..entry + 12].copy_from_slice(&offset.to_be_bytes()); subset[entry + 12..entry + 16].copy_from_slice(&(cmap.len() as u32).to_be_bytes()); subset.extend_from_slice(&cmap); + (subset, expected) +} + +#[test] +fn mac_subset_cmap_uses_pdf_unicode_without_changing_source_program() { + use skrifa::{MetadataProvider as _, raw::TableProvider as _}; + let (subset, expected) = mac_subset(); let face = skrifa::FontRef::new(&subset).unwrap(); assert!(!face.charmap().has_map(), "egui cannot directly use Mac format 6"); assert_eq!(face.cmap().unwrap().map_codepoint(65_u32), Some(expected)); @@ -190,6 +198,128 @@ 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 metrics read that adapts a program's cmap is paid for from the editor's allowance too: a +/// step short of the program, its ToUnicode and the program again for its widths, no copy is +/// adapted from guessed meanings. +#[test] +fn adapting_a_programs_cmap_is_paid_for() { + use skrifa::MetadataProvider as _; + let (subset, expected) = mac_subset(); + let mut doc = Document::new_empty(); + let program = doc.add(Object::Stream(Stream::flate(Dict::new(), &subset))); + let mut descriptor = Dict::new(); + descriptor.set(b"FontFile2".to_vec(), Object::Ref(program)); + let cmap = b"1 beginbfchar <41> <03A9> endbfchar"; + let unicode = doc.add(Object::Stream(Stream::flate(Dict::new(), cmap))); + let source = font(&mut doc, vec![("FontDescriptor", Object::Dict(descriptor)), ("ToUnicode", Object::Ref(unicode))]); + let cost = 2 * subset.len() + cmap.len(); + let mut budget = DecodeBudget::new(cost, 0); + let editor = embedded_sfnt_within(&doc, &source, &mut budget).unwrap(); + assert_eq!(skrifa::FontRef::new(&editor).unwrap().charmap().map('Ω'), Some(expected)); + assert_eq!(budget.remaining(), 0); + assert!(embedded_sfnt_within(&doc, &source, &mut DecodeBudget::new(cost - 1, 0)).is_none()); + // A composite font's metrics aren't read at all: the program is all it pays for. + let mut budget = DecodeBudget::new(cost, 0); + let mut composite = source.clone(); + composite.set(b"Subtype".to_vec(), Object::name("Type0")); + assert!(embedded_sfnt_within(&doc, &composite, &mut budget).is_none()); + assert_eq!(budget.remaining(), cost - subset.len()); +} + +/// A Type 0 subset font whose `/Encoding`, program, `/CIDToGIDMap` and ToUnicode are streams, and +/// what each decodes to. +fn subset_type0(doc: &mut Document, to_unicode: &[u8]) -> (Dict, [usize; 4]) { + let encoding = b"1 begincodespacerange <0000> endcodespacerange"; + let program = include_bytes!("../../../../assets/fonts/Inter-Regular.ttf"); + let map = [0u8, 0, 0, 1, 0, 2]; + let mut stream = |data: &[u8]| Object::Ref(doc.add(Object::Stream(Stream::flate(Dict::new(), data)))); + let mut descriptor = Dict::new(); + descriptor.set(b"FontFile2".to_vec(), stream(program)); + let mut descendant = Dict::new(); + descendant.set(b"Subtype".to_vec(), Object::name("CIDFontType2")); + descendant.set(b"FontDescriptor".to_vec(), Object::Dict(descriptor)); + descendant.set(b"CIDToGIDMap".to_vec(), stream(&map)); + let entries = vec![ + ("Subtype", Object::name("Type0")), + ("BaseFont", Object::name("ABCDEF+Inter")), + ("Encoding", stream(encoding)), + ("DescendantFonts", Object::Array(vec![Object::Dict(descendant)])), + ("ToUnicode", stream(to_unicode)), + ]; + (font(doc, entries), [encoding.len(), program.len(), map.len(), to_unicode.len()]) +} + +/// Each stream a font's metrics come from is paid for from the allowance they are read within, +/// down to the byte; a stream it can't pay for is left unread, and the metrics say so. +#[test] +fn a_fonts_streams_are_paid_for_from_its_allowance() { + let mut doc = Document::new_empty(); + let (f, sizes) = subset_type0(&mut doc, b"1 beginbfchar <0001> <0058> endbfchar"); + let cost: usize = sizes.iter().sum(); + let whole = Metrics::from_dict(&doc, &f); + assert!(!whole.past_allowance); + assert_eq!((whole.code_len, whole.text_of(1).as_deref()), (2, Some("X"))); + assert!(!whole.outlined_cids.is_empty(), "the program and its map were read"); + let mut budget = DecodeBudget::new(cost, 0); + assert_eq!(Metrics::from_dict_within(&doc, &f, &mut budget), whole); + assert_eq!(budget.remaining(), 0); + // A byte short, the ToUnicode, read last, is left unread; what came before is kept. + let mut budget = DecodeBudget::new(cost - 1, 0); + let m = Metrics::from_dict_within(&doc, &f, &mut budget); + assert!(m.past_allowance); + assert_eq!((m.code_len, m.text_of(1)), (2, None)); + assert_eq!(m.outlined_cids, whole.outlined_cids); + // With nothing left, none of them is read. + let m = Metrics::from_dict_within(&doc, &f, &mut budget); + assert!(m.past_allowance && m.outlined_cids.is_empty()); + assert_eq!(m.codes, Codes::Two, "the Encoding CMap was left unread"); + // Only a subset's program says which glyphs it lacks: the whole font pays for its CMaps alone. + let mut whole_font = f.clone(); + whole_font.set(b"BaseFont".to_vec(), Object::name("Inter")); + let mut budget = DecodeBudget::new(cost, 0); + let m = Metrics::from_dict_within(&doc, &whole_font, &mut budget); + assert!(!m.past_allowance && m.outlined_cids.is_empty()); + assert_eq!(cost - budget.remaining(), sizes[0] + sizes[3]); +} + +/// A stream past its own limit is left unread as it always was, paying for the limit, and is no +/// reason to call the font read past the allowance. +#[test] +fn a_cmap_past_its_own_limit_is_not_past_the_allowance() { + let mut doc = Document::new_empty(); + let mut cmap = b"1 beginbfchar <0001> <0058> endbfchar".to_vec(); + cmap.resize(MAX_CMAP_BYTES + 1, b' '); + let (f, sizes) = subset_type0(&mut doc, &cmap); + let mut budget = DecodeBudget::new(usize::MAX / 2, 0); + let m = Metrics::from_dict_within(&doc, &f, &mut budget); + assert!(!m.past_allowance); + assert_eq!(m.text_of(1), None); + assert_eq!(usize::MAX / 2 - budget.remaining(), sizes[..3].iter().sum::() + MAX_CMAP_BYTES); +} + +/// Fonts that each name one large ToUnicode share the allowance they are read within: it pays for +/// as many readings as it holds, and the rest are left unread, where each reading used to decode +/// it afresh. +#[test] +fn fonts_naming_one_cmap_share_one_allowance() { + let mut doc = Document::new_empty(); + let mut cmap = b"1 beginbfchar <41> <0058> endbfchar".to_vec(); + cmap.resize(1_000_000, b' '); + let unicode = doc.add(Object::Stream(Stream::flate(Dict::new(), &cmap))); + let fonts: Vec = (0..100) + .map(|_| { + font(&mut doc, vec![("Subtype", Object::name("Type1")), ("BaseFont", Object::name("Helvetica")), ("ToUnicode", Object::Ref(unicode))]) + }) + .collect(); + let mut budget = DecodeBudget::new(64 << 20, 0); + let read: Vec = fonts.iter().map(|f| Metrics::from_dict_within(&doc, f, &mut budget)).collect(); + let whole = read.iter().filter(|m| !m.past_allowance).count(); + assert_eq!(whole, (64 << 20) / cmap.len()); + assert!(read[..whole].iter().all(|m| m.text_of(0x41).as_deref() == Some("X"))); + assert!(read[whole..].iter().all(|m| m.text_of(0x41).as_deref() == Some("A")), "the encoding's meaning, unread past the allowance"); + assert_eq!(budget.remaining(), 0); +} + fn metrics_with_flags(flags: Option) -> Metrics { let mut doc = Document::new_empty(); let mut descriptor = Dict::new(); @@ -565,7 +695,7 @@ fn malformed_cmaps_never_panic() { let mut enc = Dict::new(); enc.set(b"Differences".to_vec(), Object::Array(vec![Object::Int(i64::MAX), Object::name("A"), Object::name("B")])); let f = font(&mut doc, vec![("Subtype", Object::name("Type1")), ("Encoding", Object::Dict(enc))]); - assert_eq!(unicode_map(&doc, &f, false).text(u32::MAX).as_deref(), Some("B")); + assert_eq!(unicode_map(&doc, &f, false, &mut Reads::new(&mut DecodeBudget::new(usize::MAX, 0))).text(u32::MAX).as_deref(), Some("B")); } #[test] From ade98773701713ec0cc9ee2224be8d852ad8ac74 Mon Sep 17 00:00:00 2001 From: Grzegorz Martens Date: Wed, 4 Nov 2026 22:06:46 +0100 Subject: [PATCH 02/16] M7: a font's stream behind several filters reads as it did without an allowance A DecodeBudget bounds what a stream's filter stages make together, and the first commit passed it each stream's own limit, so a /CIDToGIDMap of 65,536 CIDs behind ASCIIHex and Flate (131,072 bytes, with the hex stage's output on top) went unread, where a decode without an allowance bounds each stage alone and read it. Every Metrics::from_dict caller saw the font as missing those glyphs. The 16 MiB limits on CMaps and programs had the same edge. Each stage may now make up to the stream's own limit, as before, and the allowance pays for all of them; what the stream decodes to stays within its limit. Found by the Consumers review. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AmsdeUCdTaD72jkDw2pJKY --- crates/fonts/src/pdf.rs | 11 +++++++++-- crates/fonts/src/pdf/tests.rs | 37 +++++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/crates/fonts/src/pdf.rs b/crates/fonts/src/pdf.rs index a2f28702ab..b51bb0a657 100644 --- a/crates/fonts/src/pdf.rs +++ b/crates/fonts/src/pdf.rs @@ -316,9 +316,16 @@ impl<'a> Reads<'a> { /// `stream` decoded within `max` and the allowance; `None` when it can't be, noting when the /// allowance is why. A stream past `max` is unread as before allowances, not refused. + /// + /// `max` bounds what the stream decodes to. Its filter stages are paid for together, up to + /// `max` for each, the most a decode without an allowance lets them make: a map of 65,536 CIDs + /// behind ASCIIHex and Flate makes all of `MAX_CID_TO_GID` in its last stage, with the hex + /// stage's output on top. fn decode(&mut self, stream: &Stream, max: usize) -> Option> { - match self.budget.decode_within(stream, max) { - Ok(data) => Some(data), + let stages = stream.filters().len().max(1); + match self.budget.decode_within(stream, max.saturating_mul(stages)) { + Ok(data) if data.len() <= max => Some(data), + Ok(_) => None, Err(_) => { self.refused |= self.budget.remaining() == 0; None diff --git a/crates/fonts/src/pdf/tests.rs b/crates/fonts/src/pdf/tests.rs index 798c619018..dbcf367bd4 100644 --- a/crates/fonts/src/pdf/tests.rs +++ b/crates/fonts/src/pdf/tests.rs @@ -297,6 +297,43 @@ fn a_cmap_past_its_own_limit_is_not_past_the_allowance() { assert_eq!(usize::MAX / 2 - budget.remaining(), sizes[..3].iter().sum::() + MAX_CMAP_BYTES); } +/// A `/CIDToGIDMap` as long as a map can be reads the same behind no filter, one or two: each +/// stage may make as much as the map's limit, so the hex stage's output on top of it doesn't +/// leave it unread. A map past the limit is unread behind any of them. +#[test] +fn a_full_cid_map_reads_behind_any_number_of_filters() { + let read = |entries: usize, filters: usize| { + let mut doc = Document::new_empty(); + let (mut f, _) = subset_type0(&mut doc, b""); + let map: Vec = (0..entries).flat_map(|cid| u16::try_from(cid % 2000).unwrap().to_be_bytes()).collect(); + let flate = Stream::flate(Dict::new(), &map); + let stream = match filters { + 0 => Stream::from_raw(Dict::new(), map), + 1 => flate, + _ => { + let hex: String = flate.raw.iter().map(|b| format!("{b:02X}")).chain([">".to_string()]).collect(); + let mut dict = Dict::new(); + dict.set(b"Filter".to_vec(), Object::Array(vec![Object::name("ASCIIHexDecode"), Object::name("FlateDecode")])); + Stream::from_raw(dict, hex.into_bytes()) + } + }; + let map = Object::Ref(doc.add(Object::Stream(stream))); + let Some(Object::Array(mut descendants)) = f.get(b"DescendantFonts").cloned() else { panic!("descendant") }; + if let Some(Object::Dict(d)) = descendants.get_mut(0) { + d.set(b"CIDToGIDMap".to_vec(), map); + } + f.set(b"DescendantFonts".to_vec(), Object::Array(descendants)); + Metrics::from_dict(&doc, &f).outlined_cids + }; + let full = read(1 << 16, 1); + assert!(!full.is_empty()); + assert_eq!(read(1 << 16, 0), full); + assert_eq!(read(1 << 16, 2), full); + for filters in 0..=2 { + assert!(read((1 << 16) + 1, filters).is_empty(), "{filters} filters"); + } +} + /// Fonts that each name one large ToUnicode share the allowance they are read within: it pays for /// as many readings as it holds, and the rest are left unread, where each reading used to decode /// it afresh. From fd4b698c71f1b638c14ce0969c79b1aebeec24b7 Mon Sep 17 00:00:00 2001 From: Grzegorz Martens Date: Wed, 4 Nov 2026 22:29:04 +0100 Subject: [PATCH 03/16] M7: a font's streams are read with /DecodeParms given by reference A /CIDToGIDMap, CMap or program whose /DecodeParms are an indirect object, or an array holding one, was decoded without them, as on main: behind a PNG predictor its tag bytes stayed in, so the map read wrong and the CMap or program not at all. Each of the five metrics reads and the editor's program read now go through Document::with_direct_parms, which shares the stream's data and copies its dictionary only when a parameter is indirect. Reads::decode's comment now says what the stage bound does: a stage may make more than the stream's limit where a decode without an allowance stops it, and a stream past its limit counts as refused when what is left can't pay for its stages. Found by the Legal forms review. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AmsdeUCdTaD72jkDw2pJKY --- crates/fonts/src/pdf.rs | 26 +++++++++++------- crates/fonts/src/pdf/tests.rs | 51 ++++++++++++++++++++++++++++++++++- 2 files changed, 66 insertions(+), 11 deletions(-) diff --git a/crates/fonts/src/pdf.rs b/crates/fonts/src/pdf.rs index b51bb0a657..e526f2a8e6 100644 --- a/crates/fonts/src/pdf.rs +++ b/crates/fonts/src/pdf.rs @@ -115,7 +115,7 @@ pub fn embedded_sfnt_within(doc: &Document, font: &Dict, budget: &mut DecodeBudg let Some(object) = descriptor.get(key) else { continue }; let object = doc.resolve(object); let Object::Stream(stream) = object.as_ref() else { continue }; - let Ok(mut bytes) = budget.decode_within(stream, MAX_SFNT) else { continue }; + let Ok(mut bytes) = budget.decode_within(&doc.with_direct_parms(stream), MAX_SFNT) else { continue }; let Ok(face) = skrifa::FontRef::from_index(&bytes, 0) else { continue }; if face.metrics(skrifa::instance::Size::unscaled(), skrifa::instance::LocationRef::default()).units_per_em == 0 { continue; @@ -305,25 +305,31 @@ const MAX_CID_TO_GID: usize = 1 << 17; /// The streams one reading of a font decodes, against the caller's allowance, and whether one was /// left unread because that allowance was spent. struct Reads<'a> { + doc: &'a Document, budget: &'a mut DecodeBudget, refused: bool, } impl<'a> Reads<'a> { - fn new(budget: &'a mut DecodeBudget) -> Self { - Self { budget, refused: false } + fn new(doc: &'a Document, budget: &'a mut DecodeBudget) -> Self { + Self { doc, budget, refused: false } } - /// `stream` decoded within `max` and the allowance; `None` when it can't be, noting when the - /// allowance is why. A stream past `max` is unread as before allowances, not refused. + /// `stream` decoded within `max` and the allowance, its `/DecodeParms` read where they are given + /// by reference too ([`Document::with_direct_parms`]); `None` when it can't be, noting when the + /// allowance is why. /// - /// `max` bounds what the stream decodes to. Its filter stages are paid for together, up to - /// `max` for each, the most a decode without an allowance lets them make: a map of 65,536 CIDs + /// `max` bounds what the stream decodes to, and its filter stages together may make up to `max` + /// for each of them, as much as a decode without an allowance lets them: a map of 65,536 CIDs /// behind ASCIIHex and Flate makes all of `MAX_CID_TO_GID` in its last stage, with the hex - /// stage's output on top. + /// stage's output on top. One stage may so make more than `max`, which a decode without an + /// allowance stops, so a stream that one leaves unread can read here, never the other way + /// round. A stream past `max` is unread, not refused, when what is left can pay for its stages; + /// when it can't, the stream spends it and counts as refused. fn decode(&mut self, stream: &Stream, max: usize) -> Option> { + let stream = self.doc.with_direct_parms(stream); let stages = stream.filters().len().max(1); - match self.budget.decode_within(stream, max.saturating_mul(stages)) { + match self.budget.decode_within(&stream, max.saturating_mul(stages)) { Ok(data) if data.len() <= max => Some(data), Ok(_) => None, Err(_) => { @@ -690,7 +696,7 @@ impl Metrics { /// it reads (a page's, say): each stream pays for what it decodes, usable or not. A stream the /// allowance can't pay for is left unread, and [`Metrics::past_allowance`] says so. pub fn from_dict_within(doc: &Document, font: &Dict, budget: &mut DecodeBudget) -> Self { - let mut reads = Reads::new(budget); + let mut reads = Reads::new(doc, budget); let mut m = Self::read_metrics(doc, font, &mut reads); let base = font .name(b"BaseFont") diff --git a/crates/fonts/src/pdf/tests.rs b/crates/fonts/src/pdf/tests.rs index dbcf367bd4..3e914816bb 100644 --- a/crates/fonts/src/pdf/tests.rs +++ b/crates/fonts/src/pdf/tests.rs @@ -334,6 +334,55 @@ fn a_full_cid_map_reads_behind_any_number_of_filters() { } } +/// `data` in rows of `columns` bytes behind Flate and a PNG predictor, each row tagged None, with +/// its `/DecodeParms` given by reference or direct. +fn predicted(doc: &mut Document, data: &[u8], columns: usize, by_reference: bool) -> Object { + let rows: Vec = data.chunks(columns).flat_map(|row| std::iter::once(0).chain(row.iter().copied())).collect(); + let mut s = Stream::flate(Dict::new(), &rows); + let parms: Dict = + [(b"Predictor".to_vec(), Object::Int(12)), (b"Columns".to_vec(), Object::Int(i64::try_from(columns).unwrap()))].into_iter().collect(); + let parms = if by_reference { Object::Ref(doc.add(Object::Dict(parms))) } else { Object::Dict(parms) }; + s.dict.set(b"DecodeParms".to_vec(), parms); + Object::Ref(doc.add(Object::Stream(s))) +} + +/// A stream whose `/DecodeParms` are given by reference reads as it does with them direct: a PNG +/// predictor's tag bytes are taken out of the map, the CMap and the editor's program, not left in. +#[test] +fn parameters_given_by_reference_are_read() { + let read = |by_reference: bool| { + let mut doc = Document::new_empty(); + let (mut f, _) = subset_type0(&mut doc, b""); + let map: Vec = (0..1000u16).flat_map(u16::to_be_bytes).collect(); + let map = predicted(&mut doc, &map, 2, by_reference); + let mut cmap = b"1 beginbfchar <0001> <0058> endbfchar".to_vec(); + cmap.resize(48, b' '); + let unicode = predicted(&mut doc, &cmap, 16, by_reference); + let Some(Object::Array(mut descendants)) = f.get(b"DescendantFonts").cloned() else { panic!("descendant") }; + if let Some(Object::Dict(d)) = descendants.get_mut(0) { + d.set(b"CIDToGIDMap".to_vec(), map); + } + f.set(b"DescendantFonts".to_vec(), Object::Array(descendants)); + f.set(b"ToUnicode".to_vec(), unicode); + let m = Metrics::from_dict(&doc, &f); + (m.outlined_cids.clone(), m.text_of(1).map(Cow::into_owned)) + }; + let direct = read(false); + assert!(!direct.0.is_empty()); + assert_eq!(direct.1.as_deref(), Some("X")); + assert_eq!(read(true), direct); + let mut program = include_bytes!("../../../../assets/fonts/Inter-Regular.ttf").to_vec(); + program.resize(program.len().next_multiple_of(4), 0); + for by_reference in [false, true] { + let mut doc = Document::new_empty(); + let file = predicted(&mut doc, &program, 4, by_reference); + let mut descriptor = Dict::new(); + descriptor.set(b"FontFile2".to_vec(), file); + let simple = font(&mut doc, vec![("FontDescriptor", Object::Dict(descriptor))]); + assert_eq!(embedded_sfnt(&doc, &simple).as_deref(), Some(program.as_slice()), "by reference: {by_reference}"); + } +} + /// Fonts that each name one large ToUnicode share the allowance they are read within: it pays for /// as many readings as it holds, and the rest are left unread, where each reading used to decode /// it afresh. @@ -732,7 +781,7 @@ fn malformed_cmaps_never_panic() { let mut enc = Dict::new(); enc.set(b"Differences".to_vec(), Object::Array(vec![Object::Int(i64::MAX), Object::name("A"), Object::name("B")])); let f = font(&mut doc, vec![("Subtype", Object::name("Type1")), ("Encoding", Object::Dict(enc))]); - assert_eq!(unicode_map(&doc, &f, false, &mut Reads::new(&mut DecodeBudget::new(usize::MAX, 0))).text(u32::MAX).as_deref(), Some("B")); + assert_eq!(unicode_map(&doc, &f, false, &mut Reads::new(&doc, &mut DecodeBudget::new(usize::MAX, 0))).text(u32::MAX).as_deref(), Some("B")); } #[test] From 02360536a7d8edb3bd306a33d78e1b158a08f8c2 Mon Sep 17 00:00:00 2001 From: Grzegorz Martens Date: Thu, 5 Nov 2026 03:22:45 +0100 Subject: [PATCH 04/16] M7: tests and docs say what the font reads do The Claims review of fd4b698c7: - The predictor test's rows are tagged Up, not None: read without the predictor, None rows leave only zero bytes, which a CMap's parser skips, so its ToUnicode part passed without the fix. - embedded_sfnt_within's comment and its test's doc gave guessed meanings as the reason no copy is adapted; it is the metrics read past the allowance, the widths' read of the program again included. - The full-map test is named for the filters it covers, and the docs of Reads::decode, MAX_CMAP_BYTES and the own-limit test carry the conditions they hold under. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AmsdeUCdTaD72jkDw2pJKY --- crates/fonts/src/pdf.rs | 9 +++++---- crates/fonts/src/pdf/tests.rs | 29 ++++++++++++++++++----------- 2 files changed, 23 insertions(+), 15 deletions(-) diff --git a/crates/fonts/src/pdf.rs b/crates/fonts/src/pdf.rs index e526f2a8e6..c0ae9e8765 100644 --- a/crates/fonts/src/pdf.rs +++ b/crates/fonts/src/pdf.rs @@ -128,7 +128,8 @@ pub fn embedded_sfnt_within(doc: &Document, font: &Dict, budget: &mut DecodeBudg // only this in-memory copy gets a new cmap, never the working PDF or a font asset. let Ok(cmap) = face.cmap() else { continue }; // A composite font's codes aren't these, so its metrics aren't read (Metrics::composite - // is this test). Meanings read past the allowance would be guesses: no copy is adapted. + // is this test). Metrics read past the allowance, its widths' read of the program again + // included, adapt no copy. if font.name(b"Subtype") == Some(b"Type0") { continue; } @@ -297,7 +298,7 @@ const MAX_CMAP_ENTRIES: usize = 1 << 20; /// thousand. const MAX_CMAP_RANGES: usize = 1 << 16; /// Largest decoded Encoding or ToUnicode CMap stream read; real CMaps stay well under it, and a larger -/// one is not decoded at all. +/// one is left unread (behind one filter not decoded past it; behind several, see `Reads::decode`). const MAX_CMAP_BYTES: usize = 16 << 20; /// Largest decoded `/CIDToGIDMap` stream read: two bytes for each CID below 65,536. const MAX_CID_TO_GID: usize = 1 << 17; @@ -321,8 +322,8 @@ impl<'a> Reads<'a> { /// /// `max` bounds what the stream decodes to, and its filter stages together may make up to `max` /// for each of them, as much as a decode without an allowance lets them: a map of 65,536 CIDs - /// behind ASCIIHex and Flate makes all of `MAX_CID_TO_GID` in its last stage, with the hex - /// stage's output on top. One stage may so make more than `max`, which a decode without an + /// behind ASCIIHex and Flate, its Flate data smaller than the map, makes all of `MAX_CID_TO_GID` + /// in its last stage, with the hex stage's output on top. One stage may so make more than `max`, which a decode without an /// allowance stops, so a stream that one leaves unread can read here, never the other way /// round. A stream past `max` is unread, not refused, when what is left can pay for its stages; /// when it can't, the stream spends it and counts as refused. diff --git a/crates/fonts/src/pdf/tests.rs b/crates/fonts/src/pdf/tests.rs index 3e914816bb..e770cd60c2 100644 --- a/crates/fonts/src/pdf/tests.rs +++ b/crates/fonts/src/pdf/tests.rs @@ -199,8 +199,8 @@ fn mac_subset_cmap_uses_pdf_unicode_without_changing_source_program() { } /// The metrics read that adapts a program's cmap is paid for from the editor's allowance too: a -/// step short of the program, its ToUnicode and the program again for its widths, no copy is -/// adapted from guessed meanings. +/// step short of the program, its ToUnicode and the program again for its widths, the metrics are +/// read past the allowance and no copy is adapted. #[test] fn adapting_a_programs_cmap_is_paid_for() { use skrifa::MetadataProvider as _; @@ -282,8 +282,8 @@ fn a_fonts_streams_are_paid_for_from_its_allowance() { assert_eq!(cost - budget.remaining(), sizes[0] + sizes[3]); } -/// A stream past its own limit is left unread as it always was, paying for the limit, and is no -/// reason to call the font read past the allowance. +/// A stream behind one filter past its own limit, with more left than its limit, is left unread as +/// it always was, paying for the limit, and is no reason to call the font read past the allowance. #[test] fn a_cmap_past_its_own_limit_is_not_past_the_allowance() { let mut doc = Document::new_empty(); @@ -297,11 +297,11 @@ fn a_cmap_past_its_own_limit_is_not_past_the_allowance() { assert_eq!(usize::MAX / 2 - budget.remaining(), sizes[..3].iter().sum::() + MAX_CMAP_BYTES); } -/// A `/CIDToGIDMap` as long as a map can be reads the same behind no filter, one or two: each -/// stage may make as much as the map's limit, so the hex stage's output on top of it doesn't -/// leave it unread. A map past the limit is unread behind any of them. +/// A `/CIDToGIDMap` as long as a map can be, which compresses well, reads the same behind no +/// filter, Flate, or ASCIIHex and Flate: each stage may make as much as the map's limit, so the hex +/// stage's output on top of it doesn't leave it unread. A map past the limit is unread behind each. #[test] -fn a_full_cid_map_reads_behind_any_number_of_filters() { +fn a_full_cid_map_reads_behind_one_or_two_filters() { let read = |entries: usize, filters: usize| { let mut doc = Document::new_empty(); let (mut f, _) = subset_type0(&mut doc, b""); @@ -334,10 +334,17 @@ fn a_full_cid_map_reads_behind_any_number_of_filters() { } } -/// `data` in rows of `columns` bytes behind Flate and a PNG predictor, each row tagged None, with -/// its `/DecodeParms` given by reference or direct. +/// `data` in rows of `columns` bytes behind Flate and a PNG predictor, each row tagged Up (the +/// difference from the row above), so read without the predictor it is garbled; with its +/// `/DecodeParms` given by reference or direct. fn predicted(doc: &mut Document, data: &[u8], columns: usize, by_reference: bool) -> Object { - let rows: Vec = data.chunks(columns).flat_map(|row| std::iter::once(0).chain(row.iter().copied())).collect(); + let mut above = vec![0u8; columns]; + let mut rows = Vec::new(); + for row in data.chunks(columns) { + rows.push(2); + rows.extend(row.iter().zip(&above).map(|(b, a)| b.wrapping_sub(*a))); + above[..row.len()].copy_from_slice(row); + } let mut s = Stream::flate(Dict::new(), &rows); let parms: Dict = [(b"Predictor".to_vec(), Object::Int(12)), (b"Columns".to_vec(), Object::Int(i64::try_from(columns).unwrap()))].into_iter().collect(); From fb01bbfe771f3d5f06d1a6f202011ae12d36d870 Mon Sep 17 00:00:00 2001 From: Grzegorz Martens Date: Sat, 7 Nov 2026 03:27:33 +0100 Subject: [PATCH 05/16] M7: the editor's program read takes its limit once for each filter stage The editor's read of a font program bounded all its filter stages together by the program's 16 MiB limit, as main does: a 12 MiB program behind ASCIIHex and Flate, its Flate data about 8 MiB, was left unread, though behind Flate alone it reads. It now takes the limit once for each stage, as the metrics reads do, which the stages share and any one of them may use, and then holds the program itself to 16 MiB. Behind eight filters a read may so decode up to 128 MiB, where main stopped at 16 MiB. Once the allowance is spent, the metrics reads refuse a stream with data, and the editor's program read passes a program over, before resolving its /DecodeParms, as the decode itself would refuse it, so each font left doesn't pay for that work the allowance doesn't count. What is read and what is refused are as before, an empty stream still read rather than refused. Tests: an_editors_program_reads_behind_ascii_hex_and_flate_within_its_limit, and an_empty_stream_reads_with_nothing_left. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AmsdeUCdTaD72jkDw2pJKY --- crates/fonts/src/pdf.rs | 21 ++++++++++++- crates/fonts/src/pdf/tests.rs | 58 +++++++++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+), 1 deletion(-) diff --git a/crates/fonts/src/pdf.rs b/crates/fonts/src/pdf.rs index c0ae9e8765..b960909bfb 100644 --- a/crates/fonts/src/pdf.rs +++ b/crates/fonts/src/pdf.rs @@ -115,7 +115,20 @@ pub fn embedded_sfnt_within(doc: &Document, font: &Dict, budget: &mut DecodeBudg let Some(object) = descriptor.get(key) else { continue }; let object = doc.resolve(object); let Object::Stream(stream) = object.as_ref() else { continue }; - let Ok(mut bytes) = budget.decode_within(&doc.with_direct_parms(stream), MAX_SFNT) else { continue }; + // As in `Reads::decode`: once the allowance is spent the decode refuses a program with + // data, so it is passed over before its parameters are resolved, work the allowance doesn't + // count; an empty one is no program either way. + if budget.remaining() == 0 { + continue; + } + let stream = doc.with_direct_parms(stream); + // As in `Reads::decode`: the stages share the limit once for each of them, which any one + // may use, and the program itself must be within it. + let stages = stream.filters().len().max(1); + let Ok(mut bytes) = budget.decode_within(&stream, MAX_SFNT.saturating_mul(stages)) else { continue }; + if bytes.len() > MAX_SFNT { + continue; + } let Ok(face) = skrifa::FontRef::from_index(&bytes, 0) else { continue }; if face.metrics(skrifa::instance::Size::unscaled(), skrifa::instance::LocationRef::default()).units_per_em == 0 { continue; @@ -328,6 +341,12 @@ impl<'a> Reads<'a> { /// round. A stream past `max` is unread, not refused, when what is left can pay for its stages; /// when it can't, the stream spends it and counts as refused. fn decode(&mut self, stream: &Stream, max: usize) -> Option> { + // Once the allowance is spent the decode refuses the stream; refused here, before the + // parameters are resolved, which is work the allowance doesn't count. + if self.budget.remaining() == 0 && !stream.raw.is_empty() { + self.refused = true; + return None; + } let stream = self.doc.with_direct_parms(stream); let stages = stream.filters().len().max(1); match self.budget.decode_within(&stream, max.saturating_mul(stages)) { diff --git a/crates/fonts/src/pdf/tests.rs b/crates/fonts/src/pdf/tests.rs index e770cd60c2..301d90be77 100644 --- a/crates/fonts/src/pdf/tests.rs +++ b/crates/fonts/src/pdf/tests.rs @@ -282,6 +282,20 @@ fn a_fonts_streams_are_paid_for_from_its_allowance() { assert_eq!(cost - budget.remaining(), sizes[0] + sizes[3]); } +/// With nothing left, an empty stream still reads, as the allowance reads one, and is no reason to +/// call the font read past it; a stream with data is left unread, and is. +#[test] +fn an_empty_stream_reads_with_nothing_left() { + let past = |data: &[u8]| { + let mut doc = Document::new_empty(); + let unicode = Object::Ref(doc.add(Object::Stream(Stream::from_raw(Dict::new(), data.to_vec())))); + let f = font(&mut doc, vec![("Subtype", Object::name("TrueType")), ("ToUnicode", unicode)]); + Metrics::from_dict_within(&doc, &f, &mut DecodeBudget::new(0, 0)).past_allowance + }; + assert!(!past(b"")); + assert!(past(b"1 beginbfchar <41> <0058> endbfchar")); +} + /// A stream behind one filter past its own limit, with more left than its limit, is left unread as /// it always was, paying for the limit, and is no reason to call the font read past the allowance. #[test] @@ -334,6 +348,50 @@ fn a_full_cid_map_reads_behind_one_or_two_filters() { } } +/// The editor's program is held to its limit as the metrics reads' streams are: a 12 MiB program +/// whose Flate data is about 8 MiB reads behind ASCIIHex and Flate as behind Flate alone, though +/// the two stages make about 20 MiB together. A program of the limit reads behind either, and one a +/// byte past it is unread behind either. +#[test] +fn an_editors_program_reads_behind_ascii_hex_and_flate_within_its_limit() { + let program = |len: usize, noise: usize| { + let mut program = include_bytes!("../../../../assets/fonts/Inter-Regular.ttf").to_vec(); + // Bytes Flate can't shrink, then zeros. + let mut x = 1u32; + program.extend((0..noise).map(|_| { + x = x.wrapping_mul(1_103_515_245).wrapping_add(12_345); + x.to_be_bytes()[0] + })); + program.resize(len, 0); + program + }; + let read = |program: &[u8], hex: bool| { + let mut doc = Document::new_empty(); + let flate = Stream::flate(Dict::new(), program); + let stream = if hex { + let digits = b"0123456789ABCDEF"; + let mut data: Vec = flate.raw.iter().flat_map(|b| [digits[usize::from(b >> 4)], digits[usize::from(b & 15)]]).collect(); + data.push(b'>'); + let mut dict = Dict::new(); + dict.set(b"Filter".to_vec(), Object::Array(vec![Object::name("ASCIIHexDecode"), Object::name("FlateDecode")])); + Stream::from_raw(dict, data) + } else { + flate + }; + let mut descriptor = Dict::new(); + descriptor.set(b"FontFile2".to_vec(), Object::Ref(doc.add(Object::Stream(stream)))); + let simple = font(&mut doc, vec![("FontDescriptor", Object::Dict(descriptor))]); + embedded_sfnt(&doc, &simple) + }; + let near = program(12 << 20, 8 << 20); + let full = program(MAX_SFNT, 0); + for hex in [false, true] { + assert_eq!(read(&near, hex).as_deref(), Some(near.as_slice()), "behind ASCIIHex too: {hex}"); + assert_eq!(read(&full, hex).as_deref(), Some(full.as_slice()), "behind ASCIIHex too: {hex}"); + assert!(read(&program(MAX_SFNT + 1, 0), hex).is_none(), "behind ASCIIHex too: {hex}"); + } +} + /// `data` in rows of `columns` bytes behind Flate and a PNG predictor, each row tagged Up (the /// difference from the row above), so read without the predictor it is garbled; with its /// `/DecodeParms` given by reference or direct. From 6f5f36136817b16a6162e910e2db39a8d2d00eef Mon Sep 17 00:00:00 2001 From: FredApps Date: Sun, 8 Nov 2026 14:39:32 +0100 Subject: [PATCH 06/16] Refuse encoding unverified glyphs after a font decode allowance refusal --- crates/fonts/src/pdf.rs | 8 +++++++- crates/fonts/src/pdf/tests.rs | 23 +++++++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/crates/fonts/src/pdf.rs b/crates/fonts/src/pdf.rs index 7972856188..2627e47330 100644 --- a/crates/fonts/src/pdf.rs +++ b/crates/fonts/src/pdf.rs @@ -979,8 +979,14 @@ impl Metrics { } /// The bytes that show `text` in this font, or `None` if some character has no code or - /// no glyph in it (the caller then substitutes another font). + /// no glyph in it, or the metrics were read past their allowance (the caller then substitutes + /// another font). Empty text needs no verified glyph and always encodes. pub fn encode(&self, text: &str) -> Option> { + // A refused read cannot establish the mappings or glyphs needed to write new text. + // Empty text needs neither and remains usable for erasing existing text. + if self.past_allowance && !text.is_empty() { + return None; + } // The lowest code showing each wanted character. Explicit entries and the encoding are // checked one by one; each range is asked for the wanted characters it can show. let wanted: HashSet = text.chars().collect(); diff --git a/crates/fonts/src/pdf/tests.rs b/crates/fonts/src/pdf/tests.rs index a334ecba6a..f9cd7ec761 100644 --- a/crates/fonts/src/pdf/tests.rs +++ b/crates/fonts/src/pdf/tests.rs @@ -1173,3 +1173,26 @@ fn low_codes_resolve_to_the_covering_range() { assert_eq!(covered, [5, 6, 7, 100, 101]); assert_eq!((u.covering(0xFFFF).is_some(), u.covering(0x1_0001).is_some(), u.covering(0x1_0002).is_none()), (true, true, true)); } + +#[test] +fn refused_subset_reads_do_not_encode_unverified_glyphs() { + let mut doc = Document::new_empty(); + let (mut f, sizes) = subset_type0(&mut doc, b""); + f.set(b"Encoding".to_vec(), Object::name("Identity-H")); + f.set(b"ToUnicode".to_vec(), Object::name("Identity-H")); + if let Some(Object::Array(descendants)) = f.get_mut(b"DescendantFonts") + && let Some(Object::Dict(descendant)) = descendants.first_mut() + { + descendant.set(b"DW".to_vec(), Object::Int(1000)); + } + let whole = Metrics::from_dict(&doc, &f); + assert!(!whole.past_allowance && !whole.outlined_cids.is_empty()); + assert!(whole.encode("\u{ffff}").is_none()); + // Refuse the program, then the CID map after reading the complete program. + for allowance in [0, sizes[1]] { + let partial = Metrics::from_dict_within(&doc, &f, &mut DecodeBudget::new(allowance, 0)); + assert!(partial.past_allowance); + assert_eq!(partial.encode(""), Some(Vec::new())); + assert!(partial.encode("\u{ffff}").is_none(), "allowance {allowance} must not turn /DW into proof of a glyph"); + } +} From 5b204d595b9b419428e95de70594824e6b0eb241 Mon Sep 17 00:00:00 2001 From: FredApps Date: Sun, 8 Nov 2026 14:39:34 +0100 Subject: [PATCH 07/16] Refuse ruled-line extraction after incomplete page content --- crates/edit/src/rules.rs | 4 ++++ crates/edit/src/tests.rs | 18 ++++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/crates/edit/src/rules.rs b/crates/edit/src/rules.rs index a5bcfebbf1..ef3f78f772 100644 --- a/crates/edit/src/rules.rs +++ b/crates/edit/src/rules.rs @@ -26,6 +26,10 @@ pub fn page_rules(doc: &Document, page: usize) -> Result, EditErro let pages = pdfcraft_model::pages(doc); let p = pages.get(page).ok_or(EditError::NoSuchPage(page))?; let streams = crate::text::content_streams(doc, &p.dict)?; + // Unread content can change the graphics state used by every later stream. + if streams.iter().any(|stream| stream.read != crate::text::Read::Whole) { + return Err(EditError::Invalid("page content could not be completely decoded; cannot extract its ruled lines safely".into())); + } let pieces = Pieces::join(&streams.iter().map(|stream| stream.data.as_slice()).collect::>()); let ops = pieces.parse().ops; diff --git a/crates/edit/src/tests.rs b/crates/edit/src/tests.rs index 7c8f65dbab..8bc7e9e437 100644 --- a/crates/edit/src/tests.rs +++ b/crates/edit/src/tests.rs @@ -11202,3 +11202,21 @@ fn review_inside_scope_group_move_matches_direct_transform_under_outer_scale() { } } } + +#[test] +fn page_rules_refuse_unread_graphics_state() { + let mut dict = Dict::new(); + dict.set(b"Filter".to_vec(), Object::name("UnsupportedFilter")); + let unread = Stream::from_raw(dict, b"2 0 0 2 0 0 cm".to_vec()); + let damaged = damaged_flate(b"0 g\n", b"2 0 0 2 0 0 cm"); + for stream in [unread, damaged] { + let mut doc = image_page_with("10 10 m 100 10 l S"); + let incomplete = doc.add(Object::Stream(stream)); + doc.update_dict(ObjRef::new(3, 0), |page| { + page.set(b"Contents".to_vec(), Object::Array(vec![Object::Ref(incomplete), Object::Ref(ObjRef::new(4, 0))])); + }) + .unwrap(); + assert!(page_rules(&doc, 0).is_err(), "later geometry cannot be trusted after unread graphics state"); + } + assert_eq!(page_rules(&image_page_with("10 10 m 100 10 l S"), 0).unwrap(), [[10.0, 10.0, 100.0, 10.0]]); +} From 063f8629e4c1364a1e10733bcc1d49a2a9f2d074 Mon Sep 17 00:00:00 2001 From: Grzegorz Martens Date: Mon, 9 Nov 2026 13:36:29 +0100 Subject: [PATCH 08/16] M7: each filter stage of a font's stream is held to its limit, as without an allowance The Resources review of 8eead8ecd. The program read and the metrics reads passed their limit times the number of filter stages as one total, which any one stage could use: a FontFile2 of Flate making 128 MiB of whitespace, under seven ASCIIHex stages (a 130 KB file), made all of it before being refused, about 0.1 s a font where `main`'s bound stopped it at 16 MiB. `edit`'s `matching_thai_outline` reads such a program for each of a page's fonts with no total, and every `Metrics::from_dict` up to three times. Now each stage is held to the limit, as a decode without an allowance holds it, and the stages together to the limit for each of them, so every stream the stage scaling was for still reads: a full `/CIDToGIDMap` or a 12 MiB program behind ASCIIHex and Flate. The two bounds take an entry point of their own: `pdfcraft_filters::decode_tolerant_counted_each`, which `run` already supports, and `DecodeBudget::decode_stages_within`, which `Reads::decode` and the editor's program read use. A stage past the limit is charged what the stages before it decoded and the limit; past the stages' total, or past what is left, it is charged as `decode_within` charges. The program, the last stage, is so within the limit without a check of its own. Tests: `each_stage_is_held_to_its_own_bound_within_the_total` in `pdfcraft-filters` (a Flate stage stopped at its bound hands the next stage nothing), `each_stage_is_held_to_the_limit` in `pdfcraft-cos`, and in `pdfcraft-fonts` `an_editors_program_read_holds_each_stage_to_its_limit` and `each_stage_of_a_metrics_stream_is_held_to_its_limit`. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AmsdeUCdTaD72jkDw2pJKY --- crates/cos/src/budget.rs | 78 +++++++++++++++++++++++++++++++++++ crates/cos/src/object.rs | 20 +++++++++ crates/filters/src/lib.rs | 36 ++++++++++++++++ crates/fonts/src/pdf.rs | 31 +++++--------- crates/fonts/src/pdf/tests.rs | 50 ++++++++++++++++++++++ 5 files changed, 195 insertions(+), 20 deletions(-) diff --git a/crates/cos/src/budget.rs b/crates/cos/src/budget.rs index 56286ce4a0..f456583732 100644 --- a/crates/cos/src/budget.rs +++ b/crates/cos/src/budget.rs @@ -283,6 +283,45 @@ impl DecodeBudget { self.charge(stream, max, Stream::decoded_within_counted).map(|(data, _)| data) } + /// Like [`DecodeBudget::decode_within`], with every filter stage held to `max`, as + /// [`Stream::decoded_within`] holds it, and the stages together to `max` for each of them. A + /// stream so reads here when it reads without an allowance and the allowance can pay for + /// what its stages decode, the output of one stage on top of another's. A stage past `max` + /// is an error that charges what the stages before it decoded and `max`, the work it took to + /// find out; past the stages' total it charges that total, and past what is left it spends + /// all of it, as [`DecodeBudget::decode_within`] does. + pub fn decode_stages_within(&mut self, stream: &Stream, max: usize) -> Result, CosError> { + if self.bytes == 0 && !stream.raw.is_empty() { + return Err(CosError::Filter(FilterError::LimitExceeded(self.limit).to_string())); + } + let total = max.saturating_mul(stream.filters().len().max(1)); + let room = total.min(self.bytes); + match stream.decoded_within_each_counted(max, room) { + Ok((data, _, spent)) => { + self.bytes = self.bytes.saturating_sub(spent); + Ok(data) + } + // A stage stopped at `max`, below what was left of the room. + Err((FilterError::LimitExceeded(_), spent)) if room.saturating_sub(spent) > max => { + self.bytes = self.bytes.saturating_sub(spent.saturating_add(max)); + Err(CosError::Filter(FilterError::LimitExceeded(max).to_string())) + } + // At the stages' total, with more left. + Err((FilterError::LimitExceeded(_), _)) if total < self.bytes => { + self.bytes = self.bytes.saturating_sub(total); + Err(CosError::Filter(FilterError::LimitExceeded(total).to_string())) + } + Err((FilterError::LimitExceeded(_), _)) => { + self.bytes = 0; + Err(CosError::Filter(FilterError::LimitExceeded(self.limit).to_string())) + } + Err((e, spent)) => { + self.bytes = self.bytes.saturating_sub(spent); + Err(CosError::Filter(e.to_string())) + } + } + } + /// [`DecodeBudget::decode_within`] (`usize::MAX` for no bound of its own), with the stream's /// `/DecodeParms` given by reference read in `doc`, also saying whether the stream is damaged: /// `true` when a filter stopped at corrupt data, so the bytes are only what decoded before it. @@ -1025,4 +1064,43 @@ mod tests { assert!(budget.decode_strict(&Document::new_empty(), &chained).is_ok()); assert_eq!(budget.remaining(), (10 << 20) - 3 * charged); } + + /// `decode_stages_within` holds every stage to the limit, as a decode without an allowance + /// does, and the stages together to the limit for each of them. A stream behind ASCIIHex and + /// Flate whose stages each stay within it reads, the hex stage's output charged on top; one + /// whose Flate stage would make more stops there, charged what the hex stage made and the + /// limit, the rest of the allowance left; a Flate stage making hex text past the limit leaves + /// the stream unread though the total has room, as `Stream::decoded_within` leaves it; and a + /// stream past what is left spends all of it. + #[test] + fn each_stage_is_held_to_the_limit() { + let chained = |data: &[u8], filters: [&str; 2]| { + let mut dict = Dict::new(); + dict.set(b"Filter".to_vec(), Object::Array(filters.into_iter().map(Object::name).collect())); + Stream::from_raw(dict, data.to_vec()) + }; + let hexed = |data: &[u8]| { + let flate = Stream::flate(Dict::new(), data); + let hex: Vec = flate.raw.iter().flat_map(|b| format!("{b:02X}").into_bytes()).chain(*b">").collect(); + (chained(&hex, ["ASCIIHexDecode", "FlateDecode"]), flate.raw.len()) + }; + let max = 1 << 16; + let (fits, inner) = hexed(&vec![0u8; max]); + let mut budget = DecodeBudget::new(1 << 20, 0); + assert_eq!(budget.decode_stages_within(&fits, max).map(|d| d.len()).ok(), Some(max)); + assert_eq!(budget.remaining(), (1 << 20) - inner - max); + let (bomb, inner) = hexed(&vec![0u8; 8 * max]); + let mut budget = DecodeBudget::new(1 << 20, 0); + assert!(budget.decode_stages_within(&bomb, max).is_err()); + assert_eq!(budget.remaining(), (1 << 20) - inner - max, "the Flate stage stopped at the limit"); + // Flate making 1.2 times the limit of hex text, which ASCIIHex halves: the stages together + // make 1.8 times the limit, within twice it, but the Flate stage alone is past it. + let text = "00".repeat(max * 6 / 10).into_bytes(); + let wide = chained(&Stream::flate(Dict::new(), &text).raw, ["FlateDecode", "ASCIIHexDecode"]); + assert!(wide.decoded_within(max).is_err()); + assert!(DecodeBudget::new(1 << 20, 0).decode_stages_within(&wide, max).is_err()); + let mut budget = DecodeBudget::new(max, 0); + assert!(budget.decode_stages_within(&fits, max).is_err()); + assert_eq!(budget.remaining(), 0); + } } diff --git a/crates/cos/src/object.rs b/crates/cos/src/object.rs index f40f175d69..f5c25091fe 100644 --- a/crates/cos/src/object.rs +++ b/crates/cos/src/object.rs @@ -382,6 +382,26 @@ impl Stream { self.decode_chain(max, decode).map(|(v, spent)| (v, damaged, spent)) } + /// [`Stream::decoded_within_counted`] with every filter stage also held to `each`, as + /// [`Stream::decoded_within`] holds it, while `max` bounds the stages together + /// ([`pdfcraft_filters::decode_tolerant_counted_each`]). Without filters the data is held to + /// both. + pub(crate) fn decoded_within_each_counted( + &self, + each: usize, + max: usize, + ) -> Result<(Vec, bool, usize), (pdfcraft_filters::FilterError, usize)> { + let mut damaged = false; + let decode = |chain: &[(Filter, Params)], data: &[u8], max| { + pdfcraft_filters::decode_tolerant_counted_each(chain, data, each.min(MAX_DECODED), max).map(|(v, partial, spent)| { + damaged = partial; + (v, spent) + }) + }; + let max = if self.readable_filters().is_ok_and(|chain| chain.is_empty()) { max.min(each) } else { max }; + self.decode_chain(max, decode).map(|(v, spent)| (v, damaged, spent)) + } + /// [`Stream::decoded_within`] spending from `work`, what an operation may still do across a /// document's images ([`pdfcraft_filters::decode_tolerant_within`]). pub(crate) fn decoded_spending(&self, max: usize, work: &mut u64) -> Result, CosError> { diff --git a/crates/filters/src/lib.rs b/crates/filters/src/lib.rs index 9342213577..4bee6b451c 100644 --- a/crates/filters/src/lib.rs +++ b/crates/filters/src/lib.rs @@ -212,6 +212,18 @@ pub fn decode_tolerant_counted(chain: &[(Filter, Params)], data: &[u8], max_outp run(chain, data, max_output, Some(max_output), false, false, None) } +/// [`decode_tolerant_counted`] with the two bounds apart: `max_stage` bounds every stage, as +/// [`decode_tolerant`]'s bound does, and `max_output` the bytes all stages decode together. A +/// stage stopped by either gives [`FilterError::LimitExceeded`] and the bytes decoded before it. +pub fn decode_tolerant_counted_each( + chain: &[(Filter, Params)], + data: &[u8], + max_stage: usize, + max_output: usize, +) -> Result<(Vec, bool, usize), (FilterError, usize)> { + run(chain, data, max_stage, Some(max_output), false, false, None) +} + /// [`decode_tolerant`], spending from `work`, what decoding may still cost across a run of images: /// a step for each byte of `data`, paid before anything is decoded, [`FLATE_READ_WORK`] for each /// byte a Flate stage reads, paid before it runs (again if it reads them twice), and one for each @@ -451,6 +463,30 @@ mod tests { assert_eq!(super::decode(&chain(super::MAX_CHAIN), &data, 1 << 20).ok(), Some(data)); } + /// [`decode_tolerant_counted_each`] holds every stage to its own bound as well as the stages + /// to their total: a Flate stage that would make more than that bound stops there, though + /// the total has room, and the stage after it gets nothing; within both, the chain reads + /// whole, one stage's output charged on top of another's. + #[test] + fn each_stage_is_held_to_its_own_bound_within_the_total() { + let hex = "00".repeat(1 << 16).into_bytes(); + let data = super::flate::encode(&hex); + let chain = [(Filter::Flate, Params::default()), (Filter::AsciiHex, Params::default())]; + // The total alone lets the Flate stage make all of the hex text, and the hex stage read it. + STAGE_BYTES.with(|c| c.set(0)); + let whole = decode_tolerant_counted(&chain, &data, 3 << 16).map(|(v, _, _)| v); + assert_eq!(whole.ok(), Some(vec![0u8; 1 << 16])); + assert_eq!(STAGE_BYTES.with(|c| c.get()), data.len() + hex.len()); + // Each stage held to 64 KiB, the Flate stage stops there and the hex stage gets nothing. + STAGE_BYTES.with(|c| c.set(0)); + let held = decode_tolerant_counted_each(&chain, &data, 1 << 16, 3 << 16); + assert!(matches!(held, Err((FilterError::LimitExceeded(m), 0)) if m == 1 << 16), "{held:?}"); + assert_eq!(STAGE_BYTES.with(|c| c.get()), data.len()); + // Within both, it reads whole. + let (out, _, spent) = decode_tolerant_counted_each(&chain, &data, 2 << 16, 3 << 16).unwrap(); + assert_eq!((out.len(), spent), (1 << 16, hex.len() + (1 << 16))); + } + use super::*; #[test] diff --git a/crates/fonts/src/pdf.rs b/crates/fonts/src/pdf.rs index 024d59f937..c8f34bd1c4 100644 --- a/crates/fonts/src/pdf.rs +++ b/crates/fonts/src/pdf.rs @@ -122,14 +122,8 @@ pub fn embedded_sfnt_within(doc: &Document, font: &Dict, budget: &mut DecodeBudg if budget.remaining() == 0 { continue; } - let stream = doc.with_direct_parms(stream); - // As in `Reads::decode`: the stages share the limit once for each of them, which any one - // may use, and the program itself must be within it. - let stages = stream.filters().len().max(1); - let Ok(mut bytes) = budget.decode_within(&stream, MAX_SFNT.saturating_mul(stages)) else { continue }; - if bytes.len() > MAX_SFNT { - continue; - } + // As in `Reads::decode`: each stage within the limit, the program, the last, too. + let Ok(mut bytes) = budget.decode_stages_within(&doc.with_direct_parms(stream), MAX_SFNT) else { continue }; let Ok(face) = skrifa::FontRef::from_index(&bytes, 0) else { continue }; if face.metrics(skrifa::instance::Size::unscaled(), skrifa::instance::LocationRef::default()).units_per_em == 0 { continue; @@ -402,13 +396,13 @@ impl<'a> Reads<'a> { /// by reference too ([`Document::with_direct_parms`]); `None` when it can't be, noting when the /// allowance is why. /// - /// `max` bounds what the stream decodes to, and its filter stages together may make up to `max` - /// for each of them, as much as a decode without an allowance lets them: a map of 65,536 CIDs - /// behind ASCIIHex and Flate, its Flate data smaller than the map, makes all of `MAX_CID_TO_GID` - /// in its last stage, with the hex stage's output on top. One stage may so make more than `max`, which a decode without an - /// allowance stops, so a stream that one leaves unread can read here, never the other way - /// round. A stream past `max` is unread, not refused, when what is left can pay for its stages; - /// when it can't, the stream spends it and counts as refused. + /// Each of its filter stages is held to `max`, as a decode without an allowance holds it, and + /// the stages together to `max` for each of them ([`DecodeBudget::decode_stages_within`]): a + /// map of 65,536 CIDs behind ASCIIHex and Flate, its Flate data smaller than the map, makes + /// all of `MAX_CID_TO_GID` in its last stage, with the hex stage's output on top. So a stream + /// reads here when it reads without an allowance and what is left pays for it. A stream past + /// `max` is unread, not refused, when what is left can pay for its stages; when it can't, the + /// stream spends it and counts as refused. fn decode(&mut self, stream: &Stream, max: usize) -> Option> { // Once the allowance is spent the decode refuses the stream; refused here, before the // parameters are resolved, which is work the allowance doesn't count. @@ -416,11 +410,8 @@ impl<'a> Reads<'a> { self.refused = true; return None; } - let stream = self.doc.with_direct_parms(stream); - let stages = stream.filters().len().max(1); - match self.budget.decode_within(&stream, max.saturating_mul(stages)) { - Ok(data) if data.len() <= max => Some(data), - Ok(_) => None, + match self.budget.decode_stages_within(&self.doc.with_direct_parms(stream), max) { + Ok(data) => Some(data), Err(_) => { self.refused |= self.budget.remaining() == 0; None diff --git a/crates/fonts/src/pdf/tests.rs b/crates/fonts/src/pdf/tests.rs index 6acc6dedd1..53d43f0c72 100644 --- a/crates/fonts/src/pdf/tests.rs +++ b/crates/fonts/src/pdf/tests.rs @@ -374,6 +374,56 @@ fn a_full_cid_map_reads_behind_one_or_two_filters() { } } +/// The editor's program read holds every filter stage to the program's limit: a program behind +/// ASCIIHex and Flate whose Flate stage would make 24 MiB stops at 16 MiB, charged that and what +/// the hex stage made, where the stages' total alone would let it make all 24 MiB. A program +/// within the limit behind the same two filters reads (see also +/// `an_editors_program_reads_behind_ascii_hex_and_flate_within_its_limit`). +#[test] +fn an_editors_program_read_holds_each_stage_to_its_limit() { + let mut doc = Document::new_empty(); + let flate = Stream::flate(Dict::new(), &vec![b' '; 24 << 20]); + let hex: Vec = flate.raw.iter().flat_map(|b| format!("{b:02X}").into_bytes()).chain(*b">").collect(); + let mut dict = Dict::new(); + dict.set(b"Filter".to_vec(), Object::Array(vec![Object::name("ASCIIHexDecode"), Object::name("FlateDecode")])); + let program = doc.add(Object::Stream(Stream::from_raw(dict, hex))); + let mut descriptor = Dict::new(); + descriptor.set(b"FontFile2".to_vec(), Object::Ref(program)); + let f = font(&mut doc, vec![("Subtype", Object::name("TrueType")), ("FontDescriptor", Object::Dict(descriptor))]); + let mut budget = DecodeBudget::new(usize::MAX / 2, 0); + assert!(embedded_sfnt_within(&doc, &f, &mut budget).is_none()); + assert_eq!(usize::MAX / 2 - budget.remaining(), flate.raw.len() + MAX_SFNT); +} + +/// Each stage of a stream a font's metrics are read from is held to the stream's limit, as a +/// decode without an allowance holds it: a `/CIDToGIDMap` behind Flate and ASCIIHex whose Flate +/// stage makes hex text past the map's limit is unread, as `Stream::decoded_within` leaves it, +/// though the two stages together make less than twice the limit; with its hex text within the +/// limit it reads. +#[test] +fn each_stage_of_a_metrics_stream_is_held_to_its_limit() { + let read = |cids: usize| { + let mut doc = Document::new_empty(); + let (mut f, _) = subset_type0(&mut doc, b""); + let map: Vec = (0..cids).flat_map(|cid| u16::try_from(cid % 2000).unwrap().to_be_bytes()).collect(); + let hex: Vec = map.iter().flat_map(|b| format!("{b:02X}").into_bytes()).collect(); + let mut dict = Dict::new(); + dict.set(b"Filter".to_vec(), Object::Array(vec![Object::name("FlateDecode"), Object::name("ASCIIHexDecode")])); + let stream = Stream::from_raw(dict, Stream::flate(Dict::new(), &hex).raw.to_vec()); + let unread = stream.decoded_within(MAX_CID_TO_GID).is_err(); + let map = Object::Ref(doc.add(Object::Stream(stream))); + let Some(Object::Array(mut descendants)) = f.get(b"DescendantFonts").cloned() else { panic!("descendant") }; + if let Some(Object::Dict(d)) = descendants.get_mut(0) { + d.set(b"CIDToGIDMap".to_vec(), map); + } + f.set(b"DescendantFonts".to_vec(), Object::Array(descendants)); + (Metrics::from_dict(&doc, &f).outlined_cids.is_empty(), unread) + }; + // 0.6 of the limit: its hex text, 1.2 of it, is past the limit; the stages make 1.8 of it. + assert_eq!(read(MAX_CID_TO_GID * 6 / 20), (true, true)); + assert_eq!(read(MAX_CID_TO_GID / 4), (false, false)); +} + /// The editor's program is held to its limit as the metrics reads' streams are: a 12 MiB program /// whose Flate data is about 8 MiB reads behind ASCIIHex and Flate as behind Flate alone, though /// the two stages make about 20 MiB together. A program of the limit reads behind either, and one a From 509cf9f9fd96aa22153e2e28419f1cf9f960308c Mon Sep 17 00:00:00 2001 From: Grzegorz Martens Date: Mon, 9 Nov 2026 13:36:44 +0100 Subject: [PATCH 09/16] M7: the editor reads no metrics for a composite font it can't adapt The Resources review of 8eead8ecd: - `embedded_sfnt_within` read a composite font's metrics within the editor's allowance before `composite_glyph_maps` refused any `/Encoding` but Identity-H, so an Identity-V or UniJIS font spent its ToUnicode and `/CIDToGIDMap` for nothing. Its `/Encoding` is now checked first; a program with a cmap of its own is kept as it is, as when the rebuild finds nothing. - The Thai edit's allowance, `MAX_THAI_PROGRAMS`, now also pays for the CMaps and glyph maps its subsets' metrics and codes are read from: its doc and its refusal say "font data", not "font programs". - `a_symbolic_truetype_font_without_an_encoding_reads_its_glyph_names` also reads its font, whose `post` table names its glyphs, within an allowance: the program is paid for by the glyph-name reading, by the Mac OS Roman one and for the widths, and a byte short of the first the names go unread. Tests: `a_composite_font_that_cant_be_adapted_spends_only_its_program`, and `adapting_a_programs_cmap_is_paid_for` again expects a composite font whose codes aren't Identity-H to pay for its program only. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AmsdeUCdTaD72jkDw2pJKY --- crates/edit/src/text_regions/styled.rs | 7 ++--- crates/fonts/src/pdf.rs | 9 +++++++ crates/fonts/src/pdf/tests.rs | 37 +++++++++++++++++++++++--- 3 files changed, 47 insertions(+), 6 deletions(-) diff --git a/crates/edit/src/text_regions/styled.rs b/crates/edit/src/text_regions/styled.rs index 487bb0e20e..d45049db02 100644 --- a/crates/edit/src/text_regions/styled.rs +++ b/crates/edit/src/text_regions/styled.rs @@ -116,8 +116,9 @@ struct LegacyFace { space: f64, } -/// What one edit may decode, and so keep, of the font programs of its THSarabunPSK subsets -/// (each at most 16 MiB): a page may name many of them. +/// What one edit may decode of its THSarabunPSK subsets' font data: their programs (each at most +/// 16 MiB), which it keeps, and the CMaps and glyph maps their metrics and codes are read from. A +/// page may name many of them. const MAX_THAI_PROGRAMS: usize = 64 << 20; struct PreparedCharacters { @@ -926,7 +927,7 @@ fn legacy_thai_font( // A subset left unread would count as unusable, silently changing which subset draws the text. if budget.remaining() == 0 { return Err(EditError::Invalid(format!( - "the page's {} subsets need more than {} MiB of font programs for one edit; choose a replacement font for this text", + "the page's {} subsets need more than {} MiB of font data for one edit; choose a replacement font for this text", source.base_font.rsplit('+').next().unwrap_or(&source.base_font), MAX_THAI_PROGRAMS >> 20 ))); diff --git a/crates/fonts/src/pdf.rs b/crates/fonts/src/pdf.rs index c8f34bd1c4..43670fb4db 100644 --- a/crates/fonts/src/pdf.rs +++ b/crates/fonts/src/pdf.rs @@ -135,6 +135,15 @@ pub fn embedded_sfnt_within(doc: &Document, font: &Dict, budget: &mut DecodeBudg if own_cmap && !base_font.rsplit('+').next().is_some_and(|f| f.starts_with("THSarabunPSK")) { return Some(bytes); } + // A composite font's cmap is rebuilt only through Identity-H codes (`composite_glyph_maps`): + // with any other `/Encoding` nothing is, so its metrics aren't read, and a program with a + // cmap of its own is kept as it is, as when the rebuild finds nothing. + if font.name(b"Subtype") == Some(b"Type0") && font.name(b"Encoding") != Some(b"Identity-H") { + if own_cmap { + return Some(bytes); + } + continue; + } // PDF subsets may remove the Unicode cmap entirely. Rebuild an editor-only // cmap from ToUnicode and the actual CID-to-glyph map; source bytes stay unchanged. // Metrics read past the allowance, its widths' read of the program again included, diff --git a/crates/fonts/src/pdf/tests.rs b/crates/fonts/src/pdf/tests.rs index 53d43f0c72..54b38c6e16 100644 --- a/crates/fonts/src/pdf/tests.rs +++ b/crates/fonts/src/pdf/tests.rs @@ -243,13 +243,13 @@ fn adapting_a_programs_cmap_is_paid_for() { assert_eq!(skrifa::FontRef::new(&editor).unwrap().charmap().map('Ω'), Some(expected)); assert_eq!(budget.remaining(), 0); assert!(embedded_sfnt_within(&doc, &source, &mut DecodeBudget::new(cost - 1, 0)).is_none()); - // A composite font's metrics are read too, and paid for: the program and its ToUnicode, which - // give no glyph here (its codes aren't Identity-H). + // A composite font whose codes aren't Identity-H can't be adapted, so its metrics aren't + // read: the program is all it pays for. let mut budget = DecodeBudget::new(cost, 0); let mut composite = source.clone(); composite.set(b"Subtype".to_vec(), Object::name("Type0")); assert!(embedded_sfnt_within(&doc, &composite, &mut budget).is_none()); - assert_eq!(budget.remaining(), cost - subset.len() - cmap.len()); + assert_eq!(budget.remaining(), cost - subset.len()); } /// A Type 0 subset font whose `/Encoding`, program, `/CIDToGIDMap` and ToUnicode are streams, and @@ -721,6 +721,21 @@ fn a_symbolic_truetype_font_without_an_encoding_reads_its_glyph_names() { ], ); assert_eq!(Metrics::from_dict(&doc, &f).decode(&codes), ""); + // The names are read within the allowance the metrics are read within: the program is paid + // for by the glyph-name reading, by the Mac OS Roman one (which finds the glyphs named and + // reads nothing) and for the widths. A byte short of the first, the names go unread. + let mut doc = Document::new_empty(); + let program = doc.add(Object::Stream(Stream::flate(Dict::new(), &subset))); + let mut descriptor = Dict::new(); + descriptor.set(b"Flags".to_vec(), Object::Int(4)); + descriptor.set(b"FontFile2".to_vec(), Object::Ref(program)); + let f = font(&mut doc, vec![("Subtype", Object::name("TrueType")), ("FontDescriptor", Object::Dict(descriptor))]); + let mut budget = DecodeBudget::new(3 * subset.len(), 0); + assert_eq!(Metrics::from_dict_within(&doc, &f, &mut budget), symbolic); + assert_eq!(budget.remaining(), 0); + let short = Metrics::from_dict_within(&doc, &f, &mut DecodeBudget::new(subset.len() - 1, 0)); + assert!(short.past_allowance); + assert_ne!(short.decode(&codes), TEXT, "the names unread"); } /// The glyph list specification's `uni` and `u` forms spell their hex digits in uppercase @@ -1984,6 +1999,22 @@ fn adapting_a_composite_subsets_cmap_is_paid_for() { assert!(embedded_sfnt_within(&doc, &source, &mut DecodeBudget::new(subset.len() + cmap - 1, 0)).is_none()); } +/// A composite font whose `/Encoding` isn't Identity-H can't have its cmap rebuilt +/// (`composite_glyph_maps` reads only Identity-H codes), so the editor reads its program and no +/// more: its ToUnicode and `/CIDToGIDMap` are left for what is left of the allowance. +#[test] +fn a_composite_font_that_cant_be_adapted_spends_only_its_program() { + let mut doc = Document::new_empty(); + let (source, _, subset, _, [cmap, map]) = cmapless_composite(&mut doc); + for encoding in ["Identity-V", "UniJIS-UCS2-H"] { + let mut f = source.clone(); + f.set(b"Encoding".to_vec(), Object::name(encoding)); + let mut budget = DecodeBudget::new(subset.len() + cmap + map, 0); + assert!(embedded_sfnt_within(&doc, &f, &mut budget).is_none(), "{encoding}"); + assert_eq!(budget.remaining(), cmap + map, "{encoding}"); + } +} + /// The `/CIDToGIDMap` a composite subset's cmap is rebuilt from reads as the font's other streams /// do: a full map, which compresses well, reads behind ASCIIHex and Flate, though the hex stage's /// output comes on top of the map. From 8fb534ad4c1f82f8a1398843f81b2cb27881fc43 Mon Sep 17 00:00:00 2001 From: Grzegorz Martens Date: Mon, 9 Nov 2026 14:08:29 +0100 Subject: [PATCH 10/16] M7: tests pin what a stage past its limit charges the allowance Mutation testing of 063f8629e missed eight mutants in `DecodeBudget::decode_stages_within`: nothing pinned what a read stopped at a stage's limit, at the stages' total or at what is left of the allowance charges, which limit it reports, or where one case gives way to the next. `a_stage_past_its_limit_charges_what_came_before_and_the_limit` reads each way, and each boundary on both sides: a byte more room than the hex stage and the limit (charged them, a byte left), exactly that (past the allowance: all of it spent, its limit reported), a hex stage making exactly the limit so the Flate stage reaches the stages' total with more left (charged the total), with exactly the total left of a larger allowance and with less (spent, the allowance's limit reported), and, with nothing left, a stream with data that decodes to nothing (refused) and an empty one (read). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AmsdeUCdTaD72jkDw2pJKY --- crates/cos/src/budget.rs | 56 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/crates/cos/src/budget.rs b/crates/cos/src/budget.rs index f456583732..08654692d5 100644 --- a/crates/cos/src/budget.rs +++ b/crates/cos/src/budget.rs @@ -1103,4 +1103,60 @@ mod tests { assert!(budget.decode_stages_within(&fits, max).is_err()); assert_eq!(budget.remaining(), 0); } + + /// What `decode_stages_within` charges, and which limit it reports, on each way a read stops: + /// a stage at the stream's limit with more of the room left charges what came before and the + /// limit; at the stages' total, with more of the allowance left, it charges that total; at + /// what is left of the allowance it spends all of it and reports the allowance's limit, as + /// `decode_within` does. Each boundary between them is read on both sides. + #[test] + fn a_stage_past_its_limit_charges_what_came_before_and_the_limit() { + let max = 1 << 12; + let hex = |data: &[u8]| data.iter().flat_map(|b| format!("{b:02X}").into_bytes()).chain(*b">").collect::>(); + let two = |raw: &[u8]| { + let mut dict = Dict::new(); + dict.set(b"Filter".to_vec(), Object::Array(vec![Object::name("ASCIIHexDecode"), Object::name("FlateDecode")])); + Stream::from_raw(dict, hex(raw)) + }; + let says = |read: Result, CosError>, limit: usize| { + read.err().map(|e| e.to_string()) == Some(CosError::Filter(FilterError::LimitExceeded(limit).to_string()).to_string()) + }; + // Flate data making eight times the limit: the hex stage makes `inner` bytes of it, and + // the Flate stage stops at the limit. + let bomb = Stream::flate(Dict::new(), &vec![0u8; 8 * max]).raw.to_vec(); + let inner = bomb.len(); + // A byte more of the room than the hex stage and the limit: the Flate stage stops at the + // limit, so that is what it charges. + let mut budget = DecodeBudget::new(inner + max + 1, 0); + assert!(says(budget.decode_stages_within(&two(&bomb), max), max)); + assert_eq!(budget.remaining(), 1); + // Exactly the hex stage and the limit: what was left stopped it as much as the limit did, + // so it is past the allowance, spends it and reports the allowance's limit. + let mut budget = DecodeBudget::new(inner + max, 0); + assert!(says(budget.decode_stages_within(&two(&bomb), max), inner + max)); + assert_eq!(budget.remaining(), 0); + // The same data padded to exactly the limit: the hex stage makes all of it, and the Flate + // stage, at the limit, reaches the stages' total. With more of the allowance left, that + // total is what it charges and reports. + let mut padded = bomb.clone(); + padded.resize(max, 0); + let mut budget = DecodeBudget::new(10 * max, 0); + assert!(says(budget.decode_stages_within(&two(&padded), max), 2 * max)); + assert_eq!(budget.remaining(), 8 * max); + // With exactly the total left (of an allowance of three times the limit), it is past the + // allowance: all of it is spent, and the allowance's limit reported. + let mut budget = DecodeBudget::new(3 * max, 0); + budget.decode(&Stream::from_raw(Dict::new(), vec![0; max])).unwrap(); + assert!(says(budget.decode_stages_within(&two(&padded), max), 3 * max)); + assert_eq!(budget.remaining(), 0); + // With less than the total left, the same. + let mut budget = DecodeBudget::new(max, 0); + assert!(says(budget.decode_stages_within(&two(&padded), max), max)); + assert_eq!(budget.remaining(), 0); + // With nothing left, a stream with data is refused before it is decoded, even one that + // decodes to nothing; an empty one reads. + let mut budget = DecodeBudget::new(0, 0); + assert!(says(budget.decode_stages_within(&Stream::flate(Dict::new(), b""), max), 0)); + assert_eq!(budget.decode_stages_within(&Stream::from_raw(Dict::new(), Vec::new()), max).ok(), Some(Vec::new())); + } } From e9142b03b91489c411e2ea342b73df012a38f1f8 Mon Sep 17 00:00:00 2001 From: Grzegorz Martens Date: Mon, 9 Nov 2026 16:27:49 +0100 Subject: [PATCH 11/16] M7: a composite font's /Encoding is read through a reference MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Legal forms review of 8fb534ad4. A Type 0 font's /Encoding may be given by reference (a name or a CMap stream, either indirect), and the metrics read resolve it: an indirect /Identity-H reads two-byte codes. The editor's check before reading a composite font's metrics, and `main`'s `composite_glyph_maps` and `recovered_thai_map`, read it with `Dict::name`, which doesn't resolve: such a font counted as another encoding, so the editor adapted no copy of it and Repair Thai maps passed it over. All three now read it through `identity_h`, which resolves it as the metrics do. Tests: `a_composite_fonts_encoding_given_by_reference_is_read` (the editor adapts a copy of a composite subset with `/Encoding 5 0 R`, object 5 `/Identity-H`, that maps 'Ω', and its metrics read two-byte codes), and `recovered_thai_map_checks_every_kept_glyph_against_the_font_program` recovers the same map with the font's /Encoding by reference. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AmsdeUCdTaD72jkDw2pJKY --- crates/fonts/src/pdf.rs | 8 +++++++- crates/fonts/src/pdf/tests.rs | 14 ++++++++++++++ crates/fonts/src/pdf/thai.rs | 10 +++++++--- 3 files changed, 28 insertions(+), 4 deletions(-) diff --git a/crates/fonts/src/pdf.rs b/crates/fonts/src/pdf.rs index 43670fb4db..1573204192 100644 --- a/crates/fonts/src/pdf.rs +++ b/crates/fonts/src/pdf.rs @@ -84,6 +84,12 @@ fn dict(doc: &Document, o: Option<&Object>) -> Option { o.and_then(|o| doc.resolve(o).as_dict().cloned()) } +/// Whether `font`'s `/Encoding` is the name Identity-H, given directly or through a reference, +/// as [`Metrics::from_dict`] reads it (a Type 0 font's `/Encoding` may be indirect). +fn identity_h(doc: &Document, font: &Dict) -> bool { + font.get(b"Encoding").is_some_and(|e| doc.resolve(e).as_name() == Some(b"Identity-H".as_slice())) +} + /// Whether `base_font` (subset tag allowed) names a legacy Indic font family that is laid out in /// visual order (Kruti Dev, DevLys, Chanakya, Shusha, Shivaji, Walkman, Akruti, APS-DV, DV-TT…). fn legacy_indic_font(base_font: &str) -> bool { @@ -138,7 +144,7 @@ pub fn embedded_sfnt_within(doc: &Document, font: &Dict, budget: &mut DecodeBudg // A composite font's cmap is rebuilt only through Identity-H codes (`composite_glyph_maps`): // with any other `/Encoding` nothing is, so its metrics aren't read, and a program with a // cmap of its own is kept as it is, as when the rebuild finds nothing. - if font.name(b"Subtype") == Some(b"Type0") && font.name(b"Encoding") != Some(b"Identity-H") { + if font.name(b"Subtype") == Some(b"Type0") && !identity_h(doc, font) { if own_cmap { return Some(bytes); } diff --git a/crates/fonts/src/pdf/tests.rs b/crates/fonts/src/pdf/tests.rs index 54b38c6e16..975b71289e 100644 --- a/crates/fonts/src/pdf/tests.rs +++ b/crates/fonts/src/pdf/tests.rs @@ -2015,6 +2015,20 @@ fn a_composite_font_that_cant_be_adapted_spends_only_its_program() { } } +/// A composite font's `/Encoding` given through a reference is the name it refers to, as the +/// metrics read it: Identity-H by reference is adapted as Identity-H written directly is. +#[test] +fn a_composite_fonts_encoding_given_by_reference_is_read() { + use skrifa::MetadataProvider as _; + let mut doc = Document::new_empty(); + let (mut source, _, _, expected, _) = cmapless_composite(&mut doc); + let identity = doc.add(Object::name("Identity-H")); + source.set(b"Encoding".to_vec(), Object::Ref(identity)); + assert_eq!(Metrics::from_dict(&doc, &source).code_len, 2); + let editor = embedded_sfnt(&doc, &source).expect("Identity-H by reference is adapted"); + assert_eq!(skrifa::FontRef::new(&editor).unwrap().charmap().map('Ω'), Some(expected)); +} + /// The `/CIDToGIDMap` a composite subset's cmap is rebuilt from reads as the font's other streams /// do: a full map, which compresses well, reads behind ASCIIHex and Flate, though the hex stage's /// output comes on top of the map. diff --git a/crates/fonts/src/pdf/thai.rs b/crates/fonts/src/pdf/thai.rs index a6da5f1d77..c6e1941fbd 100644 --- a/crates/fonts/src/pdf/thai.rs +++ b/crates/fonts/src/pdf/thai.rs @@ -7,7 +7,7 @@ //! does (a mark takes none); correct maps and unrelated fonts are never changed. No font //! artwork is bundled. -use super::{Dict, Document, Metrics, Reads, dict, only_char, unicode_map}; +use super::{Dict, Document, Metrics, Reads, dict, identity_h, only_char, unicode_map}; use pdfcraft_cos::{DecodeBudget, Object}; use std::collections::BTreeMap; @@ -19,7 +19,7 @@ pub fn recovered_thai_map(doc: &Document, font: &Dict) -> Option Option<(BTreeMap, BTreeMap)> { use skrifa::{MetadataProvider as _, raw::TableProvider as _}; - if font.name(b"Encoding") != Some(b"Identity-H") { + if !identity_h(doc, font) { return None; } let descendant = dict(doc, font.get(b"DescendantFonts").and_then(|a| doc.resolve(a).as_array().and_then(|a| a.first()).cloned()).as_ref())?; @@ -548,6 +548,10 @@ mod tests { let recovered: HashMap<_, _> = recovered_thai_map(&doc, &font).unwrap().into_iter().collect(); assert_eq!(recovered[&262], 'ั'); assert_eq!(recovered[&297], '๘'); + // `/Encoding` given through a reference is the name it refers to. + let mut indirect = font.clone(); + indirect.set(b"Encoding".to_vec(), Object::Ref(doc.add(Object::name("Identity-H")))); + assert_eq!(recovered_thai_map(&doc, &indirect), recovered_thai_map(&doc, &font)); // 259–261 are ฮ, ฯ and ะ, between the last anchor (อ, 258) and the first mark: a font that // put Mai Han-akat one glyph earlier passes every anchor and only this check sees it. let mut shifted = thai_ordered_program(); From 92e8e6eff0d92aa11048a3c86a41a8736e4e1235 Mon Sep 17 00:00:00 2001 From: Grzegorz Martens Date: Mon, 9 Nov 2026 16:48:01 +0100 Subject: [PATCH 12/16] M7: Repair Thai maps reads a /CIDToGIDMap given by reference, and takes a missing one as Identity The Consumers review of ba28458ad..b7224cbc9: - `recovered_thai_map` read the descendant's /CIDToGIDMap with `Dict::name`, on the line after e9142b03b's /Encoding check, so a map given by reference made Repair Thai maps pass the font over, and so did a missing one, which is Identity (ISO 32000-2 Table 115), as `composite_glyph_maps`, `LegacyThaiFont` and the renderer read it. It now resolves the map and takes a missing one as Identity. - `MAX_THAI_PROGRAMS` says what it pays for: the programs, and the CMaps and glyph maps read to adapt their cmaps. The metrics read to find and measure the subsets aren't charged to it. Its refusal ("MiB of font data") still says what it counts. Test: `recovered_thai_map_checks_every_kept_glyph_against_the_font_program` recovers the same map with the font's /CIDToGIDMap given by reference and with none. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AmsdeUCdTaD72jkDw2pJKY --- crates/edit/src/text_regions/styled.rs | 5 +++-- crates/fonts/src/pdf/thai.rs | 22 +++++++++++++++++++++- 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/crates/edit/src/text_regions/styled.rs b/crates/edit/src/text_regions/styled.rs index d45049db02..dbb6c5d46e 100644 --- a/crates/edit/src/text_regions/styled.rs +++ b/crates/edit/src/text_regions/styled.rs @@ -117,8 +117,9 @@ struct LegacyFace { } /// What one edit may decode of its THSarabunPSK subsets' font data: their programs (each at most -/// 16 MiB), which it keeps, and the CMaps and glyph maps their metrics and codes are read from. A -/// page may name many of them. +/// 16 MiB), which it keeps, and the CMaps and glyph maps read to adapt their programs' cmaps. A +/// page may name many of them. The metrics read to find and measure the subsets +/// (`Metrics::from_dict`) aren't charged to it. const MAX_THAI_PROGRAMS: usize = 64 << 20; struct PreparedCharacters { diff --git a/crates/fonts/src/pdf/thai.rs b/crates/fonts/src/pdf/thai.rs index c6e1941fbd..28d29b0055 100644 --- a/crates/fonts/src/pdf/thai.rs +++ b/crates/fonts/src/pdf/thai.rs @@ -24,7 +24,9 @@ pub fn recovered_thai_map(doc: &Document, font: &Dict) -> Option| { + let mut f = font.clone(); + let Some(Object::Array(desc)) = f.get(b"DescendantFonts").cloned() else { panic!("fixture") }; + let Some(Object::Dict(mut desc)) = desc.first().cloned() else { panic!("fixture") }; + match map { + Some(map) => desc.set(b"CIDToGIDMap".to_vec(), map), + None => { + desc.remove(b"CIDToGIDMap"); + } + } + f.set(b"DescendantFonts".to_vec(), Object::Array(vec![Object::Dict(desc)])); + f + }; + let by_reference = with_map(Some(Object::Ref(doc.add(Object::name("Identity"))))); + assert_eq!(recovered_thai_map(&doc, &by_reference), recovered_thai_map(&doc, &font), "a map by reference"); + assert_eq!(recovered_thai_map(&doc, &with_map(None)), recovered_thai_map(&doc, &font), "a missing map"); // 259–261 are ฮ, ฯ and ะ, between the last anchor (อ, 258) and the first mark: a font that // put Mai Han-akat one glyph earlier passes every anchor and only this check sees it. let mut shifted = thai_ordered_program(); From 4d6ff291d7f13676961b1b6ec5f706cd63995291 Mon Sep 17 00:00:00 2001 From: Grzegorz Martens Date: Mon, 9 Nov 2026 16:52:47 +0100 Subject: [PATCH 13/16] M7: Reads::decode says what a stage past its limit is charged The Claims review of the description at b7224cbc9. `Reads::decode` said a stream past its limit is unread, not refused, "when what is left can pay for its stages". What `DecodeBudget::decode_stages_within` does, and `a_stage_past_its_limit_charges_what_came_before_and_the_limit` pins: with more left than what the stages before the stopped one made and the limit, it is charged those two; otherwise, with more left than the stages' total (behind one filter, the limit), it is charged that total; with no more left than that total, it spends all of it and counts as refused. `MAX_THAI_PROGRAMS` also names the second read of a subset's program, for which of its glyphs have outlines, among what it pays for. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AmsdeUCdTaD72jkDw2pJKY --- crates/edit/src/text_regions/styled.rs | 7 ++++--- crates/fonts/src/pdf.rs | 8 +++++--- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/crates/edit/src/text_regions/styled.rs b/crates/edit/src/text_regions/styled.rs index dbb6c5d46e..a7c7166f35 100644 --- a/crates/edit/src/text_regions/styled.rs +++ b/crates/edit/src/text_regions/styled.rs @@ -117,9 +117,10 @@ struct LegacyFace { } /// What one edit may decode of its THSarabunPSK subsets' font data: their programs (each at most -/// 16 MiB), which it keeps, and the CMaps and glyph maps read to adapt their programs' cmaps. A -/// page may name many of them. The metrics read to find and measure the subsets -/// (`Metrics::from_dict`) aren't charged to it. +/// 16 MiB), which it keeps, and what is read to adapt their programs' cmaps: their CMaps and glyph +/// maps, and a subset's program again, for which of its glyphs have outlines. A page may name many +/// of them. The metrics read to find and measure the subsets (`Metrics::from_dict`) aren't charged +/// to it. const MAX_THAI_PROGRAMS: usize = 64 << 20; struct PreparedCharacters { diff --git a/crates/fonts/src/pdf.rs b/crates/fonts/src/pdf.rs index 1573204192..e70683d55c 100644 --- a/crates/fonts/src/pdf.rs +++ b/crates/fonts/src/pdf.rs @@ -415,9 +415,11 @@ impl<'a> Reads<'a> { /// the stages together to `max` for each of them ([`DecodeBudget::decode_stages_within`]): a /// map of 65,536 CIDs behind ASCIIHex and Flate, its Flate data smaller than the map, makes /// all of `MAX_CID_TO_GID` in its last stage, with the hex stage's output on top. So a stream - /// reads here when it reads without an allowance and what is left pays for it. A stream past - /// `max` is unread, not refused, when what is left can pay for its stages; when it can't, the - /// stream spends it and counts as refused. + /// reads here when it reads without an allowance and what is left pays for it. A stream with a + /// stage past `max` is unread. When what is left is more than what the stages before that one + /// made and `max`, it is charged those two and isn't refused; otherwise, with more left than + /// the stages' total (behind one filter, `max`), it is charged that total and isn't refused; + /// with no more left than that total, it spends all of it and counts as refused. fn decode(&mut self, stream: &Stream, max: usize) -> Option> { // Once the allowance is spent the decode refuses the stream; refused here, before the // parameters are resolved, which is work the allowance doesn't count. From 674a208990d4925436abf16f849a23bb1f0c9e3b Mon Sep 17 00:00:00 2001 From: Grzegorz Martens Date: Mon, 9 Nov 2026 17:18:51 +0100 Subject: [PATCH 14/16] M7: a test pins that Repair Thai maps refuses a map that isn't Identity The Legal forms review of b7224cbc9..4d6ff291d: - Nothing pinned that `recovered_thai_map`'s rewritten guard still refuses a /CIDToGIDMap that isn't Identity: deleting it left every Thai test passing. `recovered_thai_map_checks_every_kept_glyph_against_ the_font_program` now asserts no map for a map stream that moves glyphs and for another name. - The guard's comment and the test's cited Identity as the default of ISO 32000-2 Table 115. It is the default of PDF 1.x (ISO 32000-1); PDF 2.0 gives it none and requires the map with a /FontFile2. Both now say so, and that the renderer takes a missing map as Identity too. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AmsdeUCdTaD72jkDw2pJKY --- crates/fonts/src/pdf/thai.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/crates/fonts/src/pdf/thai.rs b/crates/fonts/src/pdf/thai.rs index 28d29b0055..e15799ed04 100644 --- a/crates/fonts/src/pdf/thai.rs +++ b/crates/fonts/src/pdf/thai.rs @@ -24,8 +24,9 @@ pub fn recovered_thai_map(doc: &Document, font: &Dict) -> Option| { let mut f = font.clone(); let Some(Object::Array(desc)) = f.get(b"DescendantFonts").cloned() else { panic!("fixture") }; @@ -572,6 +573,10 @@ mod tests { let by_reference = with_map(Some(Object::Ref(doc.add(Object::name("Identity"))))); assert_eq!(recovered_thai_map(&doc, &by_reference), recovered_thai_map(&doc, &font), "a map by reference"); assert_eq!(recovered_thai_map(&doc, &with_map(None)), recovered_thai_map(&doc, &font), "a missing map"); + // Any other map is refused: one that moves glyphs, and another name. + let moved = Object::Ref(doc.add(Object::Stream(Stream::flate(Dict::new(), &[0, 1, 0, 0])))); + assert_eq!(recovered_thai_map(&doc, &with_map(Some(moved))), None, "a map that moves glyphs"); + assert_eq!(recovered_thai_map(&doc, &with_map(Some(Object::name("Foo")))), None, "another name"); // 259–261 are ฮ, ฯ and ะ, between the last anchor (อ, 258) and the first mark: a font that // put Mai Han-akat one glyph earlier passes every anchor and only this check sees it. let mut shifted = thai_ordered_program(); From 8c5333bfe76b6f02d75201dd2fbd55c9e8bee549 Mon Sep 17 00:00:00 2001 From: Grzegorz Martens Date: Mon, 9 Nov 2026 21:45:10 +0100 Subject: [PATCH 15/16] M7: font reads decode through a view of the stream, and refuse before resolving its parameters CodeRabbit's review of 674a20899: - Each budgeted read of a font's stream went through `Document::with_direct_parms`, which copies the whole stream when its /DecodeParms is given by reference: an unrelated value in the dictionary, however large, was copied again for every read. `DecodeBudget::decode_stages_within` now takes the document and reads through `Document::decoding_view`, as `decode_strict` and `decode_with_damage` do: the data is shared, and only /Filter and the parameters made direct are copied. `Reads::decode` and the editor's program read use it, a Type 3 font's glyph descriptions read through `decode_with_damage`, and so does Repair Thai maps' program read. - A Type 3 font's glyph descriptions resolved one description's parameters a font before the spent allowance refused it. They now ask `DecodeBudget::refuses` first, as `decode_stages_within` does for the others: once the allowance is spent a stream with data is refused, and noted, before its parameters are resolved. - `DecodeBudget::refused` counts the decodes refused for want of the allowance, where `remaining() == 0` can't tell a refusal from an allowance spent to the byte. Test: `reading_a_fonts_streams_copies_no_dictionary` (`pdfcraft-fonts`, a test binary of its own): a font whose program and ToUnicode hold an unrelated array of a million numbers and /DecodeParms by reference is read eight times by the editor's program read and the metrics read, and the process's peak grows by less than half of what one array holds. That a Type 3 font refuses before resolving parameters changes no result, only the time it takes. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AmsdeUCdTaD72jkDw2pJKY --- crates/cos/src/budget.rs | 57 +++++++++++++++++++-------- crates/fonts/src/pdf.rs | 33 ++++++++-------- crates/fonts/src/pdf/thai.rs | 2 +- crates/fonts/tests/stream_views.rs | 62 ++++++++++++++++++++++++++++++ 4 files changed, 119 insertions(+), 35 deletions(-) create mode 100644 crates/fonts/tests/stream_views.rs diff --git a/crates/cos/src/budget.rs b/crates/cos/src/budget.rs index 08654692d5..0067304f66 100644 --- a/crates/cos/src/budget.rs +++ b/crates/cos/src/budget.rs @@ -246,12 +246,14 @@ pub struct DecodeBudget { bytes: usize, forms: usize, seen: HashSet, + /// Decodes refused for lack of allowance ([`DecodeBudget::refused`]). + refusals: usize, } impl DecodeBudget { /// An allowance of `bytes` decoded bytes and `forms` form entries. pub fn new(bytes: usize, forms: usize) -> Self { - Self { limit: bytes, bytes, forms, seen: HashSet::new() } + Self { limit: bytes, bytes, forms, seen: HashSet::new(), refusals: 0 } } /// The allowance for one page: [`MAX_PAGE_DECODED`] bytes and [`MAX_PAGE_FORMS`] forms. @@ -264,6 +266,22 @@ impl DecodeBudget { self.bytes } + /// How many decodes were refused for lack of allowance: one that came once it was spent, or + /// ran past what was left. A decode that spent exactly what was left wasn't refused, so an + /// allowance used up to the byte has none. + pub fn refused(&self) -> usize { + self.refusals + } + + /// Whether the allowance is spent and `stream`, which has data, would be refused, noting the + /// refusal ([`DecodeBudget::refused`]): a caller asks this before resolving the stream's + /// parameters, work the allowance doesn't count. A stream without data still reads. + pub fn refuses(&mut self, stream: &Stream) -> bool { + let refuses = self.bytes == 0 && !stream.raw.is_empty(); + self.refusals += usize::from(refuses); + refuses + } + /// Whether a decode was refused for lack of allowance or a form entry for lack of visits. pub fn exhausted(&self) -> bool { self.bytes == 0 || self.forms == 0 @@ -284,16 +302,21 @@ impl DecodeBudget { } /// Like [`DecodeBudget::decode_within`], with every filter stage held to `max`, as - /// [`Stream::decoded_within`] holds it, and the stages together to `max` for each of them. A + /// [`Stream::decoded_within`] holds it, and the stages together to `max` for each of them, the + /// stream's `/DecodeParms` given by reference read in `doc` through a view of it that shares + /// its data and copies no more of its dictionary than its filters and parameters + /// ([`DecodeBudget::decode_with_damage`] reads it so). A /// stream so reads here when it reads without an allowance and the allowance can pay for /// what its stages decode, the output of one stage on top of another's. A stage past `max` /// is an error that charges what the stages before it decoded and `max`, the work it took to /// find out; past the stages' total it charges that total, and past what is left it spends /// all of it, as [`DecodeBudget::decode_within`] does. - pub fn decode_stages_within(&mut self, stream: &Stream, max: usize) -> Result, CosError> { - if self.bytes == 0 && !stream.raw.is_empty() { + pub fn decode_stages_within(&mut self, doc: &Document, stream: &Stream, max: usize) -> Result, CosError> { + // Before the parameters are resolved, work the allowance doesn't count. + if self.refuses(stream) { return Err(CosError::Filter(FilterError::LimitExceeded(self.limit).to_string())); } + let stream = doc.decoding_view(stream); let total = max.saturating_mul(stream.filters().len().max(1)); let room = total.min(self.bytes); match stream.decoded_within_each_counted(max, room) { @@ -313,6 +336,7 @@ impl DecodeBudget { } Err((FilterError::LimitExceeded(_), _)) => { self.bytes = 0; + self.refusals += 1; Err(CosError::Filter(FilterError::LimitExceeded(self.limit).to_string())) } Err((e, spent)) => { @@ -343,7 +367,7 @@ impl DecodeBudget { /// pays for the bytes decoded before the error, so referencing a stream again and again /// spends the allowance by the work each decode does. fn charge(&mut self, stream: &Stream, max: usize, decode: Decoder) -> Result<(Vec, bool), CosError> { - if self.bytes == 0 && !stream.raw.is_empty() { + if self.refuses(stream) { return Err(CosError::Filter(FilterError::LimitExceeded(self.limit).to_string())); } if max < self.bytes { @@ -363,6 +387,7 @@ impl DecodeBudget { } Err((FilterError::LimitExceeded(_), _)) => { self.bytes = 0; + self.refusals += 1; Err(CosError::Filter(FilterError::LimitExceeded(self.limit).to_string())) } Err((e, spent)) => { @@ -1087,20 +1112,20 @@ mod tests { let max = 1 << 16; let (fits, inner) = hexed(&vec![0u8; max]); let mut budget = DecodeBudget::new(1 << 20, 0); - assert_eq!(budget.decode_stages_within(&fits, max).map(|d| d.len()).ok(), Some(max)); + assert_eq!(budget.decode_stages_within(&Document::new_empty(), &fits, max).map(|d| d.len()).ok(), Some(max)); assert_eq!(budget.remaining(), (1 << 20) - inner - max); let (bomb, inner) = hexed(&vec![0u8; 8 * max]); let mut budget = DecodeBudget::new(1 << 20, 0); - assert!(budget.decode_stages_within(&bomb, max).is_err()); + assert!(budget.decode_stages_within(&Document::new_empty(), &bomb, max).is_err()); assert_eq!(budget.remaining(), (1 << 20) - inner - max, "the Flate stage stopped at the limit"); // Flate making 1.2 times the limit of hex text, which ASCIIHex halves: the stages together // make 1.8 times the limit, within twice it, but the Flate stage alone is past it. let text = "00".repeat(max * 6 / 10).into_bytes(); let wide = chained(&Stream::flate(Dict::new(), &text).raw, ["FlateDecode", "ASCIIHexDecode"]); assert!(wide.decoded_within(max).is_err()); - assert!(DecodeBudget::new(1 << 20, 0).decode_stages_within(&wide, max).is_err()); + assert!(DecodeBudget::new(1 << 20, 0).decode_stages_within(&Document::new_empty(), &wide, max).is_err()); let mut budget = DecodeBudget::new(max, 0); - assert!(budget.decode_stages_within(&fits, max).is_err()); + assert!(budget.decode_stages_within(&Document::new_empty(), &fits, max).is_err()); assert_eq!(budget.remaining(), 0); } @@ -1128,12 +1153,12 @@ mod tests { // A byte more of the room than the hex stage and the limit: the Flate stage stops at the // limit, so that is what it charges. let mut budget = DecodeBudget::new(inner + max + 1, 0); - assert!(says(budget.decode_stages_within(&two(&bomb), max), max)); + assert!(says(budget.decode_stages_within(&Document::new_empty(), &two(&bomb), max), max)); assert_eq!(budget.remaining(), 1); // Exactly the hex stage and the limit: what was left stopped it as much as the limit did, // so it is past the allowance, spends it and reports the allowance's limit. let mut budget = DecodeBudget::new(inner + max, 0); - assert!(says(budget.decode_stages_within(&two(&bomb), max), inner + max)); + assert!(says(budget.decode_stages_within(&Document::new_empty(), &two(&bomb), max), inner + max)); assert_eq!(budget.remaining(), 0); // The same data padded to exactly the limit: the hex stage makes all of it, and the Flate // stage, at the limit, reaches the stages' total. With more of the allowance left, that @@ -1141,22 +1166,22 @@ mod tests { let mut padded = bomb.clone(); padded.resize(max, 0); let mut budget = DecodeBudget::new(10 * max, 0); - assert!(says(budget.decode_stages_within(&two(&padded), max), 2 * max)); + assert!(says(budget.decode_stages_within(&Document::new_empty(), &two(&padded), max), 2 * max)); assert_eq!(budget.remaining(), 8 * max); // With exactly the total left (of an allowance of three times the limit), it is past the // allowance: all of it is spent, and the allowance's limit reported. let mut budget = DecodeBudget::new(3 * max, 0); budget.decode(&Stream::from_raw(Dict::new(), vec![0; max])).unwrap(); - assert!(says(budget.decode_stages_within(&two(&padded), max), 3 * max)); + assert!(says(budget.decode_stages_within(&Document::new_empty(), &two(&padded), max), 3 * max)); assert_eq!(budget.remaining(), 0); // With less than the total left, the same. let mut budget = DecodeBudget::new(max, 0); - assert!(says(budget.decode_stages_within(&two(&padded), max), max)); + assert!(says(budget.decode_stages_within(&Document::new_empty(), &two(&padded), max), max)); assert_eq!(budget.remaining(), 0); // With nothing left, a stream with data is refused before it is decoded, even one that // decodes to nothing; an empty one reads. let mut budget = DecodeBudget::new(0, 0); - assert!(says(budget.decode_stages_within(&Stream::flate(Dict::new(), b""), max), 0)); - assert_eq!(budget.decode_stages_within(&Stream::from_raw(Dict::new(), Vec::new()), max).ok(), Some(Vec::new())); + assert!(says(budget.decode_stages_within(&Document::new_empty(), &Stream::flate(Dict::new(), b""), max), 0)); + assert_eq!(budget.decode_stages_within(&Document::new_empty(), &Stream::from_raw(Dict::new(), Vec::new()), max).ok(), Some(Vec::new())); } } diff --git a/crates/fonts/src/pdf.rs b/crates/fonts/src/pdf.rs index e70683d55c..b67152684e 100644 --- a/crates/fonts/src/pdf.rs +++ b/crates/fonts/src/pdf.rs @@ -122,14 +122,10 @@ pub fn embedded_sfnt_within(doc: &Document, font: &Dict, budget: &mut DecodeBudg let Some(object) = descriptor.get(key) else { continue }; let object = doc.resolve(object); let Object::Stream(stream) = object.as_ref() else { continue }; - // As in `Reads::decode`: once the allowance is spent the decode refuses a program with - // data, so it is passed over before its parameters are resolved, work the allowance doesn't - // count; an empty one is no program either way. - if budget.remaining() == 0 { - continue; - } - // As in `Reads::decode`: each stage within the limit, the program, the last, too. - let Ok(mut bytes) = budget.decode_stages_within(&doc.with_direct_parms(stream), MAX_SFNT) else { continue }; + // As in `Reads::decode`: each stage within the limit, the program, the last, too, read + // through a view of the stream; once the allowance is spent a program with data is refused + // before its parameters are resolved. + let Ok(mut bytes) = budget.decode_stages_within(doc, stream, MAX_SFNT) else { continue }; let Ok(face) = skrifa::FontRef::from_index(&bytes, 0) else { continue }; if face.metrics(skrifa::instance::Size::unscaled(), skrifa::instance::LocationRef::default()).units_per_em == 0 { continue; @@ -408,8 +404,8 @@ impl<'a> Reads<'a> { } /// `stream` decoded within `max` and the allowance, its `/DecodeParms` read where they are given - /// by reference too ([`Document::with_direct_parms`]); `None` when it can't be, noting when the - /// allowance is why. + /// by reference too, through a view of the stream that copies no more of its dictionary than + /// its filters and parameters; `None` when it can't be, noting when the allowance is why. /// /// Each of its filter stages is held to `max`, as a decode without an allowance holds it, and /// the stages together to `max` for each of them ([`DecodeBudget::decode_stages_within`]): a @@ -421,13 +417,7 @@ impl<'a> Reads<'a> { /// the stages' total (behind one filter, `max`), it is charged that total and isn't refused; /// with no more left than that total, it spends all of it and counts as refused. fn decode(&mut self, stream: &Stream, max: usize) -> Option> { - // Once the allowance is spent the decode refuses the stream; refused here, before the - // parameters are resolved, which is work the allowance doesn't count. - if self.budget.remaining() == 0 && !stream.raw.is_empty() { - self.refused = true; - return None; - } - match self.budget.decode_stages_within(&self.doc.with_direct_parms(stream), max) { + match self.budget.decode_stages_within(self.doc, stream, max) { Ok(data) => Some(data), Err(_) => { self.refused |= self.budget.remaining() == 0; @@ -488,8 +478,15 @@ fn type3_glyph_box(doc: &Document, font: &Dict, reads: &mut Reads<'_>) -> Option let mut union: Option<[f64; 4]> = None; for proc in chosen { let Object::Stream(s) = &*doc.resolve(proc) else { continue }; + // Once the reading's allowance is spent the rest go unread, before their parameters are + // resolved. + if reads.budget.refuses(s) { + reads.refused = true; + complete = false; + break; + } let before = reads.budget.remaining(); - let read = reads.budget.decode_within(&doc.with_direct_parms(s), MAX_TYPE3_GLYPH_BYTES.min(left)); + let read = reads.budget.decode_with_damage(doc, s, MAX_TYPE3_GLYPH_BYTES.min(left)).map(|(data, _)| data); left = left.saturating_sub(before.saturating_sub(reads.budget.remaining())); let Ok(data) = read else { if reads.budget.remaining() == 0 { diff --git a/crates/fonts/src/pdf/thai.rs b/crates/fonts/src/pdf/thai.rs index e15799ed04..f34111cd5d 100644 --- a/crates/fonts/src/pdf/thai.rs +++ b/crates/fonts/src/pdf/thai.rs @@ -173,7 +173,7 @@ fn embedded_truetype(doc: &Document, descendant: &Dict) -> Option> { let descriptor = dict(doc, descendant.get(b"FontDescriptor"))?; let program = doc.resolve(descriptor.get(b"FontFile2")?); let Object::Stream(program) = program.as_ref() else { return None }; - DecodeBudget::new(super::MAX_SFNT, 0).decode_within(&doc.with_direct_parms(program), super::MAX_SFNT).ok() + DecodeBudget::new(super::MAX_SFNT, 0).decode_with_damage(doc, program, super::MAX_SFNT).ok().map(|(data, _)| data) } /// Source glyph positions and the original PDF character codes that draw them. Empty codes diff --git a/crates/fonts/tests/stream_views.rs b/crates/fonts/tests/stream_views.rs new file mode 100644 index 0000000000..de210637ae --- /dev/null +++ b/crates/fonts/tests/stream_views.rs @@ -0,0 +1,62 @@ +//! Memory regression for the font reads charged to an allowance: a stream whose `/DecodeParms` +//! is given by reference is read through a view that shares its data and copies no more of its +//! dictionary than its filters and parameters, not through a copy of the whole stream. A test +//! binary of its own, so that the one test is all the process runs while its peak is measured. + +use pdfcraft_cos::{DecodeBudget, Dict, Document, ObjRef, Object, Stream}; +use pdfcraft_fonts::pdf::{Metrics, embedded_sfnt_within}; + +/// How much the process's peak resident memory grows, in KiB, while `f` runs. `None` where the +/// peak can't be reset (`/proc/self/clear_refs` is Linux's). +fn peak_growth(f: impl FnOnce() -> T) -> Option<(T, u64)> { + fn peak_kib() -> Option { + let status = std::fs::read_to_string("/proc/self/status").ok()?; + status.lines().find_map(|l| l.strip_prefix("VmHWM:"))?.split_whitespace().next()?.parse().ok() + } + std::fs::write("/proc/self/clear_refs", "5").ok()?; + let before = peak_kib()?; + let out = f(); + Some((out, peak_kib()?.saturating_sub(before))) +} + +/// A stream of `data`, its `/DecodeParms` the reference `parms`, its dictionary holding `junk`. +fn stream(doc: &mut Document, data: &[u8], parms: ObjRef, junk: &Object) -> Object { + let mut s = Stream::flate(Dict::new(), data); + s.dict.set(b"DecodeParms".to_vec(), Object::Ref(parms)); + s.dict.set(b"Junk".to_vec(), junk.clone()); + Object::Ref(doc.add(Object::Stream(s))) +} + +/// A font whose program and ToUnicode each hold an unrelated array of a million numbers in their +/// dictionaries, with `/DecodeParms` given by reference, read eight times within an allowance by +/// the editor's program read and by the metrics read: neither copies an array. A read through a +/// copy of the stream would add about as much as one array holds. +#[test] +fn reading_a_fonts_streams_copies_no_dictionary() { + let junk = Object::Array(vec![Object::Int(0); 1 << 20]); + let array_kib = (std::mem::size_of::() << 20) as u64 >> 10; + let mut doc = Document::new_empty(); + let parms = doc.add(Object::Dict(Dict::from_iter([(b"Predictor".to_vec(), Object::Int(1))]))); + let program = stream(&mut doc, include_bytes!("../../../assets/fonts/Inter-Regular.ttf"), parms, &junk); + let unicode = stream(&mut doc, b"1 beginbfchar <41> <0058> endbfchar", parms, &junk); + drop(junk); + let mut descriptor = Dict::new(); + descriptor.set(b"FontFile2".to_vec(), program); + let mut font = Dict::new(); + font.set(b"Type".to_vec(), Object::name("Font")); + font.set(b"Subtype".to_vec(), Object::name("TrueType")); + font.set(b"FontDescriptor".to_vec(), Object::Dict(descriptor)); + font.set(b"ToUnicode".to_vec(), unicode); + let Some(((), growth)) = peak_growth(|| { + for _ in 0..8 { + let mut budget = DecodeBudget::new(64 << 20, 0); + assert!(embedded_sfnt_within(&doc, &font, &mut budget).is_some()); + let m = Metrics::from_dict_within(&doc, &font, &mut budget); + assert_eq!(m.text_of(0x41).as_deref(), Some("X")); + } + }) else { + eprintln!("the peak can't be measured here"); + return; + }; + assert!(growth < array_kib / 2, "the peak grew {growth} KiB, an array is {array_kib} KiB"); +} From 4fca5783a7a3f04aadfb84bb38ff5d354840644d Mon Sep 17 00:00:00 2001 From: Grzegorz Martens Date: Mon, 9 Nov 2026 21:45:29 +0100 Subject: [PATCH 16/16] M7: a Thai edit refuses only a read past its allowance, and reads its subsets' metrics within it CodeRabbit's review of 674a20899: - The Thai edit refused when nothing was left of its allowance, so an edit whose reads spent it to the byte was refused. It now refuses when a read was refused for want of it (`DecodeBudget::refused`). - `LegacyThaiFont::from_pdf` read its metrics with `from_dict`, outside the edit's allowance, and so did the edit for each subset it measures. Both now read them within it (`from_dict_within`), and metrics read past it adapt nothing: `from_pdf` refuses the font before reading its program, and the edit leaves the subset unread and is refused. The edit's scan of every font on the page for its face name stays outside the allowance, as on `main`: charged, each alias of a font would pay again, and other faces would spend what the face's subsets need. `MAX_THAI_PROGRAMS` says what it pays for. - When the `/CIDToGIDMap` a THSarabunPSK subset's Thai mappings are rebuilt from was left unread for want of the editor's allowance, `composite_glyph_maps` gave nothing, and a subset with a Latin-only cmap of its own was listed as it was, without its Thai mappings. The editor now lists no copy of it, as for metrics read past the allowance. Tests: `a_thai_edit_that_spends_its_allowance_to_the_byte_goes_through` (`pdfcraft-edit`: a subset costing exactly the 64 MiB is edited, and one three bytes past it is refused), `a_legacy_fonts_metrics_are_paid_for_from_the_edits_allowance` and `a_map_left_unread_for_the_allowance_lists_no_copy` (`pdfcraft-fonts`). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AmsdeUCdTaD72jkDw2pJKY --- crates/edit/src/tests.rs | 23 ++++++++++++ crates/edit/src/text_regions/styled.rs | 27 +++++++++----- crates/fonts/src/pdf.rs | 5 +++ crates/fonts/src/pdf/thai.rs | 50 ++++++++++++++++++++++++-- 4 files changed, 94 insertions(+), 11 deletions(-) diff --git a/crates/edit/src/tests.rs b/crates/edit/src/tests.rs index 57009ce99e..466e61c7f9 100644 --- a/crates/edit/src/tests.rs +++ b/crates/edit/src/tests.rs @@ -14295,6 +14295,29 @@ fn legacy_thai_subsets_are_read_within_one_budget_per_edit() { assert_eq!(pdfcraft_cos::write_full(&distinct, &Default::default()).unwrap(), before, "a refused edit changes nothing"); } +/// An edit whose THSarabunPSK subset spends the edit's allowance to the byte goes through: only a +/// read refused for want of it refuses the edit. The subset is read four times over its program +/// (the edit's metrics read, `LegacyThaiFont`'s, the editor's program read and its metrics read) +/// and three times over its ToUnicode, so a program 192 bytes short of 16 MiB and a ToUnicode of +/// 256 bytes cost exactly the 64 MiB; a byte more of ToUnicode is three past it. +#[test] +fn a_thai_edit_that_spends_its_allowance_to_the_byte_goes_through() { + let mut program = legacy_thai_program(); + program.resize((16 << 20) - 192, 0); + let edit = |unicode_len: usize| { + let mut unicode = LEGACY_THAI_UNICODE.to_vec(); + unicode.resize(unicode_len, b' '); + let mut doc = text_page("BT /F1 16 Tf 50 650 Td <00d6> Tj ET"); + let font = legacy_thai_font(&mut doc, "ABCDEF+THSarabunPSK", &program, &unicode); + doc.set(pdfcraft_cos::ObjRef::new(5, 0), Object::Dict(font)); + let style = BlockStyle { width: Some(400.0), ..Default::default() }; + text::rewrite_block(&mut doc, 0, 0, Some("กขค"), &style).map(|_| ()).map_err(|e| e.to_string()) + }; + assert_eq!(edit(256), Ok(())); + let error = edit(257).unwrap_err(); + assert!(error.contains("THSarabunPSK") && error.contains("MiB of font data"), "{error}"); +} + /// A THSarabunPSK-named subset that has OpenType Thai mark positioning is a modern face: it is /// shaped with its own layout like any source font, not by the legacy shaper (which would ask /// for Windows PUA alternates such a face doesn't need). diff --git a/crates/edit/src/text_regions/styled.rs b/crates/edit/src/text_regions/styled.rs index a7c7166f35..9c2732c35f 100644 --- a/crates/edit/src/text_regions/styled.rs +++ b/crates/edit/src/text_regions/styled.rs @@ -117,10 +117,10 @@ struct LegacyFace { } /// What one edit may decode of its THSarabunPSK subsets' font data: their programs (each at most -/// 16 MiB), which it keeps, and what is read to adapt their programs' cmaps: their CMaps and glyph -/// maps, and a subset's program again, for which of its glyphs have outlines. A page may name many -/// of them. The metrics read to find and measure the subsets (`Metrics::from_dict`) aren't charged -/// to it. +/// 16 MiB), which it keeps, and the streams of the metrics read to measure them and to adapt their +/// programs' cmaps: their CMaps and glyph maps, and a subset's program again, for which of its +/// glyphs have outlines. A page may name many of them. The metrics read to find the subsets among +/// the page's fonts aren't charged to it. const MAX_THAI_PROGRAMS: usize = 64 << 20; struct PreparedCharacters { @@ -903,6 +903,9 @@ fn legacy_thai_font( } let family = pdfcraft_fonts::embedding::face_key(&source.base_font); let mut candidates = vec![source.font.clone()]; + // Every name on the page is read here for its face name alone, aliases of one font and other + // faces' fonts too, with no total, as before: charged to the edit's allowance, each alias would + // pay again and other faces would spend what the face's subsets need. candidates.extend(fonts.iter().filter_map(|(name, font)| { let font = doc.dict(font)?; (pdfcraft_fonts::embedding::face_key(&Metrics::from_dict(doc, &font).base_font) == family).then(|| String::from_utf8_lossy(name).into_owned()) @@ -917,9 +920,15 @@ fn legacy_thai_font( let face = match object.and_then(|o| cache.values().find(|face| face.object == Some(o))) { Some(alias) => LegacyFace { glyphs: alias.glyphs.clone(), metrics: alias.metrics.clone(), object, ..*alias }, None => { - let metrics = Metrics::from_dict(doc, &font); - let space = source_space(doc, fonts, &metrics); - let glyphs = pdfcraft_fonts::pdf::LegacyThaiFont::from_pdf(doc, &font, space, budget).ok().flatten().map(std::rc::Rc::new); + // Read within the edit's allowance; past it the subset goes unread, and the edit is + // refused below. + let metrics = Metrics::from_dict_within(doc, &font, budget); + let (space, glyphs) = if metrics.past_allowance { + (0.0, None) + } else { + let space = source_space(doc, fonts, &metrics); + (space, pdfcraft_fonts::pdf::LegacyThaiFont::from_pdf(doc, &font, space, budget).ok().flatten().map(std::rc::Rc::new)) + }; let style = FontStyle::of(doc, &font, &metrics); LegacyFace { glyphs, metrics, style, object, space } } @@ -927,7 +936,9 @@ fn legacy_thai_font( cache.insert(name, face); } // A subset left unread would count as unusable, silently changing which subset draws the text. - if budget.remaining() == 0 { + // A read refused for want of the allowance is what refuses the edit: reads that spent it to the + // byte refused nothing. + if budget.refused() > 0 { return Err(EditError::Invalid(format!( "the page's {} subsets need more than {} MiB of font data for one edit; choose a replacement font for this text", source.base_font.rsplit('+').next().unwrap_or(&source.base_font), diff --git a/crates/fonts/src/pdf.rs b/crates/fonts/src/pdf.rs index b67152684e..f68761e02e 100644 --- a/crates/fonts/src/pdf.rs +++ b/crates/fonts/src/pdf.rs @@ -156,8 +156,13 @@ pub fn embedded_sfnt_within(doc: &Document, font: &Dict, budget: &mut DecodeBudg } let mut mappings = std::collections::BTreeMap::new(); if metrics.composite { + let refused = budget.refused(); if let Some((map, _)) = composite_glyph_maps(doc, font, &bytes, &metrics, budget) { mappings = map; + } else if budget.refused() > refused { + // The map the cmap is rebuilt from was left unread for want of the allowance: no + // copy is listed, adapted or as it is, as for metrics read past it. + continue; } if own_cmap { let charmap = face.charmap(); diff --git a/crates/fonts/src/pdf/thai.rs b/crates/fonts/src/pdf/thai.rs index f34111cd5d..84075f87e8 100644 --- a/crates/fonts/src/pdf/thai.rs +++ b/crates/fonts/src/pdf/thai.rs @@ -298,13 +298,17 @@ impl LegacyThaiFont { /// Thai glyphs are not in the legacy order: the nominal and alternate marks this shaper /// assumes can't be trusted there, so it is shaped as an ordinary source font. /// - /// Its streams are decoded against `budget`, which the caller shares between every font it - /// reads: what they keep is the programs decoded and the editor cmaps added to them. + /// Its streams, its metrics' too, are decoded against `budget`, which the caller shares + /// between every font it reads: what they keep is the programs decoded and the editor cmaps + /// added to them. Metrics read past it adapt nothing: the font is refused. pub fn from_pdf(doc: &Document, font: &Dict, space: f64, budget: &mut DecodeBudget) -> Result, String> { - let m = Metrics::from_dict(doc, font); + let m = Metrics::from_dict_within(doc, font, budget); if !m.base_font.rsplit('+').next().unwrap_or(&m.base_font).starts_with("THSarabunPSK") { return Ok(None); } + if m.past_allowance { + return Err("the source Thai font's metrics are past the edit's allowance".into()); + } if !space.is_finite() || space < 0.0 { return Err("the source space width is invalid".into()); } @@ -532,6 +536,46 @@ mod tests { /// An intact legacy map: nominal Thai and two tone alternates. const INTACT: &[u8] = b"2 beginbfrange <00d6> <010f> <0e01> <0110> <012a> <0e3f> endbfrange 2 beginbfchar <0160> <0e48> <0161> <0e49> endbfchar"; + /// A THSarabunPSK subset whose own cmap is Latin-only has its Thai mappings rebuilt from its + /// `/CIDToGIDMap`. With that map left unread for want of the editor's allowance, the editor + /// lists no copy, rather than the program without its Thai mappings. + #[test] + fn a_map_left_unread_for_the_allowance_lists_no_copy() { + let mut doc = Document::new_empty(); + let program = include_bytes!("../../../../assets/fonts/Inter-Regular.ttf"); + let file = Object::Ref(doc.add(Object::Stream(Stream::flate(Dict::new(), program)))); + let map: Vec = (0u16..512).flat_map(u16::to_be_bytes).collect(); + let cid_to_gid = Object::Ref(doc.add(Object::Stream(Stream::flate(Dict::new(), &map)))); + let font = thai_font_over(&mut doc, file, cid_to_gid, INTACT); + let mut budget = DecodeBudget::new(usize::MAX / 2, 0); + assert!(crate::pdf::embedded_sfnt_within(&doc, &font, &mut budget).is_some()); + let cost = usize::MAX / 2 - budget.remaining(); + // The map the cmap is rebuilt from is read last: a byte short of the cost, it is refused. + let mut short = DecodeBudget::new(cost - 1, 0); + assert!(crate::pdf::embedded_sfnt_within(&doc, &font, &mut short).is_none(), "the program is listed without its Thai mappings"); + assert_eq!(short.refused(), 1); + } + + /// `LegacyThaiFont::from_pdf` reads its metrics within the edit's allowance too: the subset's + /// program, for its outlined glyphs, and its ToUnicode are paid for on top of what the + /// editor's read of it pays. Metrics read past the allowance refuse the font before its + /// program is read. + #[test] + fn a_legacy_fonts_metrics_are_paid_for_from_the_edits_allowance() { + let mut doc = Document::new_empty(); + let program = thai_ordered_program(); + let font = thai_font(&mut doc, &program, INTACT); + let mut editor = DecodeBudget::new(usize::MAX / 2, 0); + assert!(crate::pdf::embedded_sfnt_within(&doc, &font, &mut editor).is_some()); + let editor_cost = usize::MAX / 2 - editor.remaining(); + let mut budget = DecodeBudget::new(usize::MAX / 2, 0); + LegacyThaiFont::from_pdf(&doc, &font, 0.25, &mut budget).unwrap(); + assert_eq!(usize::MAX / 2 - budget.remaining(), editor_cost + program.len() + INTACT.len()); + let mut short = DecodeBudget::new(program.len() + INTACT.len() - 1, 0); + let error = LegacyThaiFont::from_pdf(&doc, &font, 0.25, &mut short).err().unwrap_or_default(); + assert!(error.contains("past the edit's allowance"), "{error}"); + } + /// The exporter's damaged map: intact anchors, Latin/control destinations for the other Thai /// codes, and the oversized range first. const DAMAGED: &[u8] = b"begincmap 1 begincodespacerange <0000> endcodespacerange \