ratatui icon indicating copy to clipboard operation
ratatui copied to clipboard

insert_before causes panic if drawing to too much of the terminal

Open bobbobbio opened this issue 6 months ago • 1 comments

Description

If Terminal::insert_before is used with height h and the width of the terminal is w, then a panic can happen if w * h > u16::MAX.

To Reproduce

Here is some simple code to reproduce it.

use std::{
    io::{self, Stdout},
    time::Duration,
};

use anyhow::{Context, Result};
use ratatui::{
    prelude::{Widget as _, Constraint},
    backend::Backend as _,
    backend::CrosstermBackend,
    crossterm::{
        event::{self, Event, KeyCode},
        terminal::{disable_raw_mode, enable_raw_mode},
    },
    terminal::{Terminal},
    widgets::{Row, Table},
    TerminalOptions, Viewport,
};

fn main() -> Result<()> {
    let mut terminal = setup_terminal().context("setup failed")?;
    run(&mut terminal).context("app loop failed")?;
    restore_terminal(&mut terminal).context("restore terminal failed")?;
    Ok(())
}

fn setup_terminal() -> Result<Terminal<CrosstermBackend<Stdout>>> {
    let stdout = io::stdout();
    enable_raw_mode().context("failed to enable raw mode")?;
    let backend = CrosstermBackend::new(stdout);
    let height = backend.size().unwrap().height;
    Terminal::with_options(
        backend,
        TerminalOptions {
            viewport: Viewport::Inline(height / 4),
        },
    )
    .context("creating terminal failed")
}

fn restore_terminal(terminal: &mut Terminal<CrosstermBackend<Stdout>>) -> Result<()> {
    disable_raw_mode().context("failed to disable raw mode")?;
    terminal.show_cursor().context("unable to show cursor")
}

fn run(terminal: &mut Terminal<CrosstermBackend<Stdout>>) -> Result<()> {
    let required_height = (u16::MAX / terminal.backend().size().unwrap().width) + 1;
    loop {
        terminal.insert_before(required_height, |b| {
            Table::new(
                vec![Row::new(vec!["a", "b", "c"]); required_height as usize],
                [
                    Constraint::Length(1),
                    Constraint::Length(1),
                    Constraint::Fill(1),
                ],
            )
            .render(b.area, b)
        }).unwrap();
        if should_quit()? {
            break;
        }
    }
    Ok(())
}

fn should_quit() -> Result<bool> {
    if event::poll(Duration::from_millis(250)).context("event poll failed")? {
        if let Event::Key(key) = event::read().context("event read failed")? {
            return Ok(KeyCode::Char('q') == key.code);
        }
    }
    Ok(false)
}

When run on my machine, this code crashes like this (reformatted)

thread 'main' panicked at /home/remi/.cargo/registry/src/index.crates.io-6f17d22bba15001f/ratatui-0.27.0/src/buffer/buffer.rs:269:22:
index out of bounds: the len is 65535 but the index is 65535
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Expected behavior

It shouldn't be crashing, this is not that large of an area. You only need to request terminal areas around the size of 256x256 to trigger it.

If the user has a particularly wide terminal, you could end up trying to do insert before with not that many lines and it just crashes.

If not just made to work, I would instead expect this limitation documented in Terminal::insert_before, although as it stands since this relies on the width of the terminal it is pretty hard to make sure you aren't running afoul of this.

Environment

  • OS: LInux
  • Terminal Emulator: kitty
  • Font: not sure
  • Crate version: 0.27.0
  • Backend: crossterm

Additional context

I looked at the code and I think I identified where the bug happens, but then again maybe not. I don't understand how Buffer is working properly in other code paths. Here is what I discovered nonetheless.

in terminal/terminal.rs the code for insert_before

  430         // Draw contents into buffer
  431         let area = Rect {
  432             x: self.viewport_area.left(),
  433             y: 0,
  434             width: self.viewport_area.width,
  435             height,
  436         };
  437         let mut buffer = Buffer::empty(area);
  438         draw_fn(&mut buffer);

We create a Rect with the area we want to draw to. We then create a buffer for the given Rect.

in buffer/buffer.rs

   60     /// Returns a Buffer with all cells initialized with the attributes of the given Cell
   61     #[must_use]
   62     pub fn filled(area: Rect, cell: Cell) -> Self {
   63         let size = area.area() as usize;
   64         let content = vec![cell; size];
   65         Self { area, content }
   66     }

Here is the function that ultimately get called to create the Buffer. It gets the size by calling area on the Rect, but that function says it clamps the area to u16::MAX so the buffer has content field with not enough entries to satisfy the area field. This breaks an invariant mentioned in the Buffer code

in buffer/buffer.rs

   45 pub struct Buffer {
   46     /// The area represented by this buffer
   47     pub area: Rect,
   48     /// The content of the buffer. The length of this Vec should always be equal to area.width *
   49     /// area.height
   50     pub content: Vec<Cell>,
   51 }

The length of the content Vec is less than area.width * area.height.

This then causes the render of the Table in our example to crash because it tries to access something in this corrupted buffer.

bobbobbio avatar Aug 04 '24 23:08 bobbobbio