From 3c7175f00d784768a793a4141c067650bb0ef150 Mon Sep 17 00:00:00 2001 From: Jan Scheer Date: Tue, 1 Jun 2021 12:17:11 +0200 Subject: [PATCH] head/tail: add fixes and tests for bytes/lines NUM arg (undocumented sign) * change tail bytes/lines clap parsing to fix posix override behavior * change tail bytes/lines NUM parsing logic to be consistent with head --- src/uu/head/src/parse.rs | 7 ++-- src/uu/tail/src/tail.rs | 74 ++++++++++++++++++++------------------ tests/by-util/test_head.rs | 22 ++++++++++++ tests/by-util/test_tail.rs | 48 +++++++++++++++++++++++++ 4 files changed, 115 insertions(+), 36 deletions(-) diff --git a/src/uu/head/src/parse.rs b/src/uu/head/src/parse.rs index 3b788f819..b395b330b 100644 --- a/src/uu/head/src/parse.rs +++ b/src/uu/head/src/parse.rs @@ -97,9 +97,12 @@ pub fn parse_num(src: &str) -> Result<(usize, bool), ParseSizeError> { let mut all_but_last = false; if let Some(c) = size_string.chars().next() { - if c == '-' { + if c == '+' || c == '-' { + // head: '+' is not documented (8.32 man pages) size_string = &size_string[1..]; - all_but_last = true; + if c == '-' { + all_but_last = true; + } } } else { return Err(ParseSizeError::ParseFailure(src.to_string())); diff --git a/src/uu/tail/src/tail.rs b/src/uu/tail/src/tail.rs index a4634714c..acaad8c30 100644 --- a/src/uu/tail/src/tail.rs +++ b/src/uu/tail/src/tail.rs @@ -27,7 +27,7 @@ use std::io::{stdin, stdout, BufRead, BufReader, Read, Seek, SeekFrom, Write}; use std::path::Path; use std::thread::sleep; use std::time::Duration; -use uucore::parse_size::parse_size; +use uucore::parse_size::{parse_size, ParseSizeError}; use uucore::ringbuffer::RingBuffer; pub mod options { @@ -42,10 +42,9 @@ pub mod options { pub static PID: &str = "pid"; pub static SLEEP_INT: &str = "sleep-interval"; pub static ZERO_TERM: &str = "zero-terminated"; + pub static ARG_FILES: &str = "files"; } -static ARG_FILES: &str = "files"; - enum FilterMode { Bytes(usize), Lines(usize, u8), // (number of lines, delimiter) @@ -84,6 +83,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { .long(options::BYTES) .takes_value(true) .allow_hyphen_values(true) + .overrides_with_all(&[options::BYTES, options::LINES]) .help("Number of bytes to print"), ) .arg( @@ -98,6 +98,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { .long(options::LINES) .takes_value(true) .allow_hyphen_values(true) + .overrides_with_all(&[options::BYTES, options::LINES]) .help("Number of lines to print"), ) .arg( @@ -137,7 +138,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { .help("Line delimiter is NUL, not newline"), ) .arg( - Arg::with_name(ARG_FILES) + Arg::with_name(options::ARG_FILES) .multiple(true) .takes_value(true) .min_values(1), @@ -171,38 +172,21 @@ pub fn uumain(args: impl uucore::Args) -> i32 { } } - match matches.value_of(options::LINES) { - Some(n) => { - let mut slice: &str = n; - let c = slice.chars().next().unwrap_or('_'); - if c == '+' || c == '-' { - slice = &slice[1..]; - if c == '+' { - settings.beginning = true; - } - } - match parse_size(slice) { - Ok(m) => settings.mode = FilterMode::Lines(m, b'\n'), - Err(e) => crash!(1, "invalid number of bytes: {}", e.to_string()), - } + let mode_and_beginning = if let Some(arg) = matches.value_of(options::BYTES) { + match parse_num(arg) { + Ok((n, beginning)) => (FilterMode::Bytes(n), beginning), + Err(e) => crash!(1, "invalid number of bytes: {}", e.to_string()), } - None => { - if let Some(n) = matches.value_of(options::BYTES) { - let mut slice: &str = n; - let c = slice.chars().next().unwrap_or('_'); - if c == '+' || c == '-' { - slice = &slice[1..]; - if c == '+' { - settings.beginning = true; - } - } - match parse_size(slice) { - Ok(m) => settings.mode = FilterMode::Bytes(m), - Err(e) => crash!(1, "invalid number of bytes: {}", e.to_string()), - } - } + } else if let Some(arg) = matches.value_of(options::LINES) { + match parse_num(arg) { + Ok((n, beginning)) => (FilterMode::Lines(n, b'\n'), beginning), + Err(e) => crash!(1, "invalid number of lines: {}", e.to_string()), } + } else { + (FilterMode::Lines(10, b'\n'), false) }; + settings.mode = mode_and_beginning.0; + settings.beginning = mode_and_beginning.1; if matches.is_present(options::ZERO_TERM) { if let FilterMode::Lines(count, _) = settings.mode { @@ -215,7 +199,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { || matches.is_present(options::verbosity::SILENT); let files: Vec = matches - .values_of(ARG_FILES) + .values_of(options::ARG_FILES) .map(|v| v.map(ToString::to_string).collect()) .unwrap_or_default(); @@ -422,3 +406,25 @@ fn print_byte(stdout: &mut T, ch: u8) { crash!(1, "{}", err); } } + +fn parse_num(src: &str) -> Result<(usize, bool), ParseSizeError> { + let mut size_string = src.trim(); + let mut starting_with = false; + + if let Some(c) = size_string.chars().next() { + if c == '+' || c == '-' { + // tail: '-' is not documented (8.32 man pages) + size_string = &size_string[1..]; + if c == '+' { + starting_with = true; + } + } + } else { + return Err(ParseSizeError::ParseFailure(src.to_string())); + } + + match parse_size(&size_string) { + Ok(n) => Ok((n, starting_with)), + Err(e) => Err(e), + } +} diff --git a/tests/by-util/test_head.rs b/tests/by-util/test_head.rs index 349fc05d3..f26447636 100755 --- a/tests/by-util/test_head.rs +++ b/tests/by-util/test_head.rs @@ -267,3 +267,25 @@ fn test_head_invalid_num() { "head: invalid number of lines: ‘1Y’: Value too large to be stored in data type", ); } + +#[test] +fn test_head_num_with_undocumented_sign_bytes() { + // tail: '-' is not documented (8.32 man pages) + // head: '+' is not documented (8.32 man pages) + const ALPHABET: &str = "abcdefghijklmnopqrstuvwxyz"; + new_ucmd!() + .args(&["-c", "5"]) + .pipe_in(ALPHABET) + .succeeds() + .stdout_is("abcde"); + new_ucmd!() + .args(&["-c", "-5"]) + .pipe_in(ALPHABET) + .succeeds() + .stdout_is("abcdefghijklmnopqrstu"); + new_ucmd!() + .args(&["-c", "+5"]) + .pipe_in(ALPHABET) + .succeeds() + .stdout_is("abcde"); +} diff --git a/tests/by-util/test_tail.rs b/tests/by-util/test_tail.rs index 6227ac60b..9d0462c7a 100644 --- a/tests/by-util/test_tail.rs +++ b/tests/by-util/test_tail.rs @@ -352,3 +352,51 @@ fn test_positive_zero_lines() { .succeeds() .stdout_is("a\nb\nc\nd\ne\n"); } + +#[test] +fn test_tail_invalid_num() { + new_ucmd!() + .args(&["-c", "1024R", "emptyfile.txt"]) + .fails() + .stderr_is("tail: invalid number of bytes: ‘1024R’"); + new_ucmd!() + .args(&["-n", "1024R", "emptyfile.txt"]) + .fails() + .stderr_is("tail: invalid number of lines: ‘1024R’"); + #[cfg(not(target_pointer_width = "128"))] + new_ucmd!() + .args(&["-c", "1Y", "emptyfile.txt"]) + .fails() + .stderr_is( + "tail: invalid number of bytes: ‘1Y’: Value too large to be stored in data type", + ); + #[cfg(not(target_pointer_width = "128"))] + new_ucmd!() + .args(&["-n", "1Y", "emptyfile.txt"]) + .fails() + .stderr_is( + "tail: invalid number of lines: ‘1Y’: Value too large to be stored in data type", + ); +} + +#[test] +fn test_tail_num_with_undocumented_sign_bytes() { + // tail: '-' is not documented (8.32 man pages) + // head: '+' is not documented (8.32 man pages) + const ALPHABET: &str = "abcdefghijklmnopqrstuvwxyz"; + new_ucmd!() + .args(&["-c", "5"]) + .pipe_in(ALPHABET) + .succeeds() + .stdout_is("vwxyz"); + new_ucmd!() + .args(&["-c", "-5"]) + .pipe_in(ALPHABET) + .succeeds() + .stdout_is("vwxyz"); + new_ucmd!() + .args(&["-c", "+5"]) + .pipe_in(ALPHABET) + .succeeds() + .stdout_is("efghijklmnopqrstuvwxyz"); +}