diff --git a/src/uu/env/src/env.rs b/src/uu/env/src/env.rs index 8967a798a..346bf4c8e 100644 --- a/src/uu/env/src/env.rs +++ b/src/uu/env/src/env.rs @@ -1,6 +1,7 @@ // This file is part of the uutils coreutils package. // // (c) Jordi Boggiano +// (c) Thomas Queiroz // // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. @@ -23,7 +24,7 @@ use std::io::{self, Write}; use std::iter::Iterator; use std::process::Command; use uucore::display::Quotable; -use uucore::error::{UResult, USimpleError}; +use uucore::error::{UResult, USimpleError, UUsageError}; const USAGE: &str = "env [OPTION]... [-] [NAME=VALUE]... [COMMAND [ARG]...]"; const AFTER_HELP: &str = "\ @@ -50,7 +51,7 @@ fn print_env(null: bool) { } } -fn parse_name_value_opt<'a>(opts: &mut Options<'a>, opt: &'a str) -> Result { +fn parse_name_value_opt<'a>(opts: &mut Options<'a>, opt: &'a str) -> UResult { // is it a NAME=VALUE like opt ? if let Some(idx) = opt.find('=') { // yes, so push name, value pair @@ -64,17 +65,12 @@ fn parse_name_value_opt<'a>(opts: &mut Options<'a>, opt: &'a str) -> Result(opts: &mut Options<'a>, opt: &'a str) -> Result<(), i32> { +fn parse_program_opt<'a>(opts: &mut Options<'a>, opt: &'a str) -> UResult<()> { if opts.null { - eprintln!( - "{}: cannot specify --null (-0) with command", - uucore::util_name() - ); - eprintln!( - "Type \"{} --help\" for detailed information", - uucore::execution_phrase() - ); - Err(1) + Err(UUsageError::new( + 125, + "cannot specify --null (-0) with command".to_string(), + )) } else { opts.program.push(opt); Ok(()) @@ -93,10 +89,8 @@ fn load_config_file(opts: &mut Options) -> UResult<()> { Ini::load_from_file(file) }; - let conf = conf.map_err(|error| { - show_error!("{}: {}", file.maybe_quote(), error); - 1 - })?; + let conf = + conf.map_err(|e| USimpleError::new(1, format!("{}: {}", file.maybe_quote(), e)))?; for (_, prop) in &conf { // ignore all INI section lines (treat them as comments) @@ -138,7 +132,7 @@ pub fn uu_app() -> App<'static, 'static> { .long("ignore-environment") .help("start with an empty environment")) .arg(Arg::with_name("chdir") - .short("c") + .short("C") // GNU env compatibility .long("chdir") .takes_value(true) .number_of_values(1) @@ -236,6 +230,14 @@ fn run_env(args: impl uucore::Args) -> UResult<()> { } } + // GNU env tests this behavior + if opts.program.is_empty() && running_directory.is_some() { + return Err(UUsageError::new( + 125, + "must specify command with --chdir (-C)".to_string(), + )); + } + // NOTE: we manually set and unset the env vars below rather than using Command::env() to more // easily handle the case where no command is given @@ -251,6 +253,13 @@ fn run_env(args: impl uucore::Args) -> UResult<()> { // unset specified env vars for name in &opts.unsets { + if name.is_empty() || name.contains(0 as char) || name.contains('=') { + return Err(USimpleError::new( + 125, + format!("cannot unset {}: Invalid argument", name.quote()), + )); + } + env::remove_var(name); } diff --git a/src/uu/printenv/src/printenv.rs b/src/uu/printenv/src/printenv.rs index 5d32cfbcc..c3f996865 100644 --- a/src/uu/printenv/src/printenv.rs +++ b/src/uu/printenv/src/printenv.rs @@ -9,6 +9,7 @@ use clap::{crate_version, App, Arg}; use std::env; +use uucore::error::UResult; static ABOUT: &str = "Display the values of the specified environment VARIABLE(s), or (with no VARIABLE) display name and value pairs for them all."; @@ -20,7 +21,8 @@ fn usage() -> String { format!("{0} [VARIABLE]... [OPTION]...", uucore::execution_phrase()) } -pub fn uumain(args: impl uucore::Args) -> i32 { +#[uucore_procs::gen_uumain] +pub fn uumain(args: impl uucore::Args) -> UResult<()> { let usage = usage(); let matches = uu_app().usage(&usage[..]).get_matches_from(args); @@ -40,15 +42,23 @@ pub fn uumain(args: impl uucore::Args) -> i32 { for (env_var, value) in env::vars() { print!("{}={}{}", env_var, value, separator); } - return 0; + return Ok(()); } + let mut not_found = false; for env_var in variables { if let Ok(var) = env::var(env_var) { print!("{}{}", var, separator); + } else { + not_found = true; } } - 0 + + if not_found { + Err(1.into()) + } else { + Ok(()) + } } pub fn uu_app() -> App<'static, 'static> { diff --git a/tests/by-util/test_env.rs b/tests/by-util/test_env.rs index ecb215f8c..135ee72ef 100644 --- a/tests/by-util/test_env.rs +++ b/tests/by-util/test_env.rs @@ -1,7 +1,4 @@ -// spell-checker:ignore (words) bamf chdir - -#[cfg(not(windows))] -use std::fs; +// spell-checker:ignore (words) bamf chdir rlimit prlimit COMSPEC use crate::common::util::*; use std::env; @@ -80,6 +77,20 @@ fn test_combined_file_set_unset() { ); } +#[test] +fn test_unset_invalid_variables() { + use uucore::display::Quotable; + + // Cannot test input with \0 in it, since output will also contain \0. rlimit::prlimit fails + // with this error: Error { kind: InvalidInput, message: "nul byte found in provided data" } + for var in &["", "a=b"] { + new_ucmd!().arg("-u").arg(var).run().stderr_only(format!( + "env: cannot unset {}: Invalid argument", + var.quote() + )); + } +} + #[test] fn test_single_name_value_pair() { let out = new_ucmd!().arg("FOO=bar").run(); @@ -163,7 +174,7 @@ fn test_fail_null_with_program() { fn test_change_directory() { let scene = TestScenario::new(util_name!()); let temporary_directory = tempdir().unwrap(); - let temporary_path = fs::canonicalize(temporary_directory.path()).unwrap(); + let temporary_path = std::fs::canonicalize(temporary_directory.path()).unwrap(); assert_ne!(env::current_dir().unwrap(), temporary_path); // command to print out current working directory @@ -179,27 +190,36 @@ fn test_change_directory() { assert_eq!(out.trim(), temporary_path.as_os_str()) } -// no way to consistently get "current working directory", `cd` doesn't work @ CI -// instead, we test that the unique temporary directory appears somewhere in the printed variables #[cfg(windows)] #[test] fn test_change_directory() { let scene = TestScenario::new(util_name!()); let temporary_directory = tempdir().unwrap(); - let temporary_path = temporary_directory.path(); - assert_ne!(env::current_dir().unwrap(), temporary_path); + let temporary_path = temporary_directory.path(); + let temporary_path = temporary_path + .strip_prefix(r"\\?\") + .unwrap_or(temporary_path); + + let env_cd = env::current_dir().unwrap(); + let env_cd = env_cd.strip_prefix(r"\\?\").unwrap_or(&env_cd); + + assert_ne!(env_cd, temporary_path); + + // COMSPEC is a variable that contains the full path to cmd.exe + let cmd_path = env::var("COMSPEC").unwrap(); + + // command to print out current working directory + let pwd = [&*cmd_path, "/C", "cd"]; let out = scene .ucmd() .arg("--chdir") .arg(&temporary_path) + .args(&pwd) .succeeds() .stdout_move_str(); - - assert!(!out - .lines() - .any(|line| line.ends_with(temporary_path.file_name().unwrap().to_str().unwrap()))); + assert_eq!(out.trim(), temporary_path.as_os_str()) } #[test]