od, nl: match GNU on -w suffixes and huge -l values

src/uu/nl/src/nl.rs

@@ -7,6 +7,7 @@ use clap::{Arg, ArgAction, Command};
 use std::ffi::{OsStr, OsString};
 use std::fs::File;
 use std::io::{self, BufRead, BufReader, BufWriter, Read, Write, stdin, stdout};
+use std::num::{IntErrorKind, ParseIntError};
 use std::path::Path;
 use uucore::display::Quotable;
 use uucore::error::{FromIo, UResult, USimpleError, set_exit_code};
@@ -213,6 +214,25 @@ pub mod options {
     pub const NUMBER_WIDTH: &str = "number-width";
 }
 
+/// Parses the value of `-l`. A number too large for `u64` saturates instead of
+/// being rejected: it only sets how many blank lines make up one numbered line.
+fn parse_join_blank_lines(s: &str) -> Result<u64, ParseIntError> {
+    s.parse::<u64>().or_else(|e| {
+        // `parse` reports an overflow without looking at the rest of the
+        // value, so only saturate when all of it is digits.
+        let all_digits = s
+            .strip_prefix('+')
+            .unwrap_or(s)
+            .bytes()
+            .all(|b| b.is_ascii_digit());
+        if *e.kind() == IntErrorKind::PosOverflow && all_digits {
+            Ok(u64::MAX)
+        } else {
+            Err(e)
+        }
+    })
+}
+
 #[uucore::main]
 pub fn uumain(args: impl uucore::Args) -> UResult<()> {
     let matches = uucore::clap_localization::handle_clap_result(uu_app(), args)?;
@@ -337,7 +357,7 @@ pub fn uu_app() -> Command {
                 .long(options::JOIN_BLANK_LINES)
                 .help(translate!("nl-help-join-blank-lines"))
                 .value_name("NUMBER")
-                .value_parser(clap::value_parser!(u64)),
+                .value_parser(parse_join_blank_lines),
         )
         .arg(
             Arg::new(options::NUMBER_FORMAT)

src/uu/od/src/od.rs

@@ -33,7 +33,7 @@ use crate::multifile_reader::{HasError, InputSource, MultifileReader};
 use crate::output_info::OutputInfo;
 use crate::parse_formats::{ParsedFormatterItemInfo, parse_format_flags};
 use crate::parse_inputs::{CommandLineInputs, parse_inputs};
-use crate::parse_nrofbytes::parse_number_of_bytes;
+use crate::parse_nrofbytes::{parse_number_of_bytes, parse_width};
 use crate::partial_reader::PartialReader;
 use crate::peek_reader::{PeekRead, PeekReader};
 use crate::prn_char::format_ascii_dump;
@@ -145,7 +145,7 @@ impl OdOptions {
             matches.value_source(options::WIDTH),
         ) {
             let width_display = option_display_name(args, options::WIDTH, 'w');
-            let parsed = parse_number_of_bytes(s).map_err(|e| {
+            let parsed = parse_width(s).map_err(|e| {
                 let message = format_error_message(&e, s, &width_display);
                 e.size_value_error(
                     diag_args,

src/uu/od/src/parse_nrofbytes.rs

@@ -77,6 +77,20 @@ pub fn parse_number_of_bytes(s: &str) -> Result<u64, ParseSizeError> {
         .ok_or_else(|| ParseSizeError::SizeTooBig(s.to_string()))
 }
 
+/// Parses a plain decimal number, as `-w` takes: no radix prefix (a leading
+/// zero is not octal) and no multiplier suffix.
+pub fn parse_width(s: &str) -> Result<u64, ParseSizeError> {
+    let digits = s.bytes().take_while(u8::is_ascii_digit).count();
+    if digits == 0 {
+        return Err(ParseSizeError::ParseFailure(s.to_string()));
+    }
+    if digits < s.len() {
+        return Err(ParseSizeError::InvalidSuffix(s.to_string()));
+    }
+    s.parse()
+        .map_err(|_| ParseSizeError::SizeTooBig(s.to_string()))
+}
+
 #[test]
 #[allow(clippy::cognitive_complexity)]
 fn test_parse_number_of_bytes() {
@@ -94,3 +108,38 @@ fn test_parse_number_of_bytes() {
     assert_eq!(16 * 1024, parse_number_of_bytes("0x10k").unwrap());
     assert_eq!(16 * 1_048_576, parse_number_of_bytes("0x10m").unwrap());
 }
+
+#[test]
+fn test_parse_width() {
+    assert_eq!(Ok(4), parse_width("4"));
+    // Not octal.
+    assert_eq!(Ok(10), parse_width("010"));
+    assert_eq!(Ok(8), parse_width("08"));
+    assert_eq!(Ok(0), parse_width("00"));
+    assert_eq!(Ok(u64::MAX), parse_width("18446744073709551615"));
+
+    for s in [
+        "1K",
+        "1k",
+        "2b",
+        "1KB",
+        "0x8",
+        "4B",
+        "99999999999999999999K",
+    ] {
+        assert!(matches!(
+            parse_width(s),
+            Err(ParseSizeError::InvalidSuffix(_))
+        ));
+    }
+    for s in ["", "abc", "-4", "+4", "K"] {
+        assert!(matches!(
+            parse_width(s),
+            Err(ParseSizeError::ParseFailure(_))
+        ));
+    }
+    assert!(matches!(
+        parse_width("18446744073709551616"),
+        Err(ParseSizeError::SizeTooBig(_))
+    ));
+}

tests/by-util/test_nl.rs

@@ -976,3 +976,50 @@ fn test_no_skip_after_error() {
         .fails()
         .stdout_is("     1\thello\n");
 }
+
+#[test]
+fn test_join_blank_lines_above_u64_saturates() {
+    // Too many blank lines to ever join, so the value saturates instead of
+    // being rejected.
+    for value in [
+        "18446744073709551615",
+        "18446744073709551616",
+        "99999999999999999999",
+        "99999999999999999999999999999999999999999999999",
+    ] {
+        for arg in [format!("-l{value}"), format!("--join-blank-lines={value}")] {
+            new_ucmd!()
+                .arg(arg)
+                .arg("-ba")
+                .pipe_in("a\n\n\n\nb\n")
+                .succeeds()
+                .stdout_is("     1\ta\n       \n       \n       \n     2\tb\n");
+        }
+    }
+}
+
+#[test]
+fn test_join_blank_lines_still_rejects_invalid_numbers() {
+    for value in [
+        "-1",
+        "abc",
+        "1K",
+        "99999999999999999999x",
+        "-99999999999999999999",
+    ] {
+        new_ucmd!()
+            .arg(format!("--join-blank-lines={value}"))
+            .fails_with_code(1)
+            .stderr_contains(format!("invalid value '{value}'"));
+    }
+}
+
+#[test]
+fn test_other_numeric_options_still_reject_values_above_the_limit() {
+    for arg in ["-v", "-i", "-w"] {
+        new_ucmd!()
+            .args(&[arg, "18446744073709551616"])
+            .fails_with_code(1)
+            .stderr_contains("invalid value '18446744073709551616'");
+    }
+}

tests/by-util/test_od.rs

@@ -1334,15 +1334,9 @@ fn test_od_invalid_bytes() {
     const INVALID_SUFFIX: &str = "1fb4t";
     const BIG_SIZE: &str = "1Y";
 
-    // NOTE:
-    // GNU's od (8.32) with option '--width' does not accept 'Y' as valid suffix.
-    // According to the man page it should be valid in the same way it is valid for
-    // '--read-bytes' and '--skip-bytes'.
-
     let options = [
         "--read-bytes",
         "--skip-bytes",
-        "--width",
         // "--strings", // TODO: consider testing here once '--strings' is implemented
     ];
     for option in &options {
@@ -1652,3 +1646,58 @@ fn test_octal_alias() {
             .stdout_only(unindent(ALPHA_OUT));
     }
 }
+
+#[test]
+fn test_width_rejects_size_suffixes() {
+    // Unlike -N and -j, -w takes a plain number: no multiplier and no radix prefix.
+    for value in ["1K", "1k", "2b", "1KB", "1M", "1Y", "0x8", "4B"] {
+        for (arg, option) in [
+            (format!("-w{value}"), "-w"),
+            (format!("--width={value}"), "--width"),
+        ] {
+            new_ucmd!().arg(arg).fails_with_code(1).stderr_only(format!(
+                "od: invalid suffix in {option} argument '{value}'\n"
+            ));
+        }
+    }
+}
+
+#[test]
+fn test_width_is_decimal_even_with_leading_zero() {
+    for (value, expected) in [
+        (
+            "010",
+            " 61 62 63 64 65 66 67 68 69 6a\n 6b 6c 6d 6e 6f 70 71 72 73 74\n",
+        ),
+        (
+            "08",
+            " 61 62 63 64 65 66 67 68\n 69 6a 6b 6c 6d 6e 6f 70\n 71 72 73 74\n",
+        ),
+    ] {
+        new_ucmd!()
+            .args(&["-An", "-tx1", &format!("-w{value}")])
+            .pipe_in("abcdefghijklmnopqrst")
+            .succeeds()
+            .stdout_only(expected);
+    }
+}
+
+#[test]
+fn test_width_invalid_numbers() {
+    for (value, message) in [
+        ("abc", "invalid --width argument 'abc'"),
+        ("0", "invalid --width argument '0'"),
+        ("00", "invalid --width argument '00'"),
+        ("-4", "invalid --width argument '-4'"),
+        ("1fb4t", "invalid suffix in --width argument '1fb4t'"),
+        (
+            "99999999999999999999",
+            "--width argument '99999999999999999999' too large",
+        ),
+    ] {
+        new_ucmd!()
+            .arg(format!("--width={value}"))
+            .fails_with_code(1)
+            .stderr_only(format!("od: {message}\n"));
+    }
+}