Skip to content

hayro-syntax: object numbers as u32 (I8) #18

Closed

Description

@yhay81

Context

DESIGN.md §6, I8. hayro-syntax reads object numbers as i32, so a reference above 2,147,483,647 cannot resolve. pylopdf renumbers objects above 8,388,607 at load (unallocated.rs), partly for this reason. pyropdf keeps object numbers as the file has them, so hayro must accept the whole u32 range.

Goal

An upstream pull request that reads object numbers as u32, and its adoption.

Acceptance

  • A draft patch with tests of numbers at and above 2^31
  • With the owner's go-ahead: the upstream pull request, linked here
  • Adoption, with a test of a file whose objects use numbers above 2^31

Activity

  1. added this to the Phase 0: Foundation milestone on Oct 24, 2026
  2. added
    upstreamWork that lands in an engine or dependency repository
    area:storecrates/pyropdf-store: objects, index, versions, saving, security
    engine:hayroEngine or dependency: hayro
    on Oct 24, 2026
  3. added
    in-progressClaimed; the latest comment names the branch
    on Oct 26, 2026
  4. yhay81 commented on Oct 26, 2026

    @yhay81
    OwnerAuthor

    Claimed. Draft patch on hayro branch u32-object-numbers (local worktree off LaurenzV/hayro main a553afcb); not published upstream until the owner says so.

  5. yhay81 commented on Oct 26, 2026

    @yhay81
    OwnerAuthor

    Draft patch ready, not published.

    • Branch: u32-object-numbers in the local hayro checkout (worktree hayro-wt/u32-object-numbers), commit 76319bc3 on LaurenzV/hayro main a553afcb. Not pushed to the fork.
    • Change: ObjRef::obj_number and ObjectIdentifier::obj_number become u32 (public field and new signature: a breaking change for 0.9). Every cross-reference reader already produced u32, so the as i32 casts go. gen_number stays i32. xref_stream_subsection now uses start.checked_add(i)?; with u32 numbers reachable, a subsection that passes u32::MAX used to overflow.
    • Tests: references at 2^31 and u32::MAX parse; 4294967296 0 R and -1 0 R are refused; an object numbered 3,000,000,000 resolves through an xref table subsection and through repair (no xref); an overflowing xref stream subsection fails. cargo test -p hayro-syntax 216 passed; the hayro CI clippy and cargo fmt --check are clean; cargo test -p hayro-tests showed 0 failures at 1,750 of 1,770 when the output was cut off.

    Proposed upstream PR title: "hayro-syntax: Read object numbers as u32".

    Next action: with the owner's go-ahead, push the branch to yhay81/hayro and open the PR against LaurenzV/hayro. Then link it here; adoption waits for a release.

  6. yhay81 commented on Oct 26, 2026

    @yhay81
    OwnerAuthor

    Opened upstream with the owner's go-ahead: LaurenzV/hayro#1650 (branch yhay81/hayro:u32-object-numbers). hayro-tests in release: 1,770 passed, 73 ignored. Next: follow review on #1650; adoption waits for a hayro release that contains it.

  7. added
    upstream:mergedUpstream: merged upstream; waits on a release to adopt
    adoptionpyropdf adopts a released engine change
    in-progressClaimed; the latest comment names the branch
    and removed
    in-progressClaimed; the latest comment names the branch
    upstreamWork that lands in an engine or dependency repository
    upstream:mergedUpstream: merged upstream; waits on a release to adopt
    on Oct 26, 2026
  8. 19 remaining items

  9. yhay81 commented on Nov 3, 2026

    @yhay81
    OwnerAuthor

    Handoff. Branch claude/18-hayro-0.9-u32, PR #609 (closes #18 and #224), awaiting the owner's review and the choices below. Rebased onto v0.2.1. Not merged. Claim released: the open PR keeps this issue out of the queue.

    Changed

    • [patch.crates-io] in Cargo.toml and fuzz/Cargo.toml takes all ten hayro crates from yhay81/hayro 394e4a0d (decision above); the crates name each other by path, so one source keeps their types the store's. The sdist keeps the table and the git sources (maturin 1.15.0).
    • The failures of the last handoff, each settled: the 33-operand limit (tests use 34), /Contents items that are not streams are skipped as hayro 0.9 does, mode-7 text is an INVISIBLE run and is extracted, and in the store [/AB#FG] reads as written, -1 0 R is no reference, and nesting keeps the store's own bound of 64 (value::MAX_NESTING), since cloning, comparing, dropping, copying, and decrypting a value recurse into it (F6).
    • hayro-syntax: parse a stream whose dictionary names an external file #224: the store refuses to decode a stream whose /F names a file; the round-trip test writes /F on every stream type; the spec map's engine rule is gone.
    • The acceptance test of objects numbered 2^31 to u32::MAX − 1: they resolve, render, extract, take an edit, and save incrementally and fully. The writer's property tests now generate references to any u32 number.
    • Rendering is unhinted, as with hayro 0.8, pending choice 1. evidence/hinted-subsets measures why.

    Checks (Windows): fmt, clippy -D warnings, cargo test --workspace (engine 184, store 327, corpus 2), uv run pytest (6,544 passed, 689 skipped), ruff, ruff format, ty, and cargo check of fuzz/. CI on the PR's head b328302: green (Rust, Python 3.11, 3.15, 3.15t with poppler, Audit, Windows save, the six fuzz targets, docs).

    Owner's choices (details in the PR body):

    1. Hinting: subsets keep what hinting reads (the auto-hinter's 32 Latin and 150 CJK reference characters of a TrueType program; the hints of a compacted CFF program), tests compare unhinted renders (needs a hinting switch on page.pixmap), or pyropdf renders unhinted (the PR now).
    2. SoftMask.xref: hayro 0.9 gives no reference to a mask's group; only Debug output carries a hash of the mask dictionary. The PR keeps the field, deprecated, as 0, constructible without it. Keep that, add a DeprecationWarning, or restore the number through the Debug hash.
    3. Push the tag pyropdf/glyph-run-font-dict on 394e4a0d to yhay81/hayro, so that released sdists keep fetching it if the branch of hayro-interpret: Expose the font dictionary of a glyph run LaurenzV/hayro#1734 is force-pushed. Not pushed.

    Next action: the owner answers 1 to 3; then the PR follows the answers, CI green, and merge with the owner's go-ahead. After merge: remove #584 from the blocked-by links of #19, #365, and #437, which the patched commit already serves (#224 closes with the PR); keep #584 open until hayro releases #1734; #603 removes the patch then. Follow-up #608: TilingPattern.opacity, which hayro 0.9 leaves to devices. 0.2.2's changelog entries are in the PR body.

  10. removed
    in-progressClaimed; the latest comment names the branch
    on Nov 3, 2026
  11. yhay81 commented on Nov 3, 2026

    @yhay81
    OwnerAuthor

    Superseded by the owner's decisions in the next comment: reading SoftMask.xref now warns with DeprecationWarning, and the tag is to be pushed.

    Owner's decisions (2026-11-04, "Keep the defaults" on the choices of the handoff above):

    1. Hinting: pyropdf renders text unhinted, as with hayro 0.8, as Move to hayro 0.9 with LaurenzV/hayro#1734 patched in, and read streams that name a file #609 does. Subsets stay as they are, and the tests compare the renders as before.
    2. SoftMask.xref: kept, deprecated, and 0 in what Page.content returns, with a default so that SoftMask(subtype=...) works; no DeprecationWarning, and no recovery through hayro's Debug output. The next minor release removes it (follow-up issue to come).

    Still open: 3, pushing the tag pyropdf/glyph-run-font-dict on 394e4a0d to yhay81/hayro.

  12. yhay81 commented on Nov 3, 2026

    @yhay81
    OwnerAuthor

    Owner's decisions on #609's three choices (2026-11-04, 「推奨どおりで」, as recommended). This supersedes item 2 of the comment above, which recorded no warning.

    1. Hinting. pyropdf renders text unhinted in 0.2.2, as Move to hayro 0.9 with LaurenzV/hayro#1734 patched in, and read streams that name a file #609 does, so that a patch release changes no render. Keeping the auto-hinter's reference characters in subsets is rejected: it would add about 150 CJK glyphs to each subset. Follow-ups: Render text hinted on request: a hinting option of page.pixmap and Document.render #612, an opt-in hinting=True of page.pixmap and Document.render, which only adds to the API and can ship in a patch release; Keep a CFF face's hints when a save compacts it #611, compacted CFF programs that lose their hints, which matters to other viewers too.
    2. SoftMask.xref. Kept as 0, and reading it warns with DeprecationWarning; SoftMask(subtype=...) and SoftMask(xref=..., subtype=...) still build a mask, which stays a frozen dataclass, and nothing but reading xref warns (Move to hayro 0.9 with LaurenzV/hayro#1734 patched in, and read streams that name a file #609, d30ae65). Not restored through hayro's Debug hash. The next minor release removes it (Remove the deprecated SoftMask.xref in the next minor release #610); 0.2.2's changelog line in Move to hayro 0.9 with LaurenzV/hayro#1734 patched in, and read streams that name a file #609's body says so.
    3. The tag. pyropdf/glyph-run-font-dict on 394e4a0d37baf8af513f6ebda15f4bada85d6f23 goes to yhay81/hayro, so that released sdists keep fetching the commit. Not pushed yet.

    #609 waits for the owner's review; not merged.

  13. yhay81 commented on Nov 3, 2026

    @yhay81
    OwnerAuthor

    Tag pushed with the owner's go-ahead ("Push the tag", 2026-11-04): yhay81/hayro refs/tags/pyropdf/glyph-run-font-dict, a signed annotated tag (972ea320) on 394e4a0d37baf8af513f6ebda15f4bada85d6f23, so that sdists built with the patch keep fetching the commit if the branch of LaurenzV/hayro#1734 is force-pushed. Keep it while such an sdist is published; #603 says so.

  14. added a commit that references this issue on Nov 3, 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

    adoptionpyropdf adopts a released engine changearea:storecrates/pyropdf-store: objects, index, versions, saving, securityengine:hayroEngine or dependency: hayro

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions