[read-fonts] type1: remove unbounded recursion - #1993
Merged
Conversation
Fixes two cases: 1. Binary blob parsing: an integer followed by RD or -| is tokenized as a blob. This used `peek` which called `next` meaning that a series of integers would recurse through `next` calls until a non-integer was detected, leading to potential stack overflow and generally bad performance. Fixed by adding a specialized `peek_is_rd` function that looks for the specific tokens. 2. Procedure parsing: procs can be nested and this was also handled with a recursive call to `next`. Changed to an iterative approach, tracking nesting depth as we do for strings. Additionally: removed `peek` entirely and changed `accept` to just copy the current position of the cloned parser rather than parsing the token twice. Added benchmarks for type1 parsing. This patch improves performance for our test PFA and PFB fonts by 3.6% and 10%, respectively. internal bug ref: b/537783114
cmyr
approved these changes
Aug 5, 2026
| return None; | ||
| } | ||
| let end = self.pos; | ||
| self.pos += 1; |
Contributor
Author
There was a problem hiding this comment.
and I would have gotten away with it too...
we only have one caller and the next token is unconditionally skipped, causing us to parse it twice unnecessarily
This was referenced Aug 5, 2026
dfrg
pushed a commit
that referenced
this pull request
Aug 5, 2026
Thread a depth counter through eval_condition and return RecursionLimitExceeded past GLYF_COMPOSITE_RECURSION_LIMIT, fixing unbounded recursion / stack overflow reachable via the public outline API on a crafted VARC condition tree. Adds regression tests. Same bug class as #1993. Closes #2013 Found with rust-in-peace (https://github.com/scadastrangelove/rust-in-peace).
dfrg
added a commit
that referenced
this pull request
Aug 12, 2026
) Note: patch bumps for font-types and shared-brotli-patch-decoder were already done in the MSRV reduction patch Changes for font-types from font-types-v0.12.2 to 0.12.3 d28db34 [font-types] remove assert from Version16Dot16::new (#2041) 21b17b4 [font-types] add normalize method to bbox (#2043) 8f295ea Lower MSRV to 1.85 and add MSRV CI check 227d12e [chore] Fix some typos (#2005) Changes for read-fonts from read-fonts-v0.42.1 to 0.43.0 39fdc1a [chore] tidy for release (#2045) 78aa50d [read-fonts] fvar: use different normalization path for large number of axes (#2038) 73fd5ce [read-fonts] reduce avar application complexity (#2036) e29b8b5 [read-fonts] avoid panics in classdef intersection (#2028) 4e1f904 [read-fonts] FontAPI: add pure CFF variant to FontKind (#2034) 555ff88 [read-fonts] avoid Device iter subtraction overflow (#2027) 10b5d69 [read-fonts] handle FD Select bounds (#2026) e63756f [read-fonts] Ignore short indexTablesSize when reading EBLC/CBLC index subtables (#2020) 419aa42 fix build issues caused by merging stacked PR a9aa24f [glyph closure] use IntSet is_subset() API 6cd24c9 Add is_subset() method to IntSet. bb6f871 Add is_subset() method to IntSet. e0487b9 [glyph closure] address review comments 950fd5f fix clippy ae4b992 [glyph closure] make fnv feature gated d386422 [glyph closure] add intersected_class_cache e00c732 [closure] Add FnvHashMap, use it as a cache during closure 35ca72a [glyph closure] remove unncessary set allocation 95e1159 [glyph closure] move seen_sequence_indices out of loop 1d3e931 [glyph closure] reduce set allocations for (Chain)ContextFormat1 76c63d8 [closure] reduce set allocations for (Chain)ContextFormat1 40b07af [glyph closure] reduce allocations for (Chain)ContextFormat2 3dfbae6 [glyph closure] add a fn in classdef 7d464b8 [read-fonts] aat: use wrapping to avoid panics on 32-bit (#2017) d0f5d07 [read-fonts] avoid overflow panics in CFF charset (#2016) 8f295ea Lower MSRV to 1.85 and add MSRV CI check f95a2db [read-fonts] type1: remove unbounded recursion (#1993) 227d12e [chore] Fix some typos (#2005) d876963 [read-fonts] limit work in charstring eval (#2000) 40cd87d Remove support for IFT Patch Map Format 1. 8f5f551 [read-fonts] apply imageDataOffset to format 4 sbit offsets (#1994) 746053e fix clippy d394729 [glyph closure] remove unused fn 823c1ea [glyph closure] try to reduce allocations by splitting borrows 8ec1dca [read-fonts] avoid range reset for cmap12/13 iter (#1986) fbe0cb3 [read-fonts] optimize post glyph name iter (#1983) Changes for font-test-data from font-test-data-v0.8.0 to 0.9.0 8f295ea Lower MSRV to 1.85 and add MSRV CI check 40cd87d Remove support for IFT Patch Map Format 1. fbe0cb3 [read-fonts] optimize post glyph name iter (#1983) Changes for write-fonts from write-fonts-v0.51.0 to 0.52.0 8f295ea Lower MSRV to 1.85 and add MSRV CI check 227d12e [chore] Fix some typos (#2005) 40cd87d Remove support for IFT Patch Map Format 1. Changes for skrifa from skrifa-v0.45.1 to 0.46.0 c244623 [skrifa] tthint: ignore fractional bits in parity checks- #2040 #2040 f3edd9d [skrifa] tthint: avoid overflow in line_vector (#2039) 60877df [skrifa] autohint: avoid panic on font with 0 glyphs (#2044) 29ae374 [skrifa] COLRv1: avoid unbalanced stack state on error (#2031) e829e72 [skrifa] VARC: limit number of edges processed (#2032) 9bbb04d [skrifa] COLRv1: remove nested traversal optimization and limit total work (#2009) 487a333 [skrifa] avoid more overflow panics in TT hinting (#2011) 8f295ea Lower MSRV to 1.85 and add MSRV CI check f43403c skrifa/varc: don't panic on a null MultiItemVariationStore offset (#2012) 867fe4d skrifa/varc: bound condition-evaluation recursion (#2014) 5d8cc1d [skrifa] autohint: explicit guards for indexed access (#1988) 38fdf8c [skrifa] autohint: ensure indices fit in u16 (#1989) b7487ff [skrifa] autohint: avoid panic on overflow (#1987) 1041d90 [skrifa] smallvec: don't pretend we do fallible alloc (#1990) fbe0cb3 [read-fonts] optimize post glyph name iter (#1983) Changes for skera from skera-v0.5.1 to 0.6.0 961143e [skera-repacker] use enum type for parents b65f470 [skera-repack] single parent optimization 8f295ea Lower MSRV to 1.85 and add MSRV CI check 4d03189 [skera] add doc for profiling skera 227d12e [chore] Fix some typos (#2005) aeede19 [skera] name: don't panic on out-of-range name record count 37c89dd [skera] move plan creation backc into interation loop 21571e8 [skera] move plan creation out of iterating loop 739ee69 [skera] tweak Benchmark suite 0d5779d [skera-repack] revert previous change to add_parent() 0c818f8 [skera] fix comment a16ef39 [skera-repack] fix fix_virtual_links() in ligatureSubst splitting 3213a55 [skera-rapack] remove unused fn d2b3791 [skera-repack] fix LigastureSubst table splitting 660a4f8 [skera-repack] fix MarkBasePos table splitting aa57058 [skera-repack] make a copy of shared ClassDef before mutation 0d01f50 [skera-repack] make a copy of shared coverage table before mutation ad9d216 [skera] update remap_child() dd3b9cf [skera-repack] fix is_shared() c54a417 [skera] add copy_data option for duplicate_vertex() Changes for shared-brotli-patch-decoder from shared-brotli-patch-decoder-v0.1.4 to 0.1.5 8f295ea Lower MSRV to 1.85 and add MSRV CI check Changes for incremental-font-transfer from incremental-font-transfer-v0.6.0 to 0.7.0 8f295ea Lower MSRV to 1.85 and add MSRV CI check 40cd87d Remove support for IFT Patch Map Format 1.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes two cases:
Binary blob parsing: an integer followed by RD or -| is tokenized as a blob. This used
peekwhich callednextmeaning that a series of integers would recurse throughnextcalls until a non-integer was detected, leading to potential stack overflow and generally bad performance. Fixed by adding a specializedaccept_blob_startfunction that looks for the specific tokens.Procedure parsing: procs can be nested and this was also handled with a recursive call to
next. Changed to an iterative approach, tracking nesting depth as we do for strings.Additionally: removed
peekentirely and changedacceptto just copy the current position of the cloned parser rather than parsing the token twice.Added benchmarks for type1 parsing. This patch improves performance for our test PFA and PFB fonts by 3.6% and 10%, respectively.
internal bug ref: b/537783114