From 8cc7a90d7ceb580b695ac5c2de264467456bbd69 Mon Sep 17 00:00:00 2001 From: Yagiz Degirmenci <62724709+ycd@users.noreply.github.com> Date: Mon, 29 Mar 2021 14:03:56 +0300 Subject: [PATCH] sum: fix crash on invalid inputs, move to clap, add tests (#1952) --- Cargo.lock | 6 +-- src/uu/sum/Cargo.toml | 2 +- src/uu/sum/src/sum.rs | 93 ++++++++++++++++++++++----------------- tests/by-util/test_sum.rs | 20 +++++++++ 4 files changed, 77 insertions(+), 44 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8d42c573e..c950fb58e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2250,9 +2250,9 @@ dependencies = [ name = "uu_sum" version = "0.0.4" dependencies = [ - "getopts", - "uucore", - "uucore_procs", + "clap 2.33.3 (registry+https://github.com/rust-lang/crates.io-index)", + "uucore 0.0.7", + "uucore_procs 0.0.5", ] [[package]] diff --git a/src/uu/sum/Cargo.toml b/src/uu/sum/Cargo.toml index a880ba1f7..1e8590616 100644 --- a/src/uu/sum/Cargo.toml +++ b/src/uu/sum/Cargo.toml @@ -15,7 +15,7 @@ edition = "2018" path = "src/sum.rs" [dependencies] -getopts = "0.2.18" +clap = "2.33" uucore = { version=">=0.0.7", package="uucore", path="../../uucore" } uucore_procs = { version=">=0.0.5", package="uucore_procs", path="../../uucore_procs" } diff --git a/src/uu/sum/src/sum.rs b/src/uu/sum/src/sum.rs index 5c1652196..ed5655a3d 100644 --- a/src/uu/sum/src/sum.rs +++ b/src/uu/sum/src/sum.rs @@ -10,12 +10,16 @@ #[macro_use] extern crate uucore; +use clap::{App, Arg}; use std::fs::File; use std::io::{stdin, Read, Result}; use std::path::Path; static NAME: &str = "sum"; static VERSION: &str = env!("CARGO_PKG_VERSION"); +static USAGE: &str = + "[OPTION]... [FILE]...\nWith no FILE, or when FILE is -, read standard input."; +static SUMMARY: &str = "Checksum and count the blocks in a file."; fn bsd_sum(mut reader: Box) -> (usize, u16) { let mut buf = [0; 1024]; @@ -64,55 +68,59 @@ fn open(name: &str) -> Result> { match name { "-" => Ok(Box::new(stdin()) as Box), _ => { - let f = File::open(&Path::new(name))?; + let path = &Path::new(name); + if path.is_dir() { + return Err(std::io::Error::new( + std::io::ErrorKind::InvalidInput, + "Is a directory", + )); + }; + if !path.metadata().is_ok() { + return Err(std::io::Error::new( + std::io::ErrorKind::NotFound, + "No such file or directory", + )); + }; + let f = File::open(path)?; Ok(Box::new(f) as Box) } } } +mod options { + pub static FILE: &str = "file"; + pub static BSD_COMPATIBLE: &str = "r"; + pub static SYSTEM_V_COMPATIBLE: &str = "sysv"; +} + pub fn uumain(args: impl uucore::Args) -> i32 { let args = args.collect_str(); - let mut opts = getopts::Options::new(); + let matches = App::new(executable!()) + .name(NAME) + .version(VERSION) + .usage(USAGE) + .about(SUMMARY) + .arg(Arg::with_name(options::FILE).multiple(true).hidden(true)) + .arg( + Arg::with_name(options::BSD_COMPATIBLE) + .short(options::BSD_COMPATIBLE) + .help("use the BSD compatible algorithm (default)"), + ) + .arg( + Arg::with_name(options::SYSTEM_V_COMPATIBLE) + .short("s") + .long(options::SYSTEM_V_COMPATIBLE) + .help("use the BSD compatible algorithm (default)"), + ) + .get_matches_from(args); - opts.optflag("r", "", "use the BSD compatible algorithm (default)"); - opts.optflag("s", "sysv", "use System V compatible algorithm"); - opts.optflag("h", "help", "show this help message"); - opts.optflag("v", "version", "print the version and exit"); - - let matches = match opts.parse(&args[1..]) { - Ok(m) => m, - Err(f) => crash!(1, "Invalid options\n{}", f), + let files: Vec = match matches.values_of(options::FILE) { + Some(v) => v.clone().map(|v| v.to_owned()).collect(), + None => vec!["-".to_owned()], }; - if matches.opt_present("help") { - let msg = format!( - "{0} {1} - -Usage: - {0} [OPTION]... [FILE]... - -Checksum and count the blocks in a file.", - NAME, VERSION - ); - println!( - "{}\nWith no FILE, or when FILE is -, read standard input.", - opts.usage(&msg) - ); - return 0; - } - if matches.opt_present("version") { - println!("{} {}", NAME, VERSION); - return 0; - } - - let sysv = matches.opt_present("sysv"); - - let files = if matches.free.is_empty() { - vec!["-".to_owned()] - } else { - matches.free - }; + let sysv = matches.is_present(options::SYSTEM_V_COMPATIBLE); let print_names = if sysv { files.len() > 1 || files[0] != "-" @@ -120,10 +128,15 @@ Checksum and count the blocks in a file.", files.len() > 1 }; + let mut exit_code = 0; for file in &files { let reader = match open(file) { Ok(f) => f, - _ => crash!(1, "unable to open file"), + Err(error) => { + show_error!("'{}' {}", file, error); + exit_code = 2; + continue; + } }; let (blocks, sum) = if sysv { sysv_sum(reader) @@ -138,5 +151,5 @@ Checksum and count the blocks in a file.", } } - 0 + exit_code } diff --git a/tests/by-util/test_sum.rs b/tests/by-util/test_sum.rs index 83cf689ac..d12455749 100644 --- a/tests/by-util/test_sum.rs +++ b/tests/by-util/test_sum.rs @@ -52,3 +52,23 @@ fn test_sysv_stdin() { .succeeds() .stdout_only_fixture("sysv_stdin.expected"); } + +#[test] +fn test_invalid_file() { + let (at, mut ucmd) = at_and_ucmd!(); + + at.mkdir("a"); + + ucmd.arg("a") + .fails() + .stderr_is("sum: error: 'a' Is a directory"); +} + +#[test] +fn test_invalid_metadata() { + let (_, mut ucmd) = at_and_ucmd!(); + + ucmd.arg("b") + .fails() + .stderr_is("sum: error: 'b' No such file or directory"); +}