From 2737722973234d56da2322210b8a91df7011d735 Mon Sep 17 00:00:00 2001 From: fixfon <12512757+fixfon@users.noreply.github.com> Date: Wed, 21 Oct 2026 15:40:06 +0300 Subject: [PATCH 1/4] od: reject size suffixes in -w/--width -w takes a plain decimal number. It was parsed like a size, so 1K, 2b or 0x8 were accepted and a leading zero meant octal. --- src/uu/od/src/od.rs | 4 +- src/uu/od/src/parse_nrofbytes.rs | 49 ++++++++++++++++++++++++ tests/by-util/test_od.rs | 66 ++++++++++++++++++++++++++++++-- 3 files changed, 113 insertions(+), 6 deletions(-) diff --git a/src/uu/od/src/od.rs b/src/uu/od/src/od.rs index 0caf7ffe643..5173cb481e4 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_plain_number}; 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_plain_number(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..797bc1ef610 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_plain_number(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_plain_number() { + assert_eq!(Ok(4), parse_plain_number("4")); + // Not octal. + assert_eq!(Ok(10), parse_plain_number("010")); + assert_eq!(Ok(8), parse_plain_number("08")); + assert_eq!(Ok(0), parse_plain_number("00")); + assert_eq!(Ok(u64::MAX), parse_plain_number("18446744073709551615")); + + for s in [ + "1K", + "1k", + "2b", + "1KB", + "0x8", + "4B", + "99999999999999999999K", + ] { + assert!(matches!( + parse_plain_number(s), + Err(ParseSizeError::InvalidSuffix(_)) + )); + } + for s in ["", "abc", "-4", "+4", "K"] { + assert!(matches!( + parse_plain_number(s), + Err(ParseSizeError::ParseFailure(_)) + )); + } + assert!(matches!( + parse_plain_number("18446744073709551616"), + Err(ParseSizeError::SizeTooBig(_)) + )); +} diff --git a/tests/by-util/test_od.rs b/tests/by-util/test_od.rs index d0a6d1e6c00..e0534b16881 100644 --- a/tests/by-util/test_od.rs +++ b/tests/by-util/test_od.rs @@ -1335,14 +1335,12 @@ fn test_od_invalid_bytes() { 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'. + // '--width' takes a plain number, so it does not accept 'Y' as a suffix + // (see test_width_rejects_size_suffixes). let options = [ "--read-bytes", "--skip-bytes", - "--width", // "--strings", // TODO: consider testing here once '--strings' is implemented ]; for option in &options { @@ -1652,3 +1650,63 @@ 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) + .pipe_in("abcdefgh") + .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}")) + .pipe_in("abcdefgh") + .fails_with_code(1) + .stderr_only(format!("od: {message}\n")); + } +} From 9d6375c97b46f3193c87166e61e0cd8390555454 Mon Sep 17 00:00:00 2001 From: fixfon <12512757+fixfon@users.noreply.github.com> Date: Wed, 21 Oct 2026 15:40:06 +0300 Subject: [PATCH 2/4] nl: accept a -l/--join-blank-lines value above u64::MAX A number too large for u64 now saturates instead of being rejected. -v, -i and -w still reject values that are too large. --- src/uu/nl/src/nl.rs | 22 +++++++++++++++++- tests/by-util/test_nl.rs | 49 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 1 deletion(-) 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/tests/by-util/test_nl.rs b/tests/by-util/test_nl.rs index 931f9c86c70..8d892625b3b 100644 --- a/tests/by-util/test_nl.rs +++ b/tests/by-util/test_nl.rs @@ -976,3 +976,52 @@ 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}")) + .pipe_in("a\n") + .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"]) + .pipe_in("a\n") + .fails_with_code(1) + .stderr_contains("invalid value '18446744073709551616'"); + } +} From ceb4220b65125cd5d660f8150a8d6b590ee7fd96 Mon Sep 17 00:00:00 2001 From: fixfon <12512757+fixfon@users.noreply.github.com> Date: Wed, 21 Oct 2026 19:29:49 +0300 Subject: [PATCH 3/4] od, nl: rename parse_width and fix review nits Rename parse_plain_number to parse_width, drop the test_od note that no longer applies now that --width is out of the list, and stop piping stdin into the nl test whose options are rejected before any input is read (the write raced with nl exiting and hit a broken pipe on i686-musl). --- src/uu/od/src/od.rs | 4 ++-- src/uu/od/src/parse_nrofbytes.rs | 20 ++++++++++---------- tests/by-util/test_nl.rs | 1 - tests/by-util/test_od.rs | 4 ---- 4 files changed, 12 insertions(+), 17 deletions(-) diff --git a/src/uu/od/src/od.rs b/src/uu/od/src/od.rs index 5173cb481e4..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, parse_plain_number}; +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_plain_number(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 797bc1ef610..69bd40101ea 100644 --- a/src/uu/od/src/parse_nrofbytes.rs +++ b/src/uu/od/src/parse_nrofbytes.rs @@ -79,7 +79,7 @@ pub fn parse_number_of_bytes(s: &str) -> Result { /// Parses a plain decimal number, as `-w` takes: no radix prefix (a leading /// zero is not octal) and no multiplier suffix. -pub fn parse_plain_number(s: &str) -> Result { +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())); @@ -110,13 +110,13 @@ fn test_parse_number_of_bytes() { } #[test] -fn test_parse_plain_number() { - assert_eq!(Ok(4), parse_plain_number("4")); +fn test_parse_width() { + assert_eq!(Ok(4), parse_width("4")); // Not octal. - assert_eq!(Ok(10), parse_plain_number("010")); - assert_eq!(Ok(8), parse_plain_number("08")); - assert_eq!(Ok(0), parse_plain_number("00")); - assert_eq!(Ok(u64::MAX), parse_plain_number("18446744073709551615")); + 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", @@ -128,18 +128,18 @@ fn test_parse_plain_number() { "99999999999999999999K", ] { assert!(matches!( - parse_plain_number(s), + parse_width(s), Err(ParseSizeError::InvalidSuffix(_)) )); } for s in ["", "abc", "-4", "+4", "K"] { assert!(matches!( - parse_plain_number(s), + parse_width(s), Err(ParseSizeError::ParseFailure(_)) )); } assert!(matches!( - parse_plain_number("18446744073709551616"), + parse_width("18446744073709551616"), Err(ParseSizeError::SizeTooBig(_)) )); } diff --git a/tests/by-util/test_nl.rs b/tests/by-util/test_nl.rs index 8d892625b3b..86304071ec1 100644 --- a/tests/by-util/test_nl.rs +++ b/tests/by-util/test_nl.rs @@ -1020,7 +1020,6 @@ fn test_other_numeric_options_still_reject_values_above_the_limit() { for arg in ["-v", "-i", "-w"] { new_ucmd!() .args(&[arg, "18446744073709551616"]) - .pipe_in("a\n") .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 e0534b16881..0c540bf255b 100644 --- a/tests/by-util/test_od.rs +++ b/tests/by-util/test_od.rs @@ -1334,10 +1334,6 @@ fn test_od_invalid_bytes() { const INVALID_SUFFIX: &str = "1fb4t"; const BIG_SIZE: &str = "1Y"; - // NOTE: - // '--width' takes a plain number, so it does not accept 'Y' as a suffix - // (see test_width_rejects_size_suffixes). - let options = [ "--read-bytes", "--skip-bytes", From 96f4a785c58c1d0c1a7fc356a6d0563706fd3582 Mon Sep 17 00:00:00 2001 From: fixfon <12512757+fixfon@users.noreply.github.com> Date: Wed, 21 Oct 2026 20:36:34 +0300 Subject: [PATCH 4/4] tests/od, tests/nl: don't pipe stdin into commands that reject their arguments These commands exit on the bad argument without reading stdin, so writing to the child's stdin raced with its exit and could fail with a broken pipe. --- tests/by-util/test_nl.rs | 1 - tests/by-util/test_od.rs | 11 +++-------- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/tests/by-util/test_nl.rs b/tests/by-util/test_nl.rs index 86304071ec1..a74879f907f 100644 --- a/tests/by-util/test_nl.rs +++ b/tests/by-util/test_nl.rs @@ -1009,7 +1009,6 @@ fn test_join_blank_lines_still_rejects_invalid_numbers() { ] { new_ucmd!() .arg(format!("--join-blank-lines={value}")) - .pipe_in("a\n") .fails_with_code(1) .stderr_contains(format!("invalid value '{value}'")); } diff --git a/tests/by-util/test_od.rs b/tests/by-util/test_od.rs index 0c540bf255b..4aa0dc8c596 100644 --- a/tests/by-util/test_od.rs +++ b/tests/by-util/test_od.rs @@ -1655,13 +1655,9 @@ fn test_width_rejects_size_suffixes() { (format!("-w{value}"), "-w"), (format!("--width={value}"), "--width"), ] { - new_ucmd!() - .arg(arg) - .pipe_in("abcdefgh") - .fails_with_code(1) - .stderr_only(format!( - "od: invalid suffix in {option} argument '{value}'\n" - )); + new_ucmd!().arg(arg).fails_with_code(1).stderr_only(format!( + "od: invalid suffix in {option} argument '{value}'\n" + )); } } } @@ -1701,7 +1697,6 @@ fn test_width_invalid_numbers() { ] { new_ucmd!() .arg(format!("--width={value}")) - .pipe_in("abcdefgh") .fails_with_code(1) .stderr_only(format!("od: {message}\n")); }