Fix usize underflow panic with wide chars at --terminal-width 1 - #3875
Open
koopatroopa787 wants to merge 2 commits into
Open
Fix usize underflow panic with wide chars at --terminal-width 1#3875koopatroopa787 wants to merge 2 commits into
koopatroopa787 wants to merge 2 commits into
Conversation
`--line-range :-N` with N near usize::MAX aborted with "capacity overflow" in two ways: 1. `largest_offset_from_end() + 1` overflowed usize when N == usize::MAX. 2. `VecDeque::with_capacity(usize::MAX)` panicked with capacity overflow. Replace the plain `+ 1` with `saturating_add(1)` and the infallible `VecDeque::with_capacity` with `try_reserve`, so an impossibly large offset produces a clear error message instead of an abort. Fixes sharkdp#3845
When `--terminal-width 1` is combined with `--wrap character` and a line contains a character whose display width exceeds the terminal (e.g. a double-width CJK char or emoji), `cursor` overshoots `cursor_max`. Two sites then performed unchecked `cursor_max - cursor` subtraction: - `printer.rs:822`: initializing `max_width` for the next text chunk - `printer.rs:934`: filling the end-of-line background Both underflow to `~usize::MAX`, and `" ".repeat(~usize::MAX)` aborts with "capacity overflow". Fix: use `saturating_sub` at both sites so an overshot cursor simply yields a fill width of 0, matching the clamping already applied elsewhere in the same function. Fixes sharkdp#3844
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes bat panics at extremely small terminal widths by preventing usize underflow/overflow when calculating remaining line width for wrapping and background filling.
Changes:
- Use
saturating_subinInteractivePrinter::print_lineto avoid underflow when the cursor overshootscursor_max(e.g., wide chars at--terminal-width 1). - Harden
Controller::print_file_rangesbuffering by usingsaturating_add(1)andVecDeque::try_reserveto avoid capacity overflow aborts for huge--line-rangeoffsets and return a user-facing error instead.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/printer.rs | Prevents width calculations from underflowing when wrapping/background-fill occurs after cursor overshoot. |
| src/controller.rs | Avoids VecDeque capacity overflow aborts when buffering lines for extreme --line-range offsets. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+269
to
+271
| buffered_lines.try_reserve(buffer_size).map_err(|_| { | ||
| format!("--line-range offset from end ({offset}) is too large to buffer") | ||
| })?; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
bat --terminal-width 1 --wrap characteraborted withcapacity overflow(exit 101) when a line contained a double-width character (CJK/emoji) and a background was painted on that line:Root Cause
cursoraccumulates the display width of each chunk. With--terminal-width 1,cursor_max = 1. A double-width character (display width 2) makescursor = 2 > cursor_max = 1.Two sites then performed unchecked
cursor_max - cursorsubtraction:printer.rs:822:let mut max_width = cursor_max - cursor;— used to size the text chunk bufferprinter.rs:934:" ".repeat(cursor_max - cursor)— used to fill the end-of-line backgroundBoth wrap to
~usize::MAX, and" ".repeat(~usize::MAX)aborts withcapacity overflow.(This is the same defect class as the
--style=snipwidth-1 panic in #3803.)Fix
Use
saturating_subat both sites so an overshot cursor yields a fill/buffer width of 0, matching the guard already applied atprinter.rs:789.Fixes #3844