Skip to content

Adopt harfrust's caller-set shaping limits once harfrust#571 ships #580

Open

Description

@yhay81

Context

#493 asked for harfrust shaping to charge the operation budget, and to poll cancellation and the deadline while it runs (DESIGN.md F6, sections 4.6 and 9). harfbuzz/harfrust#571 proposes caller-set max_len and max_ops and a cancellation callback. behdad deferred that to dfrg, and a typed error for malformed fonts is unlikely upstream. The owner accepted the residual (2026-11-02), as on #509. FontFace::shape checks the text's characters against shaped_glyph_count before shaping and polls between runs, but one shaper.shape call cannot be cancelled or charged from inside.

Goal

Once a harfrust release lets callers bound or cancel one shape call, adopt it in crates/pyropdf-engine/src/fonts/shape.rs.

Scope

In: following harfrust#571, upgrading harfrust when a suitable release is available (pyropdf already uses 0.15.0 through merged PR #620), and passing the operation's limits and cancellation into the shaper. Out: forking harfrust, and publishing upstream without the owner's go-ahead.

Paths

  • crates/pyropdf-engine/src/fonts/shape.rs and its tests
  • crates/pyropdf-engine/Cargo.toml and Cargo.lock

Acceptance

  • A harfrust release with caller-set limits or cancellation is adopted.
  • Tests exercise adversarial substitution expansion, small budgets, cancellation, and normal Latin, RTL, and complex-script shaping, with typed refusals.
  • Full project checks, the dependency audit, and current-head CI pass.

Notes

The upstream discussion is in #493's comments.

Verified on 2026-11-06: #620 merged on 2026-11-03. Current shaping checks Buffer::allocation_successful() and refuses a run exhausting harfrust's internal allowance as a font error; caller-set limits and in-call cancellation remain unavailable.

harfrust#589 is now merged as 859d44ff37806bdaa437b1fc75e1ea98ee977c52, but latest published harfrust remains 0.15.0 (2026-10-22). That accounting fix is a distinct partial improvement, not completion of harfrust#571, which remains open. A release containing #589 can be evaluated for adoption without claiming caller-set caps, cancellation or the separate preallocation length-bound gap are solved. Keep this issue open until its full acceptance criteria hold.

Activity

  1. added
    upstreamWork that lands in an engine or dependency repository
    area:enginecrates/pyropdf-engine: interpretation, rendering, writing, understanding
    upstream:filedUpstream: issue or pull request filed upstream; waits on the maintainer
    engine:harfrustEngine or dependency: harfrust
    on Nov 2, 2026
  2. yhay81 commented on Nov 3, 2026

    @yhay81
    OwnerAuthor

    Upstream status, 2026-11-03 (harfbuzz/harfrust#571):

    • behdad reproduced the expansion case on harfrust 0.15.0 and main (2026-11-02): one A that ccmp expands to 30,000 glyphs, then 2,000 lookups, takes about 0.7 s, and ordinary GSUB/GPOS passes are not charged to max_ops at all (the 65,536-operation budget stays untouched).
    • [ot] Charge GSUB/GPOS matching and lookup walks to max_ops harfbuzz/harfrust#589 (open, by behdad) charges GSUB/GPOS matching and lookup walks to max_ops. On the same case it stops the trailing passes after about 2 ms, keeps best-effort glyph output, and still returns Ok(()).
    • A separate gap: a buffer preallocated with reserve skips the max_len check (it only runs in enlarge()), so four As reach 120,000 glyphs. behdad recommends fixing that before exposing max_len as a caller bound, and treats a typed error versus a buffer status as a separate question.

    What this means here: #589 alone would bound the work of one shaper.shape call, but not cancel it or charge pyropdf's budget, and exhaustion would surface as a status rather than an error. The caller-set limits and cancellation that this issue's Goal names are still unanswered (deferred to dfrg).

    Next action: follow harfbuzz/harfrust#589 and the max_len enforcement gap. Once a harfrust release contains #589, decide whether adopting it (from 0.13.3; 0.15.0 is current) closes enough of this issue or whether it stays open for caller-set limits. Upstream state stays upstream:filed.

  3. yhay81 commented on Nov 3, 2026

    @yhay81
    OwnerAuthor

    PR #620 moves pyropdf to harfrust 0.15.0, which still has no public setter for max_len or max_ops, so this stays blocked on harfbuzz/harfrust#571. Once it merges, shaping also refuses a run that exhausts harfrust's own allowance (Buffer::allocation_successful) as a font error.

  4. added
    in-progressClaimed; the latest comment names the branch
    on Nov 5, 2026
  5. yhay81 commented on Nov 5, 2026

    @yhay81
    OwnerAuthor

    Audited on codex/580-shaping-audit at 13ea858a096493bb49e681c450a2a44302ecfaa7; no code change or new PR. Corrected the stale 0.13.3 migration instruction: #620 merged on 2026-11-03 and crates/pyropdf-engine/Cargo.toml already specifies 0.15.0. Current shaping checks allocation_successful() after harfrust::shape, preserving typed font refusal for an exhausted internal allowance.

    harfrust#589 is MERGED as 859d44ff37806bdaa437b1fc75e1ea98ee977c52. Latest published version remains 0.15.0 (2026-10-22), so no containing release is available yet. harfrust#571 stays open; its caller caps/cancellation and the separately reported preallocated-buffer length enforcement are not established by #589's work accounting.

    Next action: inspect the next release for #589 and evaluate that partial improvement with the adversarial expansion case, preserving exhaustion detection. Continue tracking #571 for actual caller-set limits/cancellation; do not close this issue merely after adopting fixed default accounting. Full adoption still requires small-budget, cancellation, Latin/RTL/complex-script regressions, project checks and dependency audit. No fork or upstream message was made. Claim released; upstream:filed retained.

  6. removed
    in-progressClaimed; the latest comment names the branch
    on Nov 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area:enginecrates/pyropdf-engine: interpretation, rendering, writing, understandingengine:harfrustEngine or dependency: harfrustupstreamWork that lands in an engine or dependency repositoryupstream:filedUpstream: issue or pull request filed upstream; waits on the maintainer

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions