Merge pull request #6105 from cre4ture/fix/findings_for_env_string_args

Fix/findings for env string args

src/uu/env/src/env.rs

@@ -292,11 +292,12 @@ impl EnvAppData {
         Ok(all_args)
     }
 
-    #[allow(clippy::cognitive_complexity)]
+    fn parse_arguments(
-    fn run_env(&mut self, original_args: impl uucore::Args) -> UResult<()> {
+        &mut self,
+        original_args: impl uucore::Args,
+    ) -> Result<(Vec<OsString>, clap::ArgMatches), Box<dyn UError>> {
         let original_args: Vec<OsString> = original_args.collect();
         let args = self.process_all_string_arguments(&original_args)?;
-
         let app = uu_app();
         let matches = app
             .try_get_matches_from(args)
@@ -316,6 +317,11 @@ impl EnvAppData {
                     }
                 }
             })?;
+        Ok((original_args, matches))
+    }
+
+    fn run_env(&mut self, original_args: impl uucore::Args) -> UResult<()> {
+        let (original_args, matches) = self.parse_arguments(original_args)?;
 
         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");
@@ -323,6 +329,109 @@ impl EnvAppData {
             debug_print_args(&original_args);
         }
 
+        let mut opts = make_options(&matches)?;
+
+        apply_change_directory(&opts)?;
+
+        // 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
+
+        apply_removal_of_all_env_vars(&opts);
+
+        // load .env-style config file prior to those given on the command-line
+        load_config_file(&mut opts)?;
+
+        apply_unset_env_vars(&opts)?;
+
+        apply_specified_env_vars(&opts);
+
+        if opts.program.is_empty() {
+            // no program provided, so just dump all env vars to stdout
+            print_env(opts.line_ending);
+        } else {
+            return self.run_program(opts, do_debug_printing);
+        }
+
+        Ok(())
+    }
+
+    fn run_program(
+        &mut self,
+        opts: Options<'_>,
+        do_debug_printing: bool,
+    ) -> Result<(), Box<dyn UError>> {
+        let prog = Cow::from(opts.program[0]);
+        let args = &opts.program[1..];
+        if do_debug_printing {
+            eprintln!("executable: {}", prog.quote());
+            for (i, arg) in args.iter().enumerate() {
+                eprintln!("arg[{}]: {}", i, arg.quote());
+            }
+        }
+        // we need to execute a command
+
+        /*
+         * On Unix-like systems Command::status either ends up calling either fork or posix_spawnp
+         * (which ends up calling clone). Keep using the current process would be ideal, but the
+         * standard library contains many checks and fail-safes to ensure the process ends up being
+         * created. This is much simpler than dealing with the hassles of calling execvp directly.
+         */
+        match process::Command::new(&*prog).args(args).status() {
+            Ok(exit) if !exit.success() => {
+                #[cfg(unix)]
+                if let Some(exit_code) = exit.code() {
+                    return Err(exit_code.into());
+                } else {
+                    // `exit.code()` returns `None` on Unix when the process is terminated by a signal.
+                    // See std::os::unix::process::ExitStatusExt for more information. This prints out
+                    // the interrupted process and the signal it received.
+                    let signal_code = exit.signal().unwrap();
+                    let signal = Signal::try_from(signal_code).unwrap();
+
+                    // We have to disable any handler that's installed by default.
+                    // This ensures that we exit on this signal.
+                    // For example, `SIGSEGV` and `SIGBUS` have default handlers installed in Rust.
+                    // We ignore the errors because there is not much we can do if that fails anyway.
+                    // SAFETY: The function is unsafe because installing functions is unsafe, but we are
+                    // just defaulting to default behavior and not installing a function. Hence, the call
+                    // is safe.
+                    let _ = unsafe {
+                        sigaction(
+                            signal,
+                            &SigAction::new(SigHandler::SigDfl, SaFlags::empty(), SigSet::all()),
+                        )
+                    };
+
+                    let _ = raise(signal);
+                }
+                return Err(exit.code().unwrap().into());
+            }
+            Err(ref err)
+                if (err.kind() == io::ErrorKind::NotFound)
+                    || (err.kind() == io::ErrorKind::InvalidInput) =>
+            {
+                return Err(self.make_error_no_such_file_or_dir(prog.deref()));
+            }
+            Err(e) => {
+                uucore::show_error!("unknown error: {:?}", e);
+                return Err(126.into());
+            }
+            Ok(_) => (),
+        }
+        Ok(())
+    }
+}
+
+fn apply_removal_of_all_env_vars(opts: &Options<'_>) {
+    // remove all env vars if told to ignore presets
+    if opts.ignore_env {
+        for (ref name, _) in env::vars_os() {
+            env::remove_var(name);
+        }
+    }
+}
+
+fn make_options(matches: &clap::ArgMatches) -> UResult<Options<'_>> {
     let ignore_env = matches.get_flag("ignore-environment");
     let line_ending = LineEnding::from_zero_flag(matches.get_flag("null"));
     let running_directory = matches.get_one::<OsString>("chdir").map(|s| s.as_os_str());
@@ -345,19 +454,6 @@ impl EnvAppData {
         program: vec![],
     };
 
-        // change directory
-        if let Some(d) = opts.running_directory {
-            match env::set_current_dir(d) {
-                Ok(()) => d,
-                Err(error) => {
-                    return Err(USimpleError::new(
-                        125,
-                        format!("cannot change directory to {}: {error}", d.quote()),
-                    ));
-                }
-            };
-        }
-
     let mut begin_prog_opts = false;
     if let Some(mut iter) = matches.get_many::<OsString>("vars") {
         // read NAME=VALUE arguments (and up to a single program argument)
@@ -379,28 +475,10 @@ impl EnvAppData {
         }
     }
 
-        // GNU env tests this behavior
+    Ok(opts)
-        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
+fn apply_unset_env_vars(opts: &Options<'_>) -> Result<(), Box<dyn UError>> {
-        //       easily handle the case where no command is given
-
-        // remove all env vars if told to ignore presets
-        if opts.ignore_env {
-            for (ref name, _) in env::vars_os() {
-                env::remove_var(name);
-            }
-        }
-
-        // load .env-style config file prior to those given on the command-line
-        load_config_file(&mut opts)?;
-
-        // unset specified env vars
     for name in &opts.unsets {
         let native_name = NativeStr::new(name);
         if name.is_empty()
@@ -415,7 +493,33 @@ impl EnvAppData {
 
         env::remove_var(name);
     }
+    Ok(())
+}
 
+fn apply_change_directory(opts: &Options<'_>) -> Result<(), Box<dyn UError>> {
+    // GNU env tests this behavior
+    if opts.program.is_empty() && opts.running_directory.is_some() {
+        return Err(UUsageError::new(
+            125,
+            "must specify command with --chdir (-C)".to_string(),
+        ));
+    }
+
+    if let Some(d) = opts.running_directory {
+        match env::set_current_dir(d) {
+            Ok(()) => d,
+            Err(error) => {
+                return Err(USimpleError::new(
+                    125,
+                    format!("cannot change directory to {}: {error}", d.quote()),
+                ));
+            }
+        };
+    }
+    Ok(())
+}
+
+fn apply_specified_env_vars(opts: &Options<'_>) {
     // set specified env vars
     for (name, val) in &opts.sets {
         /*
@@ -446,79 +550,6 @@ impl EnvAppData {
         }
         env::set_var(name, val);
     }
-
-        if opts.program.is_empty() {
-            // no program provided, so just dump all env vars to stdout
-            print_env(opts.line_ending);
-        } else {
-            // we need to execute a command
-            let prog = Cow::from(opts.program[0]);
-            let args = &opts.program[1..];
-
-            if do_debug_printing {
-                eprintln!("executable: {}", prog.quote());
-                for (i, arg) in args.iter().enumerate() {
-                    eprintln!("arg[{}]: {}", i, arg.quote());
-                }
-            }
-
-            /*
-             * On Unix-like systems Command::status either ends up calling either fork or posix_spawnp
-             * (which ends up calling clone). Keep using the current process would be ideal, but the
-             * standard library contains many checks and fail-safes to ensure the process ends up being
-             * created. This is much simpler than dealing with the hassles of calling execvp directly.
-             */
-            match process::Command::new(&*prog).args(args).status() {
-                Ok(exit) if !exit.success() => {
-                    #[cfg(unix)]
-                    if let Some(exit_code) = exit.code() {
-                        return Err(exit_code.into());
-                    } else {
-                        // `exit.code()` returns `None` on Unix when the process is terminated by a signal.
-                        // See std::os::unix::process::ExitStatusExt for more information. This prints out
-                        // the interrupted process and the signal it received.
-                        let signal_code = exit.signal().unwrap();
-                        let signal = Signal::try_from(signal_code).unwrap();
-
-                        // We have to disable any handler that's installed by default.
-                        // This ensures that we exit on this signal.
-                        // For example, `SIGSEGV` and `SIGBUS` have default handlers installed in Rust.
-                        // We ignore the errors because there is not much we can do if that fails anyway.
-                        // SAFETY: The function is unsafe because installing functions is unsafe, but we are
-                        // just defaulting to default behavior and not installing a function. Hence, the call
-                        // is safe.
-                        let _ = unsafe {
-                            sigaction(
-                                signal,
-                                &SigAction::new(
-                                    SigHandler::SigDfl,
-                                    SaFlags::empty(),
-                                    SigSet::all(),
-                                ),
-                            )
-                        };
-
-                        let _ = raise(signal);
-                    }
-                    #[cfg(not(unix))]
-                    return Err(exit.code().unwrap().into());
-                }
-                Err(ref err)
-                    if (err.kind() == io::ErrorKind::NotFound)
-                        || (err.kind() == io::ErrorKind::InvalidInput) =>
-                {
-                    return Err(self.make_error_no_such_file_or_dir(prog.deref()));
-                }
-                Err(e) => {
-                    uucore::show_error!("unknown error: {:?}", e);
-                    return Err(126.into());
-                }
-                Ok(_) => (),
-            }
-        }
-
-        Ok(())
-    }
 }
 
 #[uucore::main]