Skip to content

fix: refuse a saved screen whose declared size is past the terminal limit - #602

Merged
vyncint merged 1 commit into
vyncint:mainfrom
bercedev:fix/parse-refuses-oversized-screens
Oct 22, 2026
Merged

vyncint merged 1 commit into
vyncint:mainfrom
bercedev:fix/parse-refuses-oversized-screens

Conversation

@bercedev

Copy link
Copy Markdown
Contributor

What & why

Closes #583

Screen::parse allocated cols × rows cells from the header before reading any grid line, and the header was only checked for zero. A 31-byte file declaring 65535x65535 aborted on a 206 GB allocation.

  • parse_header (screen/parse.rs) now refuses a dimension above MAX_DIMENSION with Error::Parse. For example: line 1: 65535x65535 is past the 1000-per-axis limit of a terminal, so no termlens wrote this screen.
  • It reuses the terminal's own constant, now pub(crate) in terminal.rs with its value and doc comment unchanged, so the two limits can't drift apart.
  • The JSON reader (screen/serde_impl.rs) applies the same limit, before the row and cell counts are compared.
  • A 1000x1000 screen still parses.

I left the optional repeated-span-lines part of the issue out to keep this to one change.

Tests (tests/compat.rs):

  • a_declared_size_past_the_terminal_limit_is_refused_before_allocating: 65535x65535, 1001x1 and 1x1001 each give an Error::Parse naming the size and the limit, and 1000x1000 parses.
  • json::a_json_size_past_the_terminal_limit_is_refused: a corpus plain.json with cols and rows set to 65535 is refused with the same message.

With the old parse.rs and serde_impl.rs, the compat test binary aborts with memory allocation of 206152138800 bytes failed (SIGABRT).

Checklist

  • Linked an issue (or explained above why none exists)
  • Tests added/updated for the change
  • cargo fmt --all and cargo clippy --workspace --all-targets --all-features are clean
  • All commits are signed off (git commit -s) — see CONTRIBUTING.md §5
  • No AI attribution trailers (no AI co-authors, "Generated with" footers, or bot identities) — see CONTRIBUTING.md §6
  • CHANGELOG.md updated under [Unreleased] (user-facing changes only)
  • Snapshot changes (if any) were reviewed with cargo insta review, not blind-accepted (no snapshot changes)

Checked with Rust 1.99.1, offline:

  • cargo test --workspace --all-features: 627 passed, 0 failed.
  • cargo clippy -D warnings for the workspace with all features, and for -p termlens with default features, no default features, and decode only.
  • cargo fmt --all --check.
  • RUSTDOCFLAGS='-D warnings' cargo doc --no-deps --all-features.
  • check-cli-contract.sh: PASS.
  • check-dco.sh and check-no-ai-attribution.sh over main..HEAD.

This and #601 both add a ### Fixed section under [Unreleased]. Whichever lands second needs a one-line merge in CHANGELOG.md, and I'll rebase this one if it's second.

…imit

Screen::parse allocated cols x rows cells from the header before reading any line, so a 31-byte file declaring 65535x65535 aborted on a 206 GB allocation (vyncint#583). parse_header and the JSON reader now refuse a dimension above MAX_DIMENSION (1000) with an error that names it.
Signed-off-by: berce <75228263+bercedev@users.noreply.github.com>

@vyncint vyncint left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks — clean fix. Verified: the cap has held on the builder and on resize since 0.5.0, so no release STABILITY covers ever wrote a larger screen and the compat promise holds; compat tests pass locally, and the old code aborts on the new test as you said.

@vyncint
vyncint merged commit 364e12c into vyncint:main Oct 22, 2026
30 of 32 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A 31-byte saved screen makes Screen::parse allocate 206 GB and abort

2 participants