diff --git a/src/uu/nl/src/nl.rs b/src/uu/nl/src/nl.rs index 1c8b058b4d2..9f527dfad47 100644 --- a/src/uu/nl/src/nl.rs +++ b/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 { + s.parse::().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) diff --git a/src/uu/od/src/od.rs b/src/uu/od/src/od.rs index 0caf7ffe643..13476bf0760 100644 --- a/src/uu/od/src/od.rs +++ b/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, diff --git a/src/uu/od/src/parse_nrofbytes.rs b/src/uu/od/src/parse_nrofbytes.rs index a7b10950985..69bd40101ea 100644 --- a/src/uu/od/src/parse_nrofbytes.rs +++ b/src/uu/od/src/parse_nrofbytes.rs @@ -77,6 +77,20 @@ pub fn parse_number_of_bytes(s: &str) -> Result { .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 { + 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(_)) + )); +} diff --git a/tests/by-util/test_nl.rs b/tests/by-util/test_nl.rs index 931f9c86c70..a74879f907f 100644 --- a/tests/by-util/test_nl.rs +++ b/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'"); + } +} diff --git a/tests/by-util/test_od.rs b/tests/by-util/test_od.rs index d0a6d1e6c00..4aa0dc8c596 100644 --- a/tests/by-util/test_od.rs +++ b/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")); + } +}