diff --git a/src/uu/split/src/split.rs b/src/uu/split/src/split.rs index 517031791b..d76bfb2de7 100644 --- a/src/uu/split/src/split.rs +++ b/src/uu/split/src/split.rs @@ -13,6 +13,7 @@ use crate::filenames::FilenameIterator; use crate::filenames::SuffixType; use clap::{crate_version, parser::ValueSource, Arg, ArgAction, ArgMatches, Command}; use std::env; +use std::ffi::OsString; use std::fmt; use std::fs::{metadata, File}; use std::io; @@ -52,14 +53,127 @@ const AFTER_HELP: &str = help_section!("after help", "split.md"); #[uucore::main] pub fn uumain(args: impl uucore::Args) -> UResult<()> { + let (args, obs_lines) = handle_obsolete(args); + let matches = uu_app().try_get_matches_from(args)?; - match Settings::from(&matches) { + + match Settings::from(&matches, &obs_lines) { Ok(settings) => split(&settings), Err(e) if e.requires_usage() => Err(UUsageError::new(1, format!("{e}"))), Err(e) => Err(USimpleError::new(1, format!("{e}"))), } } +/// Extract obsolete shorthand (if any) for specifying lines in following scenarios (and similar) +/// `split -22 file` would mean `split -l 22 file` +/// `split -2de file` would mean `split -l 2 -d -e file` +/// `split -x300e file` would mean `split -x -l 300 -e file` +/// `split -x300e -22 file` would mean `split -x -e -l 22 file` (last obsolete lines option wins) +/// following GNU `split` behavior +fn handle_obsolete(args: impl uucore::Args) -> (Vec, Option) { + let mut obs_lines = None; + let mut preceding_long_opt_req_value = false; + let mut preceding_short_opt_req_value = false; + let filtered_args = args + .filter_map(|os_slice| { + let filter: Option; + if let Some(slice) = os_slice.to_str() { + // check if the slice is a true short option (and not hyphen prefixed value of an option) + // and if so, a short option that can contain obsolete lines value + if slice.starts_with('-') + && !slice.starts_with("--") + && !preceding_long_opt_req_value + && !preceding_short_opt_req_value + && !slice.starts_with("-a") + && !slice.starts_with("-b") + && !slice.starts_with("-C") + && !slice.starts_with("-l") + && !slice.starts_with("-n") + { + // start of the short option string + // that can have obsolete lines option value in it + // extract numeric part and filter it out + let mut obs_lines_extracted: Vec = vec![]; + let mut obs_lines_end_reached = false; + let filtered_slice: Vec = slice + .chars() + .filter(|c| { + // To correctly process scenario like '-x200a4' + // we need to stop extracting digits once alphabetic character is encountered + // after we already have something in obs_lines_extracted + if c.is_ascii_digit() && !obs_lines_end_reached { + obs_lines_extracted.push(*c); + false + } else { + if !obs_lines_extracted.is_empty() { + obs_lines_end_reached = true; + } + true + } + }) + .collect(); + + if obs_lines_extracted.is_empty() { + // no obsolete lines value found/extracted + filter = Some(OsString::from(slice)); + } else { + // obsolete lines value was extracted + obs_lines = Some(obs_lines_extracted.iter().collect()); + if filtered_slice.get(1).is_some() { + // there were some short options in front of or after obsolete lines value + // i.e. '-xd100' or '-100de' or similar, which after extraction of obsolete lines value + // would look like '-xd' or '-de' or similar + let filtered_slice: String = filtered_slice.iter().collect(); + filter = Some(OsString::from(filtered_slice)); + } else { + filter = None; + } + } + } else { + // either not a short option + // or a short option that cannot have obsolete lines value in it + filter = Some(OsString::from(slice)); + } + // capture if current slice is a preceding long option that requires value and does not use '=' to assign that value + // following slice should be treaded as value for this option + // even if it starts with '-' (which would be treated as hyphen prefixed value) + if slice.starts_with("--") { + preceding_long_opt_req_value = &slice[2..] == OPT_BYTES + || &slice[2..] == OPT_LINE_BYTES + || &slice[2..] == OPT_LINES + || &slice[2..] == OPT_ADDITIONAL_SUFFIX + || &slice[2..] == OPT_FILTER + || &slice[2..] == OPT_NUMBER + || &slice[2..] == OPT_SUFFIX_LENGTH; + } + // capture if current slice is a preceding short option that requires value and does not have value in the same slice (value separated by whitespace) + // following slice should be treaded as value for this option + // even if it starts with '-' (which would be treated as hyphen prefixed value) + preceding_short_opt_req_value = slice == "-b" + || slice == "-C" + || slice == "-l" + || slice == "-n" + || slice == "-a"; + // slice is a value + // reset preceding option flags + if !slice.starts_with('-') { + preceding_short_opt_req_value = false; + preceding_long_opt_req_value = false; + } + } else { + // Cannot cleanly convert os_slice to UTF-8 + // Do not process and return as-is + // This will cause failure later on, but we should not handle it here + // and let clap panic on invalid UTF-8 argument + filter = Some(os_slice); + } + // return filter + filter + }) + .collect(); + (filtered_args, obs_lines) +} + pub fn uu_app() -> Command { Command::new(uucore::util_name()) .version(crate_version!()) @@ -72,6 +186,7 @@ pub fn uu_app() -> Command { Arg::new(OPT_BYTES) .short('b') .long(OPT_BYTES) + .allow_hyphen_values(true) .value_name("SIZE") .help("put SIZE bytes per output file"), ) @@ -79,14 +194,15 @@ pub fn uu_app() -> Command { Arg::new(OPT_LINE_BYTES) .short('C') .long(OPT_LINE_BYTES) + .allow_hyphen_values(true) .value_name("SIZE") - .default_value("2") .help("put at most SIZE bytes of lines per output file"), ) .arg( Arg::new(OPT_LINES) .short('l') .long(OPT_LINES) + .allow_hyphen_values(true) .value_name("NUMBER") .default_value("1000") .help("put NUMBER lines/records per output file"), @@ -95,6 +211,7 @@ pub fn uu_app() -> Command { Arg::new(OPT_NUMBER) .short('n') .long(OPT_NUMBER) + .allow_hyphen_values(true) .value_name("CHUNKS") .help("generate CHUNKS output files; see explanation below"), ) @@ -102,6 +219,7 @@ pub fn uu_app() -> Command { .arg( Arg::new(OPT_ADDITIONAL_SUFFIX) .long(OPT_ADDITIONAL_SUFFIX) + .allow_hyphen_values(true) .value_name("SUFFIX") .default_value("") .help("additional SUFFIX to append to output file names"), @@ -109,6 +227,7 @@ pub fn uu_app() -> Command { .arg( Arg::new(OPT_FILTER) .long(OPT_FILTER) + .allow_hyphen_values(true) .value_name("COMMAND") .value_hint(clap::ValueHint::CommandName) .help( @@ -178,9 +297,10 @@ pub fn uu_app() -> Command { Arg::new(OPT_SUFFIX_LENGTH) .short('a') .long(OPT_SUFFIX_LENGTH) + .allow_hyphen_values(true) .value_name("N") .default_value(OPT_DEFAULT_SUFFIX_LENGTH) - .help("use suffixes of fixed length N. 0 implies dynamic length."), + .help("use suffixes of fixed length N. 0 implies dynamic length, starting with 2"), ) .arg( Arg::new(OPT_VERBOSE) @@ -395,7 +515,7 @@ impl fmt::Display for StrategyError { impl Strategy { /// Parse a strategy from the command-line arguments. - fn from(matches: &ArgMatches) -> Result { + fn from(matches: &ArgMatches, obs_lines: &Option) -> Result { fn get_and_parse( matches: &ArgMatches, option: &str, @@ -413,28 +533,34 @@ impl Strategy { // Check that the user is not specifying more than one strategy. // // Note: right now, this exact behavior cannot be handled by - // `ArgGroup` since `ArgGroup` considers a default value `Arg` - // as "defined". + // overrides_with_all() due to obsolete lines value option match ( + obs_lines, matches.value_source(OPT_LINES) == Some(ValueSource::CommandLine), matches.value_source(OPT_BYTES) == Some(ValueSource::CommandLine), matches.value_source(OPT_LINE_BYTES) == Some(ValueSource::CommandLine), matches.value_source(OPT_NUMBER) == Some(ValueSource::CommandLine), ) { - (false, false, false, false) => Ok(Self::Lines(1000)), - (true, false, false, false) => { + (Some(v), false, false, false, false) => { + let v = parse_size(v).map_err(|_| { + StrategyError::Lines(ParseSizeError::ParseFailure(v.to_string())) + })?; + Ok(Self::Lines(v)) + } + (None, false, false, false, false) => Ok(Self::Lines(1000)), + (None, true, false, false, false) => { get_and_parse(matches, OPT_LINES, Self::Lines, StrategyError::Lines) } - (false, true, false, false) => { + (None, false, true, false, false) => { get_and_parse(matches, OPT_BYTES, Self::Bytes, StrategyError::Bytes) } - (false, false, true, false) => get_and_parse( + (None, false, false, true, false) => get_and_parse( matches, OPT_LINE_BYTES, Self::LineBytes, StrategyError::Bytes, ), - (false, false, false, true) => { + (None, false, false, false, true) => { let s = matches.get_one::(OPT_NUMBER).unwrap(); let number_type = NumberType::from(s).map_err(StrategyError::NumberType)?; Ok(Self::Number(number_type)) @@ -553,7 +679,7 @@ impl fmt::Display for SettingsError { impl Settings { /// Parse a strategy from the command-line arguments. - fn from(matches: &ArgMatches) -> Result { + fn from(matches: &ArgMatches, obs_lines: &Option) -> Result { let additional_suffix = matches .get_one::(OPT_ADDITIONAL_SUFFIX) .unwrap() @@ -561,7 +687,8 @@ impl Settings { if additional_suffix.contains('/') { return Err(SettingsError::SuffixContainsSeparator(additional_suffix)); } - let strategy = Strategy::from(matches).map_err(SettingsError::Strategy)?; + + let strategy = Strategy::from(matches, obs_lines).map_err(SettingsError::Strategy)?; let (suffix_type, suffix_start) = suffix_type_from(matches)?; let suffix_length_str = matches.get_one::(OPT_SUFFIX_LENGTH).unwrap(); let suffix_length: usize = suffix_length_str diff --git a/tests/by-util/test_split.rs b/tests/by-util/test_split.rs index 0b7bbfec6d..053b6f8bf1 100644 --- a/tests/by-util/test_split.rs +++ b/tests/by-util/test_split.rs @@ -170,6 +170,22 @@ fn test_split_str_prefixed_chunks_by_bytes() { assert_eq!(glob.collate(), at.read_bytes(name)); } +/// Test short bytes option concatenated with value +#[test] +fn test_split_by_bytes_short_concatenated_with_value() { + let (at, mut ucmd) = at_and_ucmd!(); + let name = "split_by_bytes_short_concatenated_with_value"; + RandomFile::new(&at, name).add_bytes(10000); + ucmd.args(&["-b1000", name]).succeeds(); + + let glob = Glob::new(&at, ".", r"x[[:alpha:]][[:alpha:]]$"); + assert_eq!(glob.count(), 10); + for filename in glob.collect() { + assert_eq!(glob.directory.metadata(&filename).len(), 1000); + } + assert_eq!(glob.collate(), at.read_bytes(name)); +} + // This is designed to test what happens when the desired part size is not a // multiple of the buffer size and we hopefully don't overshoot the desired part // size. @@ -238,6 +254,18 @@ fn test_additional_suffix_no_slash() { .usage_error("invalid suffix 'a/b', contains directory separator"); } +#[test] +fn test_split_additional_suffix_hyphen_value() { + let (at, mut ucmd) = at_and_ucmd!(); + let name = "split_additional_suffix"; + RandomFile::new(&at, name).add_lines(2000); + ucmd.args(&["--additional-suffix", "-300", name]).succeeds(); + + let glob = Glob::new(&at, ".", r"x[[:alpha:]][[:alpha:]]-300$"); + assert_eq!(glob.count(), 2); + assert_eq!(glob.collate(), at.read_bytes(name)); +} + // note: the test_filter* tests below are unix-only // windows support has been waived for now because of the difficulty of getting // the `cmd` call right @@ -318,6 +346,259 @@ fn test_split_lines_number() { .fails() .code_is(1) .stderr_only("split: invalid number of lines: '2fb'\n"); + scene + .ucmd() + .args(&["--lines", "file"]) + .fails() + .code_is(1) + .stderr_only("split: invalid number of lines: 'file'\n"); +} + +/// Test short lines option with value concatenated +#[test] +fn test_split_lines_short_concatenated_with_value() { + let (at, mut ucmd) = at_and_ucmd!(); + let name = "split_num_prefixed_chunks_by_lines"; + RandomFile::new(&at, name).add_lines(10000); + ucmd.args(&["-l1000", name]).succeeds(); + + let glob = Glob::new(&at, ".", r"x[[:alpha:]][[:alpha:]]$"); + assert_eq!(glob.count(), 10); + assert_eq!(glob.collate(), at.read_bytes(name)); +} + +/// Test for obsolete lines option standalone +#[test] +fn test_split_obs_lines_standalone() { + let (at, mut ucmd) = at_and_ucmd!(); + let name = "obs-lines-standalone"; + RandomFile::new(&at, name).add_lines(4); + ucmd.args(&["-2", name]).succeeds().no_stderr().no_stdout(); + let glob = Glob::new(&at, ".", r"x[[:alpha:]][[:alpha:]]$"); + assert_eq!(glob.count(), 2); + assert_eq!(glob.collate(), at.read_bytes(name)); +} + +/// Test for obsolete lines option as part of invalid combined short options +#[test] +fn test_split_obs_lines_within_invalid_combined_shorts() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + at.touch("file"); + + scene + .ucmd() + .args(&["-2fb", "file"]) + .fails() + .code_is(1) + .stderr_contains("error: unexpected argument '-f' found\n"); +} + +/// Test for obsolete lines option as part of combined short options +#[test] +fn test_split_obs_lines_within_combined_shorts() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + let name = "obs-lines-within-shorts"; + RandomFile::new(&at, name).add_lines(400); + + scene + .ucmd() + .args(&["-x200de", name]) + .succeeds() + .no_stderr() + .no_stdout(); + let glob = Glob::new(&at, ".", r"x\d\d$"); + assert_eq!(glob.count(), 2); + assert_eq!(glob.collate(), at.read_bytes(name)) +} + +/// Test for obsolete lines option as part of combined short options with tailing suffix length with value +#[test] +fn test_split_obs_lines_within_combined_shorts_tailing_suffix_length() { + let (at, mut ucmd) = at_and_ucmd!(); + let name = "obs-lines-combined-shorts-tailing-suffix-length"; + RandomFile::new(&at, name).add_lines(1000); + ucmd.args(&["-d200a4", name]).succeeds(); + + let glob = Glob::new(&at, ".", r"x\d\d\d\d$"); + assert_eq!(glob.count(), 5); + assert_eq!(glob.collate(), at.read_bytes(name)); +} + +/// Test for obsolete lines option starts as part of combined short options +#[test] +fn test_split_obs_lines_starts_combined_shorts() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + let name = "obs-lines-starts-shorts"; + RandomFile::new(&at, name).add_lines(400); + + scene + .ucmd() + .args(&["-200xd", name]) + .succeeds() + .no_stderr() + .no_stdout(); + let glob = Glob::new(&at, ".", r"x\d\d$"); + assert_eq!(glob.count(), 2); + assert_eq!(glob.collate(), at.read_bytes(name)) +} + +/// Test for using both obsolete lines (standalone) option and short/long lines option simultaneously +#[test] +fn test_split_both_lines_and_obs_lines_standalone() { + // This test will ensure that: + // if both lines option '-l' or '--lines' (with value) and obsolete lines option '-100' are used - it fails + // if standalone lines option is used incorrectly and treated as a hyphen prefixed value of other option - it fails + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + at.touch("file"); + + scene + .ucmd() + .args(&["-l", "2", "-2", "file"]) + .fails() + .code_is(1) + .stderr_contains("split: cannot split in more than one way\n"); + scene + .ucmd() + .args(&["--lines", "2", "-2", "file"]) + .fails() + .code_is(1) + .stderr_contains("split: cannot split in more than one way\n"); +} + +/// Test for using obsolete lines option incorrectly, so it is treated as a hyphen prefixed value of other option +#[test] +fn test_split_obs_lines_as_other_option_value() { + // This test will ensure that: + // if obsolete lines option is used incorrectly and treated as a hyphen prefixed value of other option - it fails + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + at.touch("file"); + + scene + .ucmd() + .args(&["--lines", "-200", "file"]) + .fails() + .code_is(1) + .stderr_contains("split: invalid number of lines: '-200'\n"); + scene + .ucmd() + .args(&["-l", "-200", "file"]) + .fails() + .code_is(1) + .stderr_contains("split: invalid number of lines: '-200'\n"); + scene + .ucmd() + .args(&["-a", "-200", "file"]) + .fails() + .code_is(1) + .stderr_contains("split: invalid suffix length: '-200'\n"); + scene + .ucmd() + .args(&["--suffix-length", "-d200e", "file"]) + .fails() + .code_is(1) + .stderr_contains("split: invalid suffix length: '-d200e'\n"); + scene + .ucmd() + .args(&["-C", "-200", "file"]) + .fails() + .code_is(1) + .stderr_contains("split: invalid number of bytes: '-200'\n"); + scene + .ucmd() + .args(&["--line-bytes", "-x200a4", "file"]) + .fails() + .code_is(1) + .stderr_contains("split: invalid number of bytes: '-x200a4'\n"); + scene + .ucmd() + .args(&["-b", "-200", "file"]) + .fails() + .code_is(1) + .stderr_contains("split: invalid number of bytes: '-200'\n"); + scene + .ucmd() + .args(&["--bytes", "-200xd", "file"]) + .fails() + .code_is(1) + .stderr_contains("split: invalid number of bytes: '-200xd'\n"); + scene + .ucmd() + .args(&["-n", "-200", "file"]) + .fails() + .code_is(1) + .stderr_contains("split: invalid number of chunks: -200\n"); + scene + .ucmd() + .args(&["--number", "-e200", "file"]) + .fails() + .code_is(1) + .stderr_contains("split: invalid number of chunks: -e200\n"); +} + +/// Test for using more than one obsolete lines option (standalone) +/// last one wins +#[test] +fn test_split_multiple_obs_lines_standalone() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + let name = "multiple-obs-lines"; + RandomFile::new(&at, name).add_lines(400); + + scene + .ucmd() + .args(&["-3000", "-200", name]) + .succeeds() + .no_stderr() + .no_stdout(); + let glob = Glob::new(&at, ".", r"x[[:alpha:]][[:alpha:]]$"); + assert_eq!(glob.count(), 2); + assert_eq!(glob.collate(), at.read_bytes(name)) +} + +/// Test for using more than one obsolete lines option within combined shorts +/// last one wins +#[test] +fn test_split_multiple_obs_lines_within_combined() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + let name = "multiple-obs-lines"; + RandomFile::new(&at, name).add_lines(400); + + scene + .ucmd() + .args(&["-d5000x", "-e200d", name]) + .succeeds() + .no_stderr() + .no_stdout(); + let glob = Glob::new(&at, ".", r"x\d\d$"); + assert_eq!(glob.count(), 2); + assert_eq!(glob.collate(), at.read_bytes(name)) +} + +/// Test for using both obsolete lines option within combined shorts with conflicting -n option simultaneously +#[test] +fn test_split_obs_lines_within_combined_with_number() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + at.touch("file"); + + scene + .ucmd() + .args(&["-3dxen", "4", "file"]) + .fails() + .code_is(1) + .stderr_contains("split: cannot split in more than one way\n"); + scene + .ucmd() + .args(&["-dxe30n", "4", "file"]) + .fails() + .code_is(1) + .stderr_contains("split: cannot split in more than one way\n"); } #[test] @@ -524,6 +805,19 @@ fn test_invalid_suffix_length() { .stderr_contains("invalid suffix length: 'xyz'"); } +/// Test short suffix length option with value concatenated +#[test] +fn test_split_suffix_length_short_concatenated_with_value() { + let (at, mut ucmd) = at_and_ucmd!(); + let name = "split_num_prefixed_chunks_by_lines"; + RandomFile::new(&at, name).add_lines(10000); + ucmd.args(&["-a4", name]).succeeds(); + + let glob = Glob::new(&at, ".", r"x[[:alpha:]][[:alpha:]][[:alpha:]][[:alpha:]]$"); + assert_eq!(glob.count(), 10); + assert_eq!(glob.collate(), at.read_bytes(name)); +} + #[test] fn test_include_newlines() { let (at, mut ucmd) = at_and_ucmd!(); @@ -542,6 +836,19 @@ fn test_include_newlines() { assert_eq!(s, "5\n"); } +/// Test short number of chunks option concatenated with value +#[test] +fn test_split_number_chunks_short_concatenated_with_value() { + let (at, mut ucmd) = at_and_ucmd!(); + ucmd.args(&["-n3", "threebytes.txt"]) + .succeeds() + .no_stdout() + .no_stderr(); + assert_eq!(at.read("xaa"), "a"); + assert_eq!(at.read("xab"), "b"); + assert_eq!(at.read("xac"), "c"); +} + #[test] fn test_allow_empty_files() { let (at, mut ucmd) = at_and_ucmd!(); @@ -616,6 +923,16 @@ fn test_line_bytes() { assert_eq!(at.read("xad"), "ee\n"); } +#[test] +fn test_line_bytes_concatenated_with_value() { + let (at, mut ucmd) = at_and_ucmd!(); + ucmd.args(&["-C8", "letters.txt"]).succeeds(); + assert_eq!(at.read("xaa"), "aaaaaaaa"); + assert_eq!(at.read("xab"), "a\nbbbb\n"); + assert_eq!(at.read("xac"), "cccc\ndd\n"); + assert_eq!(at.read("xad"), "ee\n"); +} + #[test] fn test_line_bytes_no_final_newline() { let (at, mut ucmd) = at_and_ucmd!(); @@ -970,3 +1287,49 @@ fn test_split_invalid_input() { .no_stdout() .stderr_contains("split: invalid number of chunks: 0"); } + +/// Test if there are invalid (non UTF-8) in the arguments - unix +/// clap is expected to fail/panic +#[test] +#[cfg(unix)] +fn test_split_non_utf8_argument_unix() { + use std::ffi::OsStr; + use std::os::unix::ffi::OsStrExt; + + let (at, mut ucmd) = at_and_ucmd!(); + let name = "test_split_non_utf8_argument"; + let opt = OsStr::from_bytes("--additional-suffix".as_bytes()); + RandomFile::new(&at, name).add_lines(2000); + // Here, the values 0x66 and 0x6f correspond to 'f' and 'o' + // respectively. The value 0x80 is a lone continuation byte, invalid + // in a UTF-8 sequence. + let opt_value = [0x66, 0x6f, 0x80, 0x6f]; + let opt_value = OsStr::from_bytes(&opt_value[..]); + let name = OsStr::from_bytes(name.as_bytes()); + ucmd.args(&[opt, opt_value, name]) + .fails() + .stderr_contains("error: invalid UTF-8 was detected in one or more arguments"); +} + +/// Test if there are invalid (non UTF-8) in the arguments - windows +/// clap is expected to fail/panic +#[test] +#[cfg(windows)] +fn test_split_non_utf8_argument_windows() { + use std::ffi::OsString; + use std::os::windows::ffi::OsStringExt; + + let (at, mut ucmd) = at_and_ucmd!(); + let name = "test_split_non_utf8_argument"; + let opt = OsString::from("--additional-suffix"); + RandomFile::new(&at, name).add_lines(2000); + // Here the values 0x0066 and 0x006f correspond to 'f' and 'o' + // respectively. The value 0xD800 is a lone surrogate half, invalid + // in a UTF-16 sequence. + let opt_value = [0x0066, 0x006f, 0xD800, 0x006f]; + let opt_value = OsString::from_wide(&opt_value[..]); + let name = OsString::from(name); + ucmd.args(&[opt, opt_value, name]) + .fails() + .stderr_contains("error: invalid UTF-8 was detected in one or more arguments"); +}