Don't panic in RowIterator when stored grapheme runs overflow the row by anshul-garg27 · Pull Request #9995 · warpdotdev/warp

anshul-garg27 · 2026-05-03T21:22:19Z

Closes #9161.

Description

RowIterator::next aborts the process via panic! when a stored row's grapheme runs imply more cells than the row actually holds. The reporter's panic log captures the canonical case:

idx: 142
len: 142
grapheme runs: [
  GraphemeRun { count: 6, info: GraphemeInfo { cell_width: 1, utf8_bytes: 1 } },
  GraphemeRun { count: 1, info: GraphemeInfo { cell_width: 2, utf8_bytes: 6 } },
  GraphemeRun { count: 135, info: GraphemeInfo { cell_width: 1, utf8_bytes: 1 } },
]

The grapheme runs claim 6 * 1 + 1 * 2 + 135 * 1 = 143 cell positions in a row whose length is 142, so once the iterator advances row.occ past the wide character's spacer the next row.get_mut(row.occ) returns None and we unconditionally panic! (row_iterator.rs:100). This is the same panic site reported in the auto-closed #8515 (same file, same line) and is reproducible whenever any CJK / emoji grapheme sits at a row boundary on a row whose grapheme bookkeeping has gone out of sync with its allocated length (which the issue reporter hits daily on Windows).

Fix

Two changes in crates/warp_terminal/src/model/grid/flat_storage/row_iterator.rs:

Replace the panic! after the diagnostic log::warn! with a break. The partially-filled row is still returned, so the terminal keeps running. The pre-existing warning preserves the diagnostic context for tracking the underlying writer-side inconsistency separately — without aborting the user's session.
Bounds-check the WIDE_CHAR_SPACER write at row[idx + 1]. When a wide grapheme is the last cell of a row, idx + 1 == row.len() and the existing row[idx + 1].flags.insert(...) would panic via the Row::index_mut impl regardless of the change above. The WIDE_CHAR flag is still set on the wide cell itself; we just skip the unreachable spacer cell.

The reporter's specific row layout (6 ASCII + 1 wide + 135 ASCII overflowing a 142-cell row) is hit by both code paths. The combined fix means neither one aborts.

Testing

Added test_row_iterator_handles_wide_char_at_row_boundary_without_panicking (mod_tests.rs). It constructs a 7-cell row whose final cell holds the wide CJK character 日, pushes it through FlatStorage, and asserts that RowIterator::next returns the row instead of panicking. The test fails on the pre-fix code via the IndexMut panic at the row[idx + 1] spacer write.
Confirmed the existing wide-char tests still pass (test_row_with_double_width_char, test_row_iteration):

Starting 3 tests across 1 binary (83 tests skipped)
    PASS [0.020s] test_row_iteration
    PASS [0.021s] test_row_with_double_width_char
    PASS [0.020s] test_row_iterator_handles_wide_char_at_row_boundary_without_panicking
────────────
     Summary [0.021s] 3 tests run: 3 passed, 83 skipped

cargo fmt -p warp_terminal -- --check passes.
cargo check --tests -p warp_terminal passes.

Server API

Yes
No

Agent Mode

Yes
No

Changelog Entries

CHANGELOG-BUG-FIX: Fixed a crash on Windows when terminal output containing CJK characters or emoji landed at a row boundary (regression of #8515).

oz-for-oss · 2026-05-03T21:22:30Z

@anshul-garg27

I'm starting a first review of this pull request.

You can view the conversation on Warp.

I completed the review and no human review was requested for this pull request.

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

oz-for-oss

Overview

This PR replaces a RowIterator panic with graceful degradation and guards wide-character spacer marking at the row boundary.

Concerns

The added regression test does not currently create a stored wide-character run, so it would not fail on the pre-fix panic path.

Verdict

Found: 0 critical, 1 important, 0 suggestions

Request changes

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

oz-for-oss · 2026-05-03T21:27:51Z

+    let mut storage = FlatStorage::new(7, None, Some(2));
+
+    let mut wide_cell = Cell::default();
+    wide_cell.c = '日';


⚠️ [IMPORTANT] This cell is recorded as width 1 unless Flags::WIDE_CHAR is set, so the new test doesn't exercise the overflow/spacer panic path and would pass before the fix.

Suggested change

wide_cell.c = '日';

wide_cell.c = '日';

wide_cell.flags.insert(Flags::WIDE_CHAR);

…he row Closes warpdotdev#9161. `Workspace::RowIterator::next` could abort the process via `panic!` when a stored row's grapheme runs implied more cells than the row actually held. The reporter's logs show the canonical case: 6 ASCII + 1 wide CJK + 135 ASCII grapheme runs claim 143 cell positions in a row whose length is 142, so once the iterator advances `row.occ` past the wide char's spacer the next `row.get_mut(row.occ)` returns `None` and we unconditionally panic. This regressed the auto-closed warpdotdev#8515 (same panic, same file, same line) and is reproducible whenever any CJK or emoji character sits at a row boundary. Two fixes in `crates/warp_terminal/src/model/grid/flat_storage/row_iterator.rs`: 1. Replace the `panic!` after the diagnostic `log::warn!` with a `break` so the partially-filled row is still returned. The terminal keeps running and the warning preserves the diagnostic context for tracking the underlying writer-side inconsistency separately. 2. Bounds-check the `WIDE_CHAR_SPACER` write at `row[idx + 1]`. When a wide grapheme is the last cell of a row, `idx + 1 == row.len()` and the existing `IndexMut` would panic via the `Row::index_mut` impl. The `WIDE_CHAR` flag is still set on the wide cell itself; we just skip the unreachable spacer cell. Adds `test_row_iterator_handles_wide_char_at_row_boundary_without_panicking` to `mod_tests.rs`, which constructs a 7-cell row whose final cell holds a wide CJK character, pushes it through `FlatStorage`, and asserts that `RowIterator::next` returns the row instead of panicking. The test is written to fail on the pre-fix code (the panic site fires inside `row[idx + 1].flags.insert`).

anshul-garg27 · 2026-05-03T22:08:33Z

Pushed 4f369dd — applied your suggested fix (wide_cell.flags.insert(Flags::WIDE_CHAR)).

The test now exercises the spacer-overflow panic path correctly. Verified by reverting the production fix in row_iterator.rs to upstream/master and re-running the test — it panics:

thread '...test_row_iterator_handles_wide_char_at_row_boundary_without_panicking' panicked at row_iterator.rs:133:20:
index out of bounds: the len is 7 but the index is 7

That panic comes from the row[idx + 1] IndexMut path you flagged — the original Cell::default() cell would have had cell_width=1 and never reached that spacer write, so the test would have been a no-op pre-fix. With the production fix re-applied, all 3 tests pass:

PASS [0.014s] test_row_iteration
PASS [0.014s] test_row_iterator_handles_wide_char_at_row_boundary_without_panicking
PASS [0.014s] test_row_with_double_width_char
Summary [0.016s] 3 tests run: 3 passed, 83 skipped

/oz-review

oz-for-oss · 2026-05-03T22:08:44Z

@anshul-garg27

I'm re-reviewing this pull request in response to a review request.

You can view the conversation on Warp.

I reviewed this pull request and requested human review from: @warpdotdev/oss-maintainers.

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

Oz no longer requests changes for this pull request after the latest automated review.

oz-for-oss

Overview

This PR changes RowIterator to degrade gracefully when corrupted grapheme runs overrun a row and guards the wide-character spacer write at row boundaries. The added regression test covers a CJK wide character in the last cell and exercises the prior panic path.

Concerns

None identified in the changed lines.

Verdict

Found: 0 critical, 0 important, 0 suggestions

Approve

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

cla-bot Bot added the cla-signed label May 3, 2026

github-actions Bot added the external-contributor Indicates that a PR has been opened by someone outside the Warp team. label May 3, 2026

oz-for-oss Bot previously requested changes May 3, 2026

View reviewed changes

anshul-garg27 force-pushed the fix/9161-rowiter-cjk-crash branch from 3789c06 to 4f369dd Compare May 3, 2026 22:08

oz-for-oss Bot reviewed May 3, 2026

View reviewed changes

oz-for-oss Bot requested review from a team and vkodithala and removed request for a team May 3, 2026 22:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Don't panic in RowIterator when stored grapheme runs overflow the row#9995

Don't panic in RowIterator when stored grapheme runs overflow the row#9995
anshul-garg27 wants to merge 1 commit intowarpdotdev:masterfrom
anshul-garg27:fix/9161-rowiter-cjk-crash

anshul-garg27 commented May 3, 2026

Uh oh!

oz-for-oss Bot commented May 3, 2026 •

edited

Loading

Uh oh!

oz-for-oss Bot left a comment

Uh oh!

oz-for-oss Bot May 3, 2026

Uh oh!

anshul-garg27 commented May 3, 2026

Uh oh!

oz-for-oss Bot commented May 3, 2026 •

edited

Loading

Uh oh!

oz-for-oss Bot left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

	wide_cell.c = '日';
	wide_cell.c = '日';
	wide_cell.flags.insert(Flags::WIDE_CHAR);

Conversation

anshul-garg27 commented May 3, 2026

Description

Fix

Testing

Server API

Agent Mode

Changelog Entries

Uh oh!

oz-for-oss Bot commented May 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

oz-for-oss Bot left a comment

Choose a reason for hiding this comment

Overview

Concerns

Verdict

Uh oh!

oz-for-oss Bot May 3, 2026

Choose a reason for hiding this comment

Uh oh!

anshul-garg27 commented May 3, 2026

Uh oh!

oz-for-oss Bot commented May 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

oz-for-oss Bot left a comment

Choose a reason for hiding this comment

Overview

Concerns

Verdict

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

oz-for-oss Bot commented May 3, 2026 •

edited

Loading

oz-for-oss Bot commented May 3, 2026 •

edited

Loading