Skip to content

hayro: no panics on malformed input (I3) #19

Closed

Description

@yhay81

Context

DESIGN.md §6, I3. hayro 0.8.0 panics on some malformed input, for example on colour components that do not match their colour space (LaurenzV/hayro#1546). pylopdf contains every interpretation in catch_unwind and maps a panic to PdfError.

Goal

Malformed input yields errors from hayro, and catch_unwind stays in pyropdf only as a last defense that its tests never reach.

Scope

Acceptance

  • The fuzz target runs in CI without a panic for its time budget

Activity

  1. added this to the Phase 1: Reading milestone on Oct 24, 2026
  2. added
    upstreamWork that lands in an engine or dependency repository
    adoptionpyropdf adopts a released engine change
    area:enginecrates/pyropdf-engine: interpretation, rendering, writing, understanding
    engine:hayroEngine or dependency: hayro
    on Oct 24, 2026
  3. yhay81 commented on Oct 24, 2026

    @yhay81
    OwnerAuthor

    hayro-syntax 0.8.0 also panics, found while doing #22: ArrayIter::next (object/array.rs, the unwrap after read_with_context) and Dict::entries unwrap an element that the parser skipped but cannot read. Inputs that reach it through the public FromBytes:

    • [/AB#FG]: a name whose # is not followed by two hexadecimal digits;
    • [+ 1]: a lone +, which Number::skip accepts and Number::read refuses;
    • [<< /AB#FG /X >>]: a dictionary that Dict::skip accepts and Dict::read refuses.

    hayro's main fixes both unwraps in LaurenzV/hayro#1501 ("Don't panic on unreadable values in dictionaries and arrays"), and LaurenzV/hayro#1568 keeps a # that is not an escape; neither is released yet. Until the adoption (#7, "Adopt released versions"), the store converts objects inside catch_unwind and maps a panic to a malformed-input error. Its test a_panic_in_hayro_syntax_is_a_malformed_input_error in crates/pyropdf-store/src/value/tests.rs changes with the adoption.

  4. yhay81 commented on Oct 27, 2026

    @yhay81
    OwnerAuthor

    Upstream status, checked 2026-10-28 against LaurenzV/hayro main 8558a8a5:

    • Every hayro panic this issue names is fixed on main: #1501 (unreadable array and dict values), #1562 (colour components that do not match their colour space, the #1546 cases), and #1568 (a # that is not an escape). None is released yet: the latest tags are still hayro-interpret-v0.8.0 and its siblings. No local hayro patch is needed; this issue waits for a hayro release and its adoption (Engine work: hayro #7).
    • The remaining open case is vello_cpu#646, which pylopdf#338 tracks (it keeps contain_renderer_panics). That is a linebender repository, and pylopdf's records hold no minimal reproduction, so I have prepared no patch for it. A reproduction first needs the fuzz target this issue asks for.
    • That fuzz target is pyropdf work and could start now. It would fail on 0.8.0's known panics until the adoption, so its CI acceptance waits for the release.

    Next action: when hayro 0.9 is released, adopt it, add the interpretation fuzz target, and record any vello_cpu panic it finds with a reproduction.

  5. added
    in-progressClaimed; the latest comment names the branch
    on Oct 30, 2026
  6. yhay81 commented on Oct 30, 2026

    @yhay81
    OwnerAuthor

    Claimed for the interpretation fuzz target in Scope: branch claude/19-fuzz-interpret. The target's CI run stays off until the hayro 0.9 adoption (#7).

  7. yhay81 commented on Oct 30, 2026

    @yhay81
    OwnerAuthor

    Handoff (branch claude/19-fuzz-interpret, PR #391, awaiting owner review):

    Next action: after #7 adopts hayro 0.9, add interpret to the matrix of fuzz.yml and check this issue's acceptance; fix #392 in the meantime.

  8. removed
    in-progressClaimed; the latest comment names the branch
    on Oct 30, 2026
  9. added
    upstream:mergedUpstream: merged upstream; waits on a release to adopt
    and removed
    upstreamWork that lands in an engine or dependency repository
    upstream:mergedUpstream: merged upstream; waits on a release to adopt
    on Nov 2, 2026
  10. yhay81 commented on Nov 3, 2026

    @yhay81
    OwnerAuthor

    Triage, 2026-11-03: hayro 0.9.0 (released 2026-11-02) contains the upstream fixes this issue waited on, so I replaced upstream and upstream:merged with adoption. pyropdf cannot adopt 0.9 yet: it drops OutlineGlyph::font_cache_key() and SoftMask::id(), which the engine reads. #584 tracks putting them back (LaurenzV/hayro#1734, awaiting review), so this issue is now blocked by #584. The 0.9 bump itself resumes on #18's branch claude/18-hayro-0.9-u32.

    Next action: once a hayro release contains LaurenzV/hayro#1734 and the bump lands, adopt this issue's change.

  11. yhay81 commented on Nov 5, 2026

    @yhay81
    OwnerAuthor

    Dependency triage, 2026-11-06: removed the obsolete blocked-by link to #584. PR #609 already merged the owner-approved hayro 0.9-based commit 394e4a0d37baf8af513f6ebda15f4bada85d6f23, and both the workspace and fuzz manifests use it. This issue's 0.9 capability is available now; #603 separately retains the #584 dependency for returning to published crates. This does not close #584 or waive any acceptance criterion here.

    Next action: Enable and validate the existing interpretation fuzz target against the current dependency, and investigate any panic it finds.

    No implementation claim is held by this triage. Recheck the live queue, claim a branch and complete the existing acceptance before closing.

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

    @yhay81
    OwnerAuthor

    Handoff on codex/19-interpret-fuzz-ci, PR #708, head 5f915ec1be17cfe410c15a84d14846a69ff66598. Enabled the existing panic-sensitive interpretation target; refreshed only fuzz lockfile local versions and documented the nine-job budget. The branch is pushed and clean.

    All eight locked local checks passed (Python 7,398 passed, 705 skipped), as did native Windows interpretation-target cargo check --locked, actionlint 1.7.12, and cargo-audit 0.22.2 for both lockfiles. Seed generation produced 343 interpretation inputs.

    CI run 42122505556 has completed successfully. Fuzz run 42122505562 has eight existing targets successful; interpretation job 127578212523 is confirmed in progress in the Fuzz step. This step includes compilation. Keep observing this run; do not restart merely because it is slow. No merge until this check succeeds and its log proves the configured fuzz run completed. Reviews/comments/threads were empty at inspection.

    The implementation claim is released while this external job runs; the open PR keeps #19 out of the new-work queue. Next: inspect its result/artifacts, reclaim for any fix, then merge and verify main if acceptance succeeds. Independent #365 measurement can proceed in a fresh session while this run continues. No owner approval is pending.

  14. yhay81 commented on Nov 5, 2026

    @yhay81
    OwnerAuthor

    Known-panic follow-up while PR #708's interpretation job runs: GitHub compare confirms hayro 0.9.0 contains the #1562 fix (0a4baaf99bcab9e93cfefd7eb712ee76161b496d). The width/tile overflow discovered by this target is guarded locally by #392/#406 (MAX_WIDTH = 65_408); vello#2040 is still open. Other live upstream reports are vello#2037 (2 GiB alpha scene) and vello#2005 (fuzz findings). The old vello_cpu#646 shorthand in pylopdf#338 still does not identify a resolvable matching report, so it is not claimed fixed.

    A successful configured CI fuzz run meets this issue's bounded regression acceptance; it is not proof that every malformed input is panic-free. Keep the last-defense panic boundary and track any new finding with its reproduction. Current CI run 42122505556 has succeeded; interpretation job 127578212523 in Fuzz run 42122505562 is still confirmed running.

  15. removed
    in-progressClaimed; the latest comment names the branch
    on Nov 5, 2026
  16. yhay81 commented on Nov 5, 2026

    @yhay81
    OwnerAuthor

    Merged #708 as 938b94d (branch codex/19-interpret-fuzz-ci). All 14 current-head checks passed; no unresolved review threads. The interpretation job in run 42122505562 loaded 343 seed files and completed 453 executions in 178 seconds without a panic (797 MiB RSS). Seed initialization exceeded -max_total_time=60, so this PR run establishes seed regression coverage, not additional mutation coverage; the weekly 15-minute configuration provides a longer exploration budget. The default libFuzzer panic hook remains intact.

    Release adoption and remaining upstream panic reports are audited in #19 (comment). The last-defense panic boundary remains. Next: verify main CI for the merge; investigate any future fuzz crash without weakening the hook.

  17. yhay81 commented on Nov 5, 2026

    @yhay81
    OwnerAuthor

    Post-merge verification: the issue is closed, no in-progress label remains, and main CI 42125740133 for 938b94d succeeded. Main Fuzz run 42125740283 is still running its interpretation job; all eight other fuzz jobs passed. Continue that existing run and investigate any failure. The next-work report was refreshed after merge.

  18. yhay81 commented on Nov 5, 2026

    @yhay81
    OwnerAuthor

    Final post-merge verification for 938b94d5f9584c5c21a013265a3c8c4cfe0714b4: main CI 42125740133 and all nine jobs in Fuzz 42125740283 succeeded. Interpretation loaded 343 seed files and completed 456 executions in 185 seconds without a panic, at 784 MiB RSS. As on the PR, initialization consumed the short-run time budget; no claim of extra mutation coverage is made. The weekly 15-minute run retains the longer exploration budget. No further merge-validation work remains for #708.

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

    adoptionpyropdf adopts a released engine changearea:enginecrates/pyropdf-engine: interpretation, rendering, writing, understandingengine:hayroEngine or dependency: hayro

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions