diff --git a/src/params.rs b/src/params.rs index 9e3e5a3..c9c3d75 100644 --- a/src/params.rs +++ b/src/params.rs @@ -8,6 +8,12 @@ use crate::bre::Bre; use crate::color::ColorWhen; use crate::utils::fold_file_name_case; +/// Largest value accepted for `--tabsize`. +/// Side-by-side mode computes `tabsize + GUTTER_WIDTH_MIN` (3) as an +/// `isize` in `side_diff::Config::new`, so anything larger would overflow. +/// GNU diff accepts the same maximum. +pub(crate) const MAX_TABSIZE: usize = isize::MAX as usize - 3; + #[derive(Clone, Copy, Debug, Default, Eq, PartialEq)] pub enum Format { #[default] @@ -281,14 +287,8 @@ pub fn parse_params>( .unwrap() .as_str(); params.tabsize = match tabsize_str.parse::() { - Ok(num) => { - if num == 0 { - return Err("invalid tabsize «0»".to_string()); - } - - num - } - Err(_) => return Err(format!("invalid tabsize «{tabsize_str}»")), + Ok(num) if num != 0 && num <= MAX_TABSIZE => num, + _ => return Err(format!("invalid tabsize «{tabsize_str}»")), }; continue; @@ -1060,6 +1060,42 @@ mod tests { .peekable() ) .is_err()); + for too_large in [(MAX_TABSIZE + 1).to_string(), usize::MAX.to_string()] { + assert_eq!( + Err(format!("invalid tabsize «{too_large}»")), + parse_params( + [ + os("diff"), + os(&format!("--tabsize={too_large}")), + os("foo"), + os("bar") + ] + .iter() + .cloned() + .peekable() + ) + ); + } + assert_eq!( + Ok(Params { + executable: os("diff"), + from: os("foo"), + to: os("bar"), + tabsize: MAX_TABSIZE, + ..Default::default() + }), + parse_params( + [ + os("diff"), + os(&format!("--tabsize={MAX_TABSIZE}")), + os("foo"), + os("bar") + ] + .iter() + .cloned() + .peekable() + ) + ); } #[test] fn width() { diff --git a/src/side_diff.rs b/src/side_diff.rs index 1128e2e..faebfd1 100644 --- a/src/side_diff.rs +++ b/src/side_diff.rs @@ -1274,6 +1274,14 @@ mod tests { assert_eq!(config.separator_pos, 3); } + #[test] + fn test_max_tab_size() { + // `tab_size + GUTTER_WIDTH_MIN` must not overflow `isize`. + let config = create_config(130, crate::params::MAX_TABSIZE, false); + assert_eq!(config.sdiff_half_width, 0); + assert_eq!(config.sdiff_column_two_offset, 130); + } + #[test] fn test_tab_size_larger_than_width() { let config = create_config(5, 10, false); diff --git a/src/utils.rs b/src/utils.rs index 29ab93d..4097477 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -12,46 +12,52 @@ use std::{ }; use unicode_width::UnicodeWidthStr; -/// Replace tabs by spaces in the input line. +/// Write the input line to an output stream, replacing tabs by spaces. /// Correctly handle multi-bytes characters. +/// The spaces are written in small chunks, so a huge tab size does not +/// need a huge buffer. /// This assumes that line does not contain any line breaks (if it does, the result is undefined). -#[must_use] -pub fn do_expand_tabs(line: &[u8], tabsize: usize) -> Vec { +pub fn do_expand_tabs( + output: &mut W, + line: &[u8], + tabsize: usize, +) -> std::io::Result<()> { + const SPACES: [u8; 64] = [b' '; 64]; let tab = b'\t'; - let ntabs = line.iter().filter(|c| **c == tab).count(); - if ntabs == 0 { - return line.to_vec(); - } - let mut result = Vec::with_capacity(line.len() + ntabs * (tabsize - 1)); let mut offset = 0; let mut iter = line.split(|c| *c == tab).peekable(); while let Some(chunk) = iter.next() { - match String::from_utf8(chunk.to_vec()) { - Ok(s) => offset += UnicodeWidthStr::width(s.as_str()), + match std::str::from_utf8(chunk) { + Ok(s) => offset += UnicodeWidthStr::width(s), Err(_) => offset += chunk.len(), } - result.extend_from_slice(chunk); + output.write_all(chunk)?; if iter.peek().is_some() { - result.resize(result.len() + tabsize - offset % tabsize, b' '); + let mut spaces = tabsize - offset % tabsize; + while spaces > 0 { + let n = spaces.min(SPACES.len()); + output.write_all(&SPACES[..n])?; + spaces -= n; + } offset = 0; } } - result + Ok(()) } /// Write a single line to an output stream, expanding tabs to space if necessary. /// This assumes that line does not contain any line breaks /// (if it does and tabs are to be expanded to spaces, the result is undefined). -pub fn do_write_line( - output: &mut Vec, +pub fn do_write_line( + output: &mut W, line: &[u8], expand_tabs: bool, tabsize: usize, ) -> std::io::Result<()> { if expand_tabs { - output.write_all(do_expand_tabs(line, tabsize).as_slice()) + do_expand_tabs(output, line, tabsize) } else { output.write_all(line) } @@ -393,11 +399,14 @@ mod tests { use super::*; use pretty_assertions::assert_eq; + fn expand(line: &[u8], tabsize: usize) -> Vec { + let mut output = Vec::new(); + assert!(do_expand_tabs(&mut output, line, tabsize).is_ok()); + output + } + fn assert_tab_expansion(line: &str, tabsize: usize, expected: &str) { - assert_eq!( - do_expand_tabs(line.as_bytes(), tabsize), - expected.as_bytes() - ); + assert_eq!(expand(line.as_bytes(), tabsize), expected.as_bytes()); } #[test] @@ -426,10 +435,22 @@ mod tests { fn invalid_utf8() { // [240, 240, 152, 137] is an invalid UTF-8 sequence, so it is handled as 4 bytes assert_eq!( - do_expand_tabs(&[240, 240, 152, 137, 9, 102, 111, 111], 8), + expand(&[240, 240, 152, 137, 9, 102, 111, 111], 8), &[240, 240, 152, 137, 32, 32, 32, 32, 102, 111, 111] ); } + + #[test] + fn huge_tabsize() { + // The spaces are streamed to the writer, so the largest accepted + // tab size neither overflows nor needs a huge buffer up front. + let mut buf = [0u8; 1000]; + let mut output = &mut buf[..]; + assert!(do_expand_tabs(&mut output, b"a\tb\tc", crate::params::MAX_TABSIZE).is_err()); + let mut expected = b"a".to_vec(); + expected.resize(1000, b' '); + assert_eq!(buf[..], expected[..]); + } } mod quoting { diff --git a/tests/by-util/test_diff.rs b/tests/by-util/test_diff.rs index c6d4140..3e426ec 100644 --- a/tests/by-util/test_diff.rs +++ b/tests/by-util/test_diff.rs @@ -71,6 +71,15 @@ fn missing_newline() { .stderr_str_check(|s| s.starts_with("No newline at end of file")); } +#[test] +fn huge_tabsize() { + new_ucmd!() + .args(&["--tabsize=18446744073709551615", "foo", "bar"]) + .fails_with_code(2) + .no_stdout() + .stderr_contains("invalid tabsize «18446744073709551615»"); +} + #[rstest] #[case::second(&["file1", "-"], "bar\n", "--- file1\tTIMESTAMP\n+++ -\tTIMESTAMP\n")] #[case::first(&["-", "file2"], "foo\n", "--- -\tTIMESTAMP\n+++ file2\tTIMESTAMP\n")]