From 3a13d3e43f3810fa5cd3c00fe77e05a602989cac Mon Sep 17 00:00:00 2001 From: Carbrex <95964955+Carbrex@users.noreply.github.com> Date: Mon, 1 Apr 2024 18:10:14 +0530 Subject: [PATCH 01/10] Add test for ignoring time-style if time not defined --- tests/by-util/test_env.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/by-util/test_env.rs b/tests/by-util/test_env.rs index 13535e4161f..4367ff458d9 100644 --- a/tests/by-util/test_env.rs +++ b/tests/by-util/test_env.rs @@ -7,7 +7,7 @@ #[cfg(target_os = "linux")] use crate::common::util::expected_result; use crate::common::util::TestScenario; -use ::env::native_int_str::{Convert, NCvt}; +use env::native_int_str::{Convert, NCvt}; use std::env; use std::path::Path; use tempfile::tempdir; @@ -376,8 +376,8 @@ fn test_gnu_e20() { #[test] fn test_split_string_misc() { - use ::env::native_int_str::NCvt; - use ::env::parse_args_from_str; + use env::native_int_str::NCvt; + use env::parse_args_from_str; assert_eq!( NCvt::convert(vec!["A=B", "FOO=AR", "sh", "-c", "echo $A$FOO"]), @@ -687,8 +687,8 @@ mod tests_split_iterator { use std::ffi::OsString; - use ::env::parse_error::ParseError; use env::native_int_str::{from_native_int_representation_owned, Convert, NCvt}; + use env::parse_error::ParseError; fn split(input: &str) -> Result, ParseError> { ::env::split_iterator::split(&NCvt::convert(input)).map(|vec| { From 4e5b7640e9162db60e91e329399a93633c84f642 Mon Sep 17 00:00:00 2001 From: Carbrex <95964955+Carbrex@users.noreply.github.com> Date: Mon, 1 Apr 2024 21:20:30 +0530 Subject: [PATCH 02/10] Throw error if unset argument starts with '=' for env command Fix clippy error Fix formatting Simplify unset prefix handling Also include '=' in invalid_arg cargo fmt --- src/uu/env/src/env.rs | 17 +++++++++++++++++ tests/by-util/test_env.rs | 31 +++++++++++++++++++++++++++---- 2 files changed, 44 insertions(+), 4 deletions(-) diff --git a/src/uu/env/src/env.rs b/src/uu/env/src/env.rs index 4f2790dc8b4..0eb72b3358d 100644 --- a/src/uu/env/src/env.rs +++ b/src/uu/env/src/env.rs @@ -323,6 +323,23 @@ impl EnvAppData { fn run_env(&mut self, original_args: impl uucore::Args) -> UResult<()> { let (original_args, matches) = self.parse_arguments(original_args)?; + // clap automatically removes '=' from the beginning of the argument, so we need to check for it + for arg in &original_args { + let arg = arg.to_string_lossy(); + let prefix = if arg.starts_with("-u=") { + "-u=" + } else { + "--unset=" + }; + if arg.starts_with(prefix) { + let invalid_arg = &arg[(prefix.len() - 1)..]; + return Err(UUsageError::new( + 125, + format!("cannot unset '{}': Invalid argument", invalid_arg), + )); + } + } + let did_debug_printing_before = self.do_debug_printing; // could have been done already as part of the "-vS" string parsing let do_debug_printing = self.do_debug_printing || matches.get_flag("debug"); if do_debug_printing && !did_debug_printing_before { diff --git a/tests/by-util/test_env.rs b/tests/by-util/test_env.rs index 4367ff458d9..9d8d51c0497 100644 --- a/tests/by-util/test_env.rs +++ b/tests/by-util/test_env.rs @@ -7,7 +7,8 @@ #[cfg(target_os = "linux")] use crate::common::util::expected_result; use crate::common::util::TestScenario; -use env::native_int_str::{Convert, NCvt}; +#[rustfmt::skip] +use ::env::native_int_str::{Convert, NCvt}; use std::env; use std::path::Path; use tempfile::tempdir; @@ -376,8 +377,10 @@ fn test_gnu_e20() { #[test] fn test_split_string_misc() { - use env::native_int_str::NCvt; - use env::parse_args_from_str; + #[rustfmt::skip] + use ::env::native_int_str::NCvt; + #[rustfmt::skip] + use ::env::parse_args_from_str; assert_eq!( NCvt::convert(vec!["A=B", "FOO=AR", "sh", "-c", "echo $A$FOO"]), @@ -564,6 +567,25 @@ fn test_env_with_gnu_reference_empty_executable_double_quotes() { .stderr_is("env: '': No such file or directory\n"); } +#[test] +fn test_env_with_gnu_reference_unset_invalid_variables() { + let ts = TestScenario::new(util_name!()); + + ts.ucmd() + .args(&["-u=2EKt"]) // invalid variable name + .fails() + .code_is(125) + .no_stdout() + .stderr_contains("env: cannot unset '=2EKt': Invalid argument\n"); + + ts.ucmd() + .args(&["-0", "-u=o", "6=C"]) // invalid variable name + .fails() + .code_is(125) + .no_stdout() + .stderr_contains("env: cannot unset '=o': Invalid argument\n"); +} + #[cfg(test)] mod tests_split_iterator { @@ -687,8 +709,9 @@ mod tests_split_iterator { use std::ffi::OsString; + #[rustfmt::skip] + use ::env::parse_error::ParseError; use env::native_int_str::{from_native_int_representation_owned, Convert, NCvt}; - use env::parse_error::ParseError; fn split(input: &str) -> Result, ParseError> { ::env::split_iterator::split(&NCvt::convert(input)).map(|vec| { From 8b863f3b0c13ebf8dd21086f1458ee40e1cb095d Mon Sep 17 00:00:00 2001 From: Carbrex <95964955+Carbrex@users.noreply.github.com> Date: Tue, 2 Apr 2024 00:31:39 +0530 Subject: [PATCH 03/10] Remove rust fmt skips --- tests/by-util/test_env.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tests/by-util/test_env.rs b/tests/by-util/test_env.rs index 9d8d51c0497..872b3cc0ae7 100644 --- a/tests/by-util/test_env.rs +++ b/tests/by-util/test_env.rs @@ -7,7 +7,6 @@ #[cfg(target_os = "linux")] use crate::common::util::expected_result; use crate::common::util::TestScenario; -#[rustfmt::skip] use ::env::native_int_str::{Convert, NCvt}; use std::env; use std::path::Path; @@ -377,9 +376,7 @@ fn test_gnu_e20() { #[test] fn test_split_string_misc() { - #[rustfmt::skip] use ::env::native_int_str::NCvt; - #[rustfmt::skip] use ::env::parse_args_from_str; assert_eq!( @@ -709,7 +706,6 @@ mod tests_split_iterator { use std::ffi::OsString; - #[rustfmt::skip] use ::env::parse_error::ParseError; use env::native_int_str::{from_native_int_representation_owned, Convert, NCvt}; From 75548bee2dc4a2ef8ee36935697da58558c6031e Mon Sep 17 00:00:00 2001 From: Carbrex <95964955+Carbrex@users.noreply.github.com> Date: Tue, 2 Apr 2024 16:27:29 +0530 Subject: [PATCH 04/10] Allow = with --unset but not with -u --- src/uu/env/src/env.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/uu/env/src/env.rs b/src/uu/env/src/env.rs index 0eb72b3358d..dec2b9b91c4 100644 --- a/src/uu/env/src/env.rs +++ b/src/uu/env/src/env.rs @@ -326,11 +326,7 @@ impl EnvAppData { // clap automatically removes '=' from the beginning of the argument, so we need to check for it for arg in &original_args { let arg = arg.to_string_lossy(); - let prefix = if arg.starts_with("-u=") { - "-u=" - } else { - "--unset=" - }; + let prefix = "-u="; if arg.starts_with(prefix) { let invalid_arg = &arg[(prefix.len() - 1)..]; return Err(UUsageError::new( From 0f05b6bd88c5fa14760f71b638e23972797abce8 Mon Sep 17 00:00:00 2001 From: Carbrex <95964955+Carbrex@users.noreply.github.com> Date: Wed, 3 Apr 2024 18:10:01 +0530 Subject: [PATCH 05/10] useless --- src/uu/env/src/env.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/uu/env/src/env.rs b/src/uu/env/src/env.rs index dec2b9b91c4..481fa26b5e6 100644 --- a/src/uu/env/src/env.rs +++ b/src/uu/env/src/env.rs @@ -298,6 +298,7 @@ impl EnvAppData { ) -> Result<(Vec, clap::ArgMatches), Box> { let original_args: Vec = original_args.collect(); let args = self.process_all_string_arguments(&original_args)?; + println!("args: {:?}", args); let app = uu_app(); let matches = app .try_get_matches_from(args) @@ -322,6 +323,7 @@ impl EnvAppData { fn run_env(&mut self, original_args: impl uucore::Args) -> UResult<()> { let (original_args, matches) = self.parse_arguments(original_args)?; + println!("matches: {:?}", matches); // clap automatically removes '=' from the beginning of the argument, so we need to check for it for arg in &original_args { From 59b5c64733a75393bd78d7a4613f2e1d2e0c8ef4 Mon Sep 17 00:00:00 2001 From: Carbrex <95964955+Carbrex@users.noreply.github.com> Date: Wed, 5 Jun 2024 14:10:33 +0530 Subject: [PATCH 06/10] Add more tests --- tests/by-util/test_env.rs | 47 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/tests/by-util/test_env.rs b/tests/by-util/test_env.rs index 3ea5618e552..df1a8bd054f 100644 --- a/tests/by-util/test_env.rs +++ b/tests/by-util/test_env.rs @@ -666,6 +666,53 @@ fn test_env_with_gnu_reference_unset_invalid_variables() { .code_is(125) .no_stdout() .stderr_contains("env: cannot unset '=o': Invalid argument\n"); + + // env -i -0 -u=kQ4dALb1 A=0d CZj=lYr + // Output: A=0dCZj=lYr + ts.ucmd() + .args(&["-i", "-0", "-u=kQ4dALb1", "A=0d", "CZj=lYr"]) + .succeeds() + .stdout_is("A=0dCZj=lYr\n") + .stderr_is(""); + + // env -- -u=kQ4dALb1 A=0d CZj=lYr + // prints all environment variables + ts.ucmd() + .args(&["--", "-u=kQ4dALb1", "A=0d", "CZj=lYr"]) + .succeeds() + .stdout_contains("A=0d") + .stdout_contains("CZj=lYr") + .stderr_is(""); + + // env D=ddsa - A=0d CZj=lYr + // env: ‘-’: No such file or directory + ts.ucmd() + .args(&["D=ddsa", "-", "A=0d", "CZj=lYr"]) + .fails() + .code_is(127) + .no_stdout() + .stderr_contains("env: '-': No such file or directory\n"); + + // env -u=kQ4dALb1 - A=0d CZj=lYr + // Output: + // A=0d + // CZj=lYr + ts.ucmd() + .args(&["-u=kQ4dALb1", "-", "A=0d", "CZj=lYr"]) + .succeeds() + .stdout_contains("A=0d") + .stdout_contains("CZj=lYr") + .stderr_is(""); + + // env -i -u=kQ4dALb1 A=0d CZj=lYr + // Output: A=0d + // CZj=lYr + ts.ucmd() + .args(&["-i", "-u=kQ4dALb1", "A=0d", "CZj=lYr"]) + .succeeds() + .stdout_contains("A=0d") + .stdout_contains("CZj=lYr") + .stderr_is(""); } #[test] From ccb0162c9607e5b35dc1d81ee1af22d50dda2042 Mon Sep 17 00:00:00 2001 From: Carbrex <95964955+Carbrex@users.noreply.github.com> Date: Wed, 5 Jun 2024 14:12:42 +0530 Subject: [PATCH 07/10] Remove not working env -u code --- src/uu/env/src/env.rs | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/src/uu/env/src/env.rs b/src/uu/env/src/env.rs index f7f9fa15fa5..229488cfb51 100644 --- a/src/uu/env/src/env.rs +++ b/src/uu/env/src/env.rs @@ -384,7 +384,6 @@ impl EnvAppData { ) -> Result<(Vec, clap::ArgMatches), Box> { let original_args: Vec = original_args.collect(); let args = self.process_all_string_arguments(&original_args)?; - println!("args: {:?}", args); let app = uu_app(); let matches = app .try_get_matches_from(args) @@ -409,20 +408,6 @@ impl EnvAppData { fn run_env(&mut self, original_args: impl uucore::Args) -> UResult<()> { let (original_args, matches) = self.parse_arguments(original_args)?; - println!("matches: {:?}", matches); - - // clap automatically removes '=' from the beginning of the argument, so we need to check for it - for arg in &original_args { - let arg = arg.to_string_lossy(); - let prefix = "-u="; - if arg.starts_with(prefix) { - let invalid_arg = &arg[(prefix.len() - 1)..]; - return Err(UUsageError::new( - 125, - format!("cannot unset '{}': Invalid argument", invalid_arg), - )); - } - } self.do_debug_printing = self.do_debug_printing || (0 != matches.get_count("debug")); self.do_input_debug_printing = self From 9e0b4cdf0c0de5348ef7666ff80224f8ec7ce534 Mon Sep 17 00:00:00 2001 From: Carbrex <95964955+Carbrex@users.noreply.github.com> Date: Wed, 5 Jun 2024 14:16:14 +0530 Subject: [PATCH 08/10] Add unset tests with --unset --- tests/by-util/test_env.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/by-util/test_env.rs b/tests/by-util/test_env.rs index df1a8bd054f..03fe06a6f36 100644 --- a/tests/by-util/test_env.rs +++ b/tests/by-util/test_env.rs @@ -667,6 +667,18 @@ fn test_env_with_gnu_reference_unset_invalid_variables() { .no_stdout() .stderr_contains("env: cannot unset '=o': Invalid argument\n"); + ts.ucmd() + .args(&["--unset=2EKt"]) // works with long option + .succeeds() + .stdout_contains("A=0d") + .stdout_contains("CZj=lYr"); + + ts.ucmd() + .args(&["-0", "--unset=o", "6=C"]) // works with long option + .succeeds() + .stdout_contains("A=0d") + .stdout_contains("CZj=lYr"); + // env -i -0 -u=kQ4dALb1 A=0d CZj=lYr // Output: A=0dCZj=lYr ts.ucmd() From da441b72bc230b89507c10d605513ed4219bda1a Mon Sep 17 00:00:00 2001 From: Carbrex <95964955+Carbrex@users.noreply.github.com> Date: Wed, 5 Jun 2024 14:20:22 +0530 Subject: [PATCH 09/10] Some more tests for env -u --- tests/by-util/test_env.rs | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/tests/by-util/test_env.rs b/tests/by-util/test_env.rs index 03fe06a6f36..bf47ff58878 100644 --- a/tests/by-util/test_env.rs +++ b/tests/by-util/test_env.rs @@ -680,14 +680,14 @@ fn test_env_with_gnu_reference_unset_invalid_variables() { .stdout_contains("CZj=lYr"); // env -i -0 -u=kQ4dALb1 A=0d CZj=lYr - // Output: A=0dCZj=lYr + // Output: A=0dCZj=lYr ts.ucmd() .args(&["-i", "-0", "-u=kQ4dALb1", "A=0d", "CZj=lYr"]) .succeeds() .stdout_is("A=0dCZj=lYr\n") .stderr_is(""); - // env -- -u=kQ4dALb1 A=0d CZj=lYr + // env -- -u=kQ4dALb1 A=0d CZj=lYr // prints all environment variables ts.ucmd() .args(&["--", "-u=kQ4dALb1", "A=0d", "CZj=lYr"]) @@ -696,7 +696,7 @@ fn test_env_with_gnu_reference_unset_invalid_variables() { .stdout_contains("CZj=lYr") .stderr_is(""); - // env D=ddsa - A=0d CZj=lYr + // env D=ddsa - A=0d CZj=lYr // env: ‘-’: No such file or directory ts.ucmd() .args(&["D=ddsa", "-", "A=0d", "CZj=lYr"]) @@ -705,8 +705,8 @@ fn test_env_with_gnu_reference_unset_invalid_variables() { .no_stdout() .stderr_contains("env: '-': No such file or directory\n"); - // env -u=kQ4dALb1 - A=0d CZj=lYr - // Output: + // env -u=kQ4dALb1 - A=0d CZj=lYr + // Output: // A=0d // CZj=lYr ts.ucmd() @@ -725,6 +725,24 @@ fn test_env_with_gnu_reference_unset_invalid_variables() { .stdout_contains("A=0d") .stdout_contains("CZj=lYr") .stderr_is(""); + + // env echo gotcha -u=hello + // Output: gotcha -u=hello + ts.ucmd() + .args(&["echo", "gotcha", "-u=hello"]) + .succeeds() + .stdout_contains("gotcha -u=hello") + .stderr_is(""); + + // env -vu=hello + // unset: =hello + // env: cannot unset ‘=hello’: Invalid argument + ts.ucmd() + .args(&["-vu=hello"]) + .fails() + .code_is(125) + .no_stdout() + .stderr_contains("env: cannot unset '=hello': Invalid argument\n"); } #[test] From 4d0e31f64502da16175705959d3401891bb995d9 Mon Sep 17 00:00:00 2001 From: Carbrex <95964955+Carbrex@users.noreply.github.com> Date: Tue, 11 Jun 2024 13:24:23 +0530 Subject: [PATCH 10/10] Ignore the unset var test for env and also fix a test with -0 --- tests/by-util/test_env.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/by-util/test_env.rs b/tests/by-util/test_env.rs index bf47ff58878..b79c23e43d6 100644 --- a/tests/by-util/test_env.rs +++ b/tests/by-util/test_env.rs @@ -649,6 +649,7 @@ fn test_env_with_empty_executable_double_quotes() { .stderr_is("env: '': No such file or directory\n"); } +#[ignore = "See #6172"] #[test] fn test_env_with_gnu_reference_unset_invalid_variables() { let ts = TestScenario::new(util_name!()); @@ -680,11 +681,11 @@ fn test_env_with_gnu_reference_unset_invalid_variables() { .stdout_contains("CZj=lYr"); // env -i -0 -u=kQ4dALb1 A=0d CZj=lYr - // Output: A=0dCZj=lYr + // Output: A=0d\0CZj=lYr\0 ts.ucmd() .args(&["-i", "-0", "-u=kQ4dALb1", "A=0d", "CZj=lYr"]) .succeeds() - .stdout_is("A=0dCZj=lYr\n") + .stdout_is("A=0d\0CZj=lYr\0") .stderr_is(""); // env -- -u=kQ4dALb1 A=0d CZj=lYr