From 3c654e4e8b70fa36903cfc0f1a2842dffb12e057 Mon Sep 17 00:00:00 2001 From: sap1110 <238814652+sap1110@users.noreply.github.com> Date: Mon, 28 Sep 2026 11:36:10 +0530 Subject: [PATCH 1/4] diff: reject a --tabsize too large for GNU diff A huge --tabsize was accepted and then used to size the tab expansion buffer, so the first tab in the input made diff overflow or abort on allocation failure. Reject values above isize::MAX - 3 with "invalid tabsize" and exit status 2, as GNU diff does. --- src/params.rs | 49 +++++++++++++++++++++++++++++++------- tests/by-util/test_diff.rs | 13 ++++++++++ 2 files changed, 54 insertions(+), 8 deletions(-) diff --git a/src/params.rs b/src/params.rs index 9e3e5a35..76fc1c60 100644 --- a/src/params.rs +++ b/src/params.rs @@ -8,6 +8,9 @@ use crate::bre::Bre; use crate::color::ColorWhen; use crate::utils::fold_file_name_case; +/// Largest value accepted for `--tabsize`, matching GNU diff. +const MAX_TABSIZE: usize = isize::MAX as usize - 3; + #[derive(Clone, Copy, Debug, Default, Eq, PartialEq)] pub enum Format { #[default] @@ -281,14 +284,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 +1057,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/tests/by-util/test_diff.rs b/tests/by-util/test_diff.rs index c6d41404..5a020530 100644 --- a/tests/by-util/test_diff.rs +++ b/tests/by-util/test_diff.rs @@ -71,6 +71,19 @@ fn missing_newline() { .stderr_str_check(|s| s.starts_with("No newline at end of file")); } +#[rstest] +fn huge_tabsize(#[values(&[][..], &["-u"], &["-c"])] format: &[&str]) { + let (at, ts) = at_and_ts!(); + at.write("file1", "\tx\n"); + at.write("file2", "y\n"); + ts.ucmd() + .args(format) + .args(&["-t", "--tabsize=18446744073709551615", "file1", "file2"]) + .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")] From 683601f38673ead0e1924debd092826d0a7c270d Mon Sep 17 00:00:00 2001 From: sap1110 <238814652+sap1110@users.noreply.github.com> Date: Tue, 29 Sep 2026 19:05:59 +0530 Subject: [PATCH 2/4] fixup! diff: reject a --tabsize too large for GNU diff --- src/utils.rs | 88 +++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 67 insertions(+), 21 deletions(-) diff --git a/src/utils.rs b/src/utils.rs index 29ab93df..bcc800f1 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,47 @@ 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] ); } + + /// A writer that fails once it has been given `limit` bytes. + struct LimitedWriter { + written: Vec, + limit: usize, + } + + impl Write for LimitedWriter { + fn write(&mut self, buf: &[u8]) -> std::io::Result { + let n = buf.len().min(self.limit - self.written.len()); + if n == 0 { + return Err(std::io::ErrorKind::WriteZero.into()); + } + self.written.extend_from_slice(&buf[..n]); + Ok(n) + } + + fn flush(&mut self) -> std::io::Result<()> { + Ok(()) + } + } + + #[test] + fn huge_tabsize() { + // The spaces are streamed to the writer, so a huge tab size + // neither overflows nor needs a huge buffer up front. + for tabsize in [usize::MAX / 2, usize::MAX] { + let mut output = LimitedWriter { + written: Vec::new(), + limit: 1000, + }; + assert!(do_expand_tabs(&mut output, b"a\tb\tc", tabsize).is_err()); + let mut expected = b"a".to_vec(); + expected.resize(1000, b' '); + assert_eq!(output.written, expected); + } + } } mod quoting { From bbce9112888ead9e46c3efed195aac3b342b3576 Mon Sep 17 00:00:00 2001 From: sap1110 <238814652+sap1110@users.noreply.github.com> Date: Tue, 29 Sep 2026 19:05:59 +0530 Subject: [PATCH 3/4] fixup! diff: reject a --tabsize too large for GNU diff --- tests/by-util/test_diff.rs | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/tests/by-util/test_diff.rs b/tests/by-util/test_diff.rs index 5a020530..3e426ec7 100644 --- a/tests/by-util/test_diff.rs +++ b/tests/by-util/test_diff.rs @@ -71,14 +71,10 @@ fn missing_newline() { .stderr_str_check(|s| s.starts_with("No newline at end of file")); } -#[rstest] -fn huge_tabsize(#[values(&[][..], &["-u"], &["-c"])] format: &[&str]) { - let (at, ts) = at_and_ts!(); - at.write("file1", "\tx\n"); - at.write("file2", "y\n"); - ts.ucmd() - .args(format) - .args(&["-t", "--tabsize=18446744073709551615", "file1", "file2"]) +#[test] +fn huge_tabsize() { + new_ucmd!() + .args(&["--tabsize=18446744073709551615", "foo", "bar"]) .fails_with_code(2) .no_stdout() .stderr_contains("invalid tabsize «18446744073709551615»"); From fbb6fcffb89fa7212c06ad40234e2844d9efcf6d Mon Sep 17 00:00:00 2001 From: sap1110 <238814652+sap1110@users.noreply.github.com> Date: Fri, 30 Oct 2026 13:45:20 +0530 Subject: [PATCH 4/4] fixup! diff: reject a --tabsize too large for GNU diff --- src/params.rs | 7 +++++-- src/side_diff.rs | 8 ++++++++ src/utils.rs | 41 ++++++++--------------------------------- 3 files changed, 21 insertions(+), 35 deletions(-) diff --git a/src/params.rs b/src/params.rs index 76fc1c60..c9c3d758 100644 --- a/src/params.rs +++ b/src/params.rs @@ -8,8 +8,11 @@ use crate::bre::Bre; use crate::color::ColorWhen; use crate::utils::fold_file_name_case; -/// Largest value accepted for `--tabsize`, matching GNU diff. -const MAX_TABSIZE: usize = isize::MAX as usize - 3; +/// 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 { diff --git a/src/side_diff.rs b/src/side_diff.rs index 1128e2e7..faebfd19 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 bcc800f1..4097477f 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -440,41 +440,16 @@ mod tests { ); } - /// A writer that fails once it has been given `limit` bytes. - struct LimitedWriter { - written: Vec, - limit: usize, - } - - impl Write for LimitedWriter { - fn write(&mut self, buf: &[u8]) -> std::io::Result { - let n = buf.len().min(self.limit - self.written.len()); - if n == 0 { - return Err(std::io::ErrorKind::WriteZero.into()); - } - self.written.extend_from_slice(&buf[..n]); - Ok(n) - } - - fn flush(&mut self) -> std::io::Result<()> { - Ok(()) - } - } - #[test] fn huge_tabsize() { - // The spaces are streamed to the writer, so a huge tab size - // neither overflows nor needs a huge buffer up front. - for tabsize in [usize::MAX / 2, usize::MAX] { - let mut output = LimitedWriter { - written: Vec::new(), - limit: 1000, - }; - assert!(do_expand_tabs(&mut output, b"a\tb\tc", tabsize).is_err()); - let mut expected = b"a".to_vec(); - expected.resize(1000, b' '); - assert_eq!(output.written, expected); - } + // 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[..]); } }