fix: Ignore the deprecated dotsection operator in CFF charstrings - #228
Open
tobocop2 wants to merge 1 commit into
Open
fix: Ignore the deprecated dotsection operator in CFF charstrings#228tobocop2 wants to merge 1 commit into
tobocop2 wants to merge 1 commit into
Conversation
dotsection (escape 12 0) is a Type 1 hinting operator that Adobe's
Type 1 to Type 2 conversion preserves on dot-bearing glyphs, so
distiller-produced Type 1C subsets carry it routinely. The two-byte
operator match only knew the flex family and returned
CFFError::UnsupportedOperator, which aborted the entire charstring:
glyphs such as i, j, period, colon, exclam and question yielded no
outline at all instead of an outline missing hinting metadata.
The operator takes no arguments and only brackets outline sections
for hinters, so treat it as a pure no-op, matching read-fonts
('Nothing to do.') and FreeType. A second test pins the no-op
semantics: operands already on the stack survive it untouched.
tobocop2
force-pushed
the
cff1-dotsection
branch
from
August 2, 2026 19:56
7883db3 to
023f816
Compare
This was referenced Aug 2, 2026
tobocop2
added a commit
to tobocop2/kreuzberg
that referenced
this pull request
Aug 5, 2026
…tion fix Upstream harfbuzz/ttf-parser is currently unmaintained: at the time of vendoring the most recent commit was 2025-11-22 and correctness fixes were sitting unreviewed, so waiting on a release is not a plan. This commit is upstream source only, byte-identical to the v0.25.1 tag with upstream harfbuzz/ttf-parser#228 (commit 023f8163) cherry-picked on top. Keeping it pristine means a reviewer can diff it directly against upstream, and future upstream fixes cherry-pick without conflict. All xberg-side modifications land separately in the following commit. Not vendored: benches/, examples/, c-api/, testing-tools/, meson.build.
tobocop2
added a commit
to tobocop2/kreuzberg
that referenced
this pull request
Aug 5, 2026
Upstream pull request harfbuzz/ttf-parser#228, commit 023f8163, applied verbatim to the vendored v0.25.1 tree. ttf-parser rejects any CFF charstring containing the deprecated `dotsection` operator (escape `12 0`) with UnsupportedOperator, which aborts the whole charstring and leaves the glyph with no outline. Adobe's Type 1 to Type 2 conversion preserves the operator, so real-world fonts still carry it, and the glyphs that carry it are exactly the dot-bearing ones: i, j, ! and . A page rendered from such a PDF silently loses those characters, and OCR run over the page transcribes the gaps. The operator takes no arguments and is a no-op for outlining, matching read-fonts and FreeType. It is now skipped rather than rejected. Upstream's own two regression cases come with it and pin the behavior, including that operands already on the argument stack survive it untouched.
Goldziher
added a commit
to xberg-io/xberg
that referenced
this pull request
Aug 5, 2026
…nreleased correctness and DoS fixes (#1384) * chore(ttf-parser): vendor upstream ttf-parser v0.25.1 verbatim Upstream harfbuzz/ttf-parser is currently unmaintained: at the time of vendoring the most recent commit was 2025-11-22 and correctness fixes were sitting unreviewed, so waiting on a release is not a plan. This commit is the upstream v0.25.1 tag, unmodified, and nothing else. Every upstream fix we carry lands as its own commit on top, so `git log` shows exactly which upstream pull request was taken and when, and each one can be reviewed and reverted independently. All xberg-side modifications land in a final separate commit. Not vendored: benches/, examples/, c-api/, testing-tools/, meson.build. * fix(ttf-parser): apply upstream harfbuzz/ttf-parser#228 (dotsection) Upstream pull request harfbuzz/ttf-parser#228, commit 023f8163, applied verbatim to the vendored v0.25.1 tree. ttf-parser rejects any CFF charstring containing the deprecated `dotsection` operator (escape `12 0`) with UnsupportedOperator, which aborts the whole charstring and leaves the glyph with no outline. Adobe's Type 1 to Type 2 conversion preserves the operator, so real-world fonts still carry it, and the glyphs that carry it are exactly the dot-bearing ones: i, j, ! and . A page rendered from such a PDF silently loses those characters, and OCR run over the page transcribes the gaps. The operator takes no arguments and is a no-op for outlining, matching read-fonts and FreeType. It is now skipped rather than rejected. Upstream's own two regression cases come with it and pin the behavior, including that operands already on the argument stack survive it untouched. * docs(ttf-parser): document Face::style() OS/2 fallback (upstream #203) Upstream pull request harfbuzz/ttf-parser#203, commit b422ac0, applied verbatim to the vendored v0.25.1 tree. Face::style() reads the style from the OS/2 table and falls back to the head table's mac style bits when OS/2 is absent. The rustdoc did not say so, leaving callers to read the implementation to find out which table actually answers the query. Documentation only; no behavior change. * fix(ttf-parser): apply upstream harfbuzz/ttf-parser#222 (CFF2 BLEND stack guard) Upstream pull request harfbuzz/ttf-parser#222, commit 3a585f1, applied verbatim to the vendored v0.25.1 tree. The CFF2 `blend` operator pops its operand count off the argument stack without first checking that the stack holds anything. `ArgumentsStack::pop` is unchecked, so a charstring that reaches `blend` with an empty stack reads a stale slot and carries on with a bogus operand count. Every other pop in the CFF2 interpreter already guards its stack depth; this was the last one that did not. It now returns InvalidArgumentsStackLength like the rest. * fix(ttf-parser): apply upstream harfbuzz/ttf-parser#223 (avar i16 overflow) Upstream pull request harfbuzz/ttf-parser#223, commit 32439d2, applied verbatim to the vendored v0.25.1 tree. avar::map_value computed `value - from + to` in i16 at three sites. A segment map whose from and to coordinates sit at opposite extremes pushes that expression outside i16, and with overflow checks off in release it wraps silently and yields a normalized coordinate pointing at the wrong place on the axis. The arithmetic is promoted to i32 behind a shared shift_coordinate helper, which rejects out-of-range results rather than wrapping them. One i32 multiply in the interpolation branch of the same function can still overflow. That is a separate pre-existing defect, untouched by this patch, and is being reported upstream rather than diverged here. * fix(ttf-parser): cap total paint-graph node visits in COLRv1 painting (upstream #225) Upstream pull request harfbuzz/ttf-parser#225, commit 99aa5e3, applied verbatim to the vendored v0.25.1 tree. RecursionStack detects a cycle only on the current root-to-leaf path, since an entry is popped as soon as its call returns. A paint graph shaped as a DAG, where the same offset is reachable through several sibling branches and no single branch cycles, slips past it entirely: a chain of PaintColrLayers records whose layer slots all point at one shared next-level record forces branching^depth calls while never re-entering the active path. A total-visit budget now bounds the work for one top-level paint call regardless of branching factor. The comment on the new constant refers to glyf::MAX_COMPONENT_VISITS, which upstream #224 introduces; that pick lands later in this branch. * fix(ttf-parser): parse loca for fonts with the maximum 65535 glyphs (upstream #226) Upstream pull request harfbuzz/ttf-parser#226, commit 86daf57, applied verbatim to the vendored v0.25.1 tree. A loca table holds numGlyphs + 1 offsets, so a font at the spec maximum of 65535 glyphs has 65536 of them. The offset count was held in a u16, and u16::try_from(65536) fails, which made Table::parse return None for the whole table. Face::parse then leaves glyf as None, so the face yields no TrueType outlines at all, and the gvar path dies with it through the `glyf?` in Face::outline_glyph. A second, narrower instance of the same overflow made the last glyph unreachable even when the table did parse. The counters and the offset array widen to u32, which also makes good on the existing comment about ignoring a longer-than-expected loca. The array read stays bounded at numGlyphs + 1 entries, so nothing new is allocated or read. This is a latent fix rather than a live one: a scan of 452 system fonts plus the fixtures in tree found no glyf-flavored face large enough to trigger it. It also widens the public loca::Table signature, which no crate in this workspace's dependency graph calls. * fix(ttf-parser): read fvar HIDDEN_AXIS from bit 0, not reserved bit 3 (upstream #216) Upstream pull request harfbuzz/ttf-parser#216, commit 9b9e55f, applied verbatim to the vendored v0.25.1 tree. The fvar axis record's HIDDEN_AXIS flag is bit 0. VariationAxis::parse read bit 3, which the spec reserves and requires to be clear, so hidden was initialized from a bit that carries no meaning: every hidden axis was reported as visible, and any font that set the reserved bit was reported as hidden. Confirmed against the spec and against skrifa, which defines AXIS_HIDDEN_FLAG as 0x1. A regression test follows in the next commit; the one-line change is silently reversible and reverting it breaks nothing in the existing suite. * test(ttf-parser): pin fvar HIDDEN_AXIS bit-0 decoding xberg-authored follow-up to the previous commit. Upstream #216 ships no test, and nothing in the existing suite reads VariationAxis::hidden, so reverting the one-line fix leaves every test green. Three cases over a synthetic single-axis fvar table: bit 0 set means hidden, no flags means visible, and reserved bit 3 set means visible. The last one is what pins the change rather than smoke-testing around it, since it fails in the opposite direction if the old `flags >> 3` reading comes back. The test lives at the top level of tests/ rather than in tests/tables/, because adding a module there would mean editing tests/tables/main.rs, which is kept byte-identical to upstream so future cherry-picks stay conflict-free. No feature gate: the dev-dependency on compat/ pulls default features, which unifies variable-fonts onto the crate in every test build. * fix(ttf-parser): return None from set_variation for an unknown axis tag (upstream #207) Upstream pull request harfbuzz/ttf-parser#207, commit 52a9811, applied verbatim to the vendored v0.25.1 tree. Face::set_variation documents that it returns None when the face has no such variation axis, but it returned Some(()) unconditionally: the loop simply found no matching tag, changed nothing, and reported success. Callers could not tell a typo'd or unsupported axis from an applied one. The maintainer's open question on the PR was whether the docs were merely outdated. They are not: pre-0.20 the method carried an explicit else branch returning None, and the rewrite that turned it into a loop dropped it. This restores the documented contract rather than inventing one. A regression test follows in the next commit; nothing in the existing suite asserts this return value. * test(ttf-parser): pin set_variation unknown-axis contract xberg-authored follow-up to the previous commit. Upstream #207 ships no test and nothing in the existing suite asserts what set_variation returns, so the restored contract would silently rot. Three arms over fixtures already in tree. Only the first exercises the fix: an unknown tag on a variable face returned Some(()) before #207 and returns None after, verified by reverting the change on a pod and watching this test go red. The known-tag case guards against over-correcting into always-None. The non-variable case is already handled by the `!self.is_variable()` guard ahead of the loop, so it passes either way; it is kept for the other half of the documented contract and labelled as such rather than being left to look like coverage it does not provide. The test lives at the top level of tests/ rather than in tests/tables/, so tests/tables/main.rs stays byte-identical to upstream. * fix(ttf-parser): cap total component visits in glyf/gvar composite outlining (upstream #224) Upstream pull request harfbuzz/ttf-parser#224, commit dd2337b, applied verbatim to the vendored v0.25.1 tree. MAX_COMPONENTS bounds the depth of a single component chain, not how often a component subtree is revisited through sibling components. A composite glyph whose components all point at one shared child forces branching^depth calls while staying under the depth cap and needing only depth+1 distinct glyphs. Both outline_impl and outline_var_impl now carry a shared visit budget from the top-level entry point. The budget as upstream wrote it charges one unit per call, which bounds the number of calls but not the work inside them, since a single glyph can carry up to 65535 component records. The next commit charges per record. * fix(ttf-parser): charge the glyf/gvar visit budget per component record xberg-authored follow-up to the previous commit. Upstream #224's budget is charged once on entry to outline_impl / outline_var_impl, which bounds the number of calls but not the work inside them: a single composite glyph can carry up to 65535 component records, and components whose glyph range does not resolve are iterated and skipped without ever recursing. A glyph padded with unresolvable components therefore costs one budget unit no matter how wide it is, and can be re-walked until the depth cap stops it. The budget is now charged per component record in both loops, so total work is bounded by MAX_COMPONENT_VISITS rather than only total call count. Charging components_count up front in gvar would not do: it is computed as `.count() as u16`, so a glyph with an exact multiple of 65536 components truncates to zero and is charged nothing. MAX_COMPONENT_VISITS stays at 100_000, still around four orders of magnitude above any real font, and the per-record cost is O(1). No test: the observable difference is a timing bound, and a timing assertion would be flaky in CI. This is the first behavior-affecting xberg divergence inside the vendored src/ tree, so both sites carry a comment saying so. * test(ttf-parser): pin a total-visit budget on the CFF charstring interpreter xberg-authored regression test, committed before the fix. A single glyph whose charstring calls one global subroutine 150,000 times, each call flat (depth 1) so it stays under the nesting limit, drives one interpreter invocation per call. Without a total-visit budget the glyph outlines successfully after doing all the work; the test asserts outlining fails instead. Deterministic, not timing-based: the assertion is on the return value. Applied on its own on a pod, the test fails (the glyph outlines) and the following commit makes it pass. Lives at the top level of tests/ so tests/tables/main.rs stays byte-identical to upstream. This commit alone is red; the interpreter has no budget yet. The fix is next. * fix(ttf-parser): cap total charstring visits in the CFF interpreter xberg-authored, not from an upstream PR. Upstream #224 and #225 cap the total work of glyf/gvar composite outlining and COLRv1 painting, but the CFF charstring interpreter had no equivalent, and upstream (unmaintained) does not address it. STACK_LIMIT bounds how deeply subroutine calls nest on the current path, not how many are made in total. A charstring that calls a subroutine over and over without nesting stays under STACK_LIMIT forever and drives one _parse_char_string invocation per call with no ceiling, exponential in the worst case. This is the path a PDF renderer exercises most, since CFF is the dominant embedded-font flavor. Both the CFF1 and CFF2 interpreters now carry a MAX_CHARSTRING_VISITS budget of 100_000, charged once per invocation through the shared parser context and returning the existing NestingLimitReached error when exhausted. A real glyph needs a few hundred. Reusing the existing error keeps the public API unchanged. Mirrors the shape of upstream #224. This is the second behavior-affecting xberg change inside the vendored src/ tree, alongside the #224 per-record budget. The previous commit's regression test now passes. * feat(ttf-parser): route the transitive ttf-parser onto the vendored copy Nothing in xberg depends on ttf-parser directly; it arrives through pdf_oxide, fontdb, lopdf and rustybuzz, which request it by its crates.io name. Cargo matches [patch.crates-io] entries on package name alone, so a crate named xberg-ttf-parser cannot redirect them. The compat/ subcrate carries the upstream name, re-exports the vendored parser, and is what the workspace patch entry points at. One parser is compiled, so types stay identical across every consumer. It is never published. The only edits to vendored source are 17 lines of crate-level lint allows. The workspace lint set is stricter than upstream's CI, and these are allowed rather than fixed because div_ceil and is_multiple_of postdate upstream's 1.63 MSRV, so rewriting them would break upstream's own build. For the same reason the vendored tree is excluded from poly formatting. The vendored tests keep referring to ttf_parser via a path-only dev-dependency on the shim, which cargo strips when packaging. tiny-skia-path is bumped to the 0.12 already in the workspace so the lockfile does not carry two copies. xberg-ttf-parser is published alongside the other vendored crates. Note that [patch] does not propagate to downstream crates.io consumers: this fixes every artifact we build and ship, not people who depend on xberg from the registry. * test(pdf): render a dotsection CFF font end to end through the vendored parser Carries the regression test from the superseded PR #1363, which proves the user-visible fix: that xberg actually paints the dot-bearing glyphs, not merely that the parser accepts the operator. The vendored crate's own tests cover the charstring interpreter; nothing here proved a glyph reaches the page. Renders a single-page PDF embedding a synthetic Type 1C font (fontTools-built, no third-party font data) whose i, j, period, colon, semicolon, exclam and question glyphs carry the deprecated dotsection operator the way Adobe's Type 1 to Type 2 converter emits it. It rasterizes through the production render path and asserts per-glyph ink; three control glyphs without dotsection guard the fixture itself. Stock ttf-parser 0.25.1 drops all seven dotsection glyphs, so this passes only because the workspace routes onto the vendored parser. Gated on the `pdf` feature, which CI reaches through `full` -> `formats` -> `pdf`, so `cargo test -p xberg --features full` runs it. Verified locally as well (Apple M1 Pro, macOS 14 arm64): `cargo test -p xberg --features pdf` green. --------- Co-authored-by: Na'aman Hirschfeld <nhirschfeld@gmail.com>
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
1 participant
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.
Problem
CFF glyphs whose charstrings carry the deprecated
dotsectionoperator (escape12 0) lose their outlines entirely: the two-byte operator match knows only the flex family and aborts the whole charstring withUnsupportedOperator. Adobe's Type 1 to Type 2 conversion preservesdotsectionon dot-bearing glyphs, so distiller-produced Type 1C subsets drop everyi,j,.,:,;,!and?through any rasterizer sitting on ttf-parser.Fixes #227.
Fix
Treat
dotsectionas a pure no-op in the two-byte match, matching read-fonts ("Nothing to do.") and FreeType. It takes no arguments and only brackets outline sections for hinters.Validation
Two new charstring tests: one proves the glyph outlines instead of erroring, one pins the no-op semantics (operands on the stack survive; a
stack.clear()variant fails it). On the Century-Light subset from the issue, the four dropped glyphs return with correct outlines while control glyphs keep byte-identical segment counts (10, 7, 21), so the change is purely additive. Full suite: 151 passing; clippy warning count unchanged against main.