Skip to content

Check that pyropdf-python grows input-derived collections through the budget #677

Closed

Description

@yhay81

Context

DESIGN.md F6 and section 9. #675 shares the store's configuration as crates/clippy.toml, which binds every crate under crates/ that has no clippy.toml of its own; crates/pyropdf-python/clippy.toml, which sets nothing, keeps it from this crate until this issue deletes that file. The shared configuration refuses Dict::insert, Array::push, and Array::insert outside the store too, and its comment says why. With it, pyropdf-python has 548 uses in 25 files at 0.2.2 (11bbbba):

  • by file, most in forms.rs (120), annotations.rs (99), pixel_ops.rs (44), unrotate.rs (35), buffers.rs (34), and pages.rs (33);
  • by method, Dict::insert 230, Vec::push 90, Iterator::collect 68, Vec::with_capacity 45, slice::to_vec 24, Vec::extend_from_slice 19, Array::push 15, and vec![value; n] 13.

The binding copies Python buffers and strings and builds the objects that forms, annotations, and page operations write, so much of its growth follows what the caller passes. The notes give the command that reproduces the count.

Goal

pyropdf-python is checked by crates/clippy.toml, and every remaining use is budgeted or expects the lint with what bounds it.

Scope

In:

  • adopting crates/clippy.toml for the binding: deleting crates/pyropdf-python/clippy.toml, removing the crate from UNBOUND in tests/test_clippy_config.py, and allowing the lint under cfg(test) in lib.rs, as the store and the schema do;
  • each use: through BudgetVec (or the budget's other types) where it grows from input or from the caller's arguments, or #[expect(clippy::disallowed_methods, reason = "...")] that names its bound;
  • a fix, with a test, for each use that turns out to grow from input outside the budget.

Out: the engine (its own issue); conversions that PyO3 performs before the binding's code runs.

Paths

crates/pyropdf-python/ (src/, and its clippy.toml, which this issue deletes), and tests/ for the fixes and tests/test_clippy_config.py.

Acceptance

  • CI's cargo clippy --workspace --all-targets --all-features -- -D warnings passes with the configuration binding pyropdf-python.
  • Each expect names the bound that makes its growth safe; a size that section 9's single check of Python buffers bounds says so.
  • Each use found to grow from input outside the budget is fixed, with a Python test that the call raises LimitError with its code.
  • Where a hot path changes (pixel operations, buffer conversion), the pull request commits a before-and-after measurement under evidence/.

Notes

  • forms.rs and annotations.rs hold most of the Dict::insert calls. Check collection growth outside the store, starting with pyropdf-schema #675 keeps them checked outside the store; crates/clippy.toml says why.
  • To count the uses, delete crates/pyropdf-python/clippy.toml and run cargo clippy -p pyropdf-python --lib --all-features --message-format=short.
  • A Dict::insert with a constant key adds one entry at most, whatever the input; it expects the lint with that bound, as the store's calls in crates/pyropdf-store/src/refs/labels.rs do.
  • One module per commit keeps the pull request reviewable; it still squashes to one commit.

Activity

  1. added this to the Phase 0: Foundation milestone on Nov 5, 2026
  2. added
    area:pythoncrates/pyropdf-python and python/pyropdf: the binding and the Python API
    on Nov 5, 2026
  3. added
    in-progressClaimed; the latest comment names the branch
    on Nov 5, 2026
  4. yhay81 commented on Nov 5, 2026

    @yhay81
    OwnerAuthor

    Claiming on codex/677-binding-collection-budgets, based on main 2fe716e. PR #691 has landed, so the binding-wide audit can proceed without overlapping its pending changes. I will inventory every shared-Clippy diagnostic, document existing bounds precisely, fix unbudgeted input-derived growth with public regression tests, and commit modules separately for review. Hot-path implementation changes, if needed, will include before/after evidence.

  5. yhay81 commented on Nov 5, 2026

    @yhay81
    OwnerAuthor

    Completed on codex/677-binding-collection-budgets in PR #698, squash-merged as 2c5ae21c06a3e06fc8e9f5eaab842c088bcd579e under the owner's standing authorization.

    The binding now uses the shared collection lint. Fixed bounds and existing charged reservations are documented, while character columns, Type 3 lookup results, nested savepoints, tagged object conversions, and signature-field worklists charge the operation before growth. The TIFF palette copy is fallible; the separate encoded-image input API is tracked in #697 under phase tracker #4. Audit notes and before/after timings are in evidence/binding-collection-budgets/.

    All local check commands passed: full Python suite 7,243 passed / 692 skipped; targeted regression run 131 passed / 1 skipped; Ruff, formatting, ty, workspace Clippy with warnings denied, and workspace Rust tests passed. All six checks on the merged PR head passed, including Windows save, and there were no unresolved reviews. Issue closure is verified and the in-progress label is removed.

    Post-merge main CI also passed: https://github.com/yhay81/pyropdf/actions/runs/42067252711. This issue is complete. Work continues on #697, which needs no owner permission or release decision.

  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:pythoncrates/pyropdf-python and python/pyropdf: the binding and the Python API

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions