Revert "Fixes #46775 -- don't mutate the process's environment in Command::exec"

This reverts commit 36fe3b6.

Author: alexcrichton

src/libstd/sys/unix/process/process_common.rs

@@ -141,10 +141,6 @@ impl Command {
     pub fn get_argv(&self) -> &Vec<*const c_char> {
         &self.argv.0
     }
-    #[cfg(not(target_os = "fuchsia"))]
-    pub fn get_program(&self) -> &CString {
-        return &self.program;
-    }
 
     #[allow(dead_code)]
     pub fn get_cwd(&self) -> &Option<CString> {
@@ -248,10 +244,6 @@ impl CStringArray {
     pub fn as_ptr(&self) -> *const *const c_char {
         self.ptrs.as_ptr()
     }
-    #[cfg(not(target_os = "fuchsia"))]
-    pub fn get_items(&self) -> &[CString] {
-        return &self.items;
-    }
 }
 
 fn construct_envp(env: BTreeMap<DefaultEnvKey, OsString>, saw_nul: &mut bool) -> CStringArray {

src/libstd/sys/unix/process/process_unix.rs

@@ -8,8 +8,6 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-use env;
-use ffi::CString;
 use io::{self, Error, ErrorKind};
 use libc::{self, c_int, gid_t, pid_t, uid_t};
 use ptr;
@@ -41,15 +39,13 @@ impl Command {
             return Ok((ret, ours))
         }
 
-        let possible_paths = self.compute_possible_paths(envp.as_ref());
-
         let (input, output) = sys::pipe::anon_pipe()?;
 
         let pid = unsafe {
             match cvt(libc::fork())? {
                 0 => {
                     drop(input);
-                    let err = self.do_exec(theirs, envp.as_ref(), possible_paths);
+                    let err = self.do_exec(theirs, envp.as_ref());
                     let errno = err.raw_os_error().unwrap_or(libc::EINVAL) as u32;
                     let bytes = [
                         (errno >> 24) as u8,
@@ -117,48 +113,12 @@ impl Command {
                                   "nul byte found in provided data")
         }
 
-        let possible_paths = self.compute_possible_paths(envp.as_ref());
         match self.setup_io(default, true) {
-            Ok((_, theirs)) => unsafe { self.do_exec(theirs, envp.as_ref(), possible_paths) },
+            Ok((_, theirs)) => unsafe { self.do_exec(theirs, envp.as_ref()) },
             Err(e) => e,
         }
     }
 
-    fn compute_possible_paths(&self, maybe_envp: Option<&CStringArray>) -> Option<Vec<CString>> {
-        let program = self.get_program().as_bytes();
-        if program.contains(&b'/') {
-            return None;
-        }
-        // Outside the match so we can borrow it for the lifetime of the function.
-        let parent_path = env::var("PATH").ok();
-        let paths = match maybe_envp {
-            Some(envp) => {
-                match envp.get_items().iter().find(|var| var.as_bytes().starts_with(b"PATH=")) {
-                    Some(p) => &p.as_bytes()[5..],
-                    None => return None,
-                }
-            },
-            // maybe_envp is None if the process isn't changing the parent's env at all.
-            None => {
-                match parent_path.as_ref() {
-                    Some(p) => p.as_bytes(),
-                    None => return None,
-                }
-            },
-        };
-
-        let mut possible_paths = vec![];
-        for path in paths.split(|p| *p == b':') {
-            let mut binary_path = Vec::with_capacity(program.len() + path.len() + 1);
-            binary_path.extend_from_slice(path);
-            binary_path.push(b'/');
-            binary_path.extend_from_slice(program);
-            let c_binary_path = CString::new(binary_path).unwrap();
-            possible_paths.push(c_binary_path);
-        }
-        return Some(possible_paths);
-    }
-
     // And at this point we've reached a special time in the life of the
     // child. The child must now be considered hamstrung and unable to
     // do anything other than syscalls really. Consider the following
@@ -192,8 +152,7 @@ impl Command {
     unsafe fn do_exec(
         &mut self,
         stdio: ChildPipes,
-        maybe_envp: Option<&CStringArray>,
-        maybe_possible_paths: Option<Vec<CString>>,
+        maybe_envp: Option<&CStringArray>
     ) -> io::Error {
         use sys::{self, cvt_r};
 
@@ -234,6 +193,9 @@ impl Command {
         if let Some(ref cwd) = *self.get_cwd() {
             t!(cvt(libc::chdir(cwd.as_ptr())));
         }
+        if let Some(envp) = maybe_envp {
+            *sys::os::environ() = envp.as_ptr();
+        }
 
         // emscripten has no signal support.
         #[cfg(not(any(target_os = "emscripten")))]
@@ -269,53 +231,8 @@ impl Command {
             t!(callback());
         }
 
-        // If the program isn't an absolute path, and our environment contains a PATH var, then we
-        // implement the PATH traversal ourselves so that it honors the child's PATH instead of the
-        // parent's. This mirrors the logic that exists in glibc's execvpe, except using the
-        // child's env to fetch PATH.
-        match maybe_possible_paths {
-            Some(possible_paths) => {
-                let mut pending_error = None;
-                for path in possible_paths {
-                    libc::execve(
-                        path.as_ptr(),
-                        self.get_argv().as_ptr(),
-                        maybe_envp.map(|envp| envp.as_ptr()).unwrap_or_else(|| *sys::os::environ())
-                    );
-                    let err = io::Error::last_os_error();
-                    match err.kind() {
-                        io::ErrorKind::PermissionDenied => {
-                            // If we saw a PermissionDenied, and none of the other entries in
-                            // $PATH are successful, then we'll return the first EACCESS we see.
-                            if pending_error.is_none() {
-                                pending_error = Some(err);
-                            }
-                        },
-                        // Errors which indicate we failed to find a file are ignored and we try
-                        // the next entry in the path.
-                        io::ErrorKind::NotFound | io::ErrorKind::TimedOut => {
-                            continue
-                        },
-                        // Any other error means we found a file and couldn't execute it.
-                        _ => {
-                            return err;
-                        }
-                    }
-                }
-                if let Some(err) = pending_error {
-                    return err;
-                }
-                return io::Error::from_raw_os_error(libc::ENOENT);
-            },
-            _ => {
-                libc::execve(
-                    self.get_argv()[0],
-                    self.get_argv().as_ptr(),
-                    maybe_envp.map(|envp| envp.as_ptr()).unwrap_or_else(|| *sys::os::environ())
-                );
-                return io::Error::last_os_error()
-            }
-        }
+        libc::execvp(self.get_argv()[0], self.get_argv().as_ptr());
+        io::Error::last_os_error()
     }
 
     #[cfg(not(any(target_os = "macos", target_os = "freebsd",

src/test/run-pass/command-exec.rs

@@ -48,13 +48,6 @@ fn main() {
                 println!("passed");
             }
 
-            "exec-test5" => {
-                env::set_var("VARIABLE", "ABC");
-                Command::new("definitely-not-a-real-binary").env("VARIABLE", "XYZ").exec();
-                assert_eq!(env::var("VARIABLE").unwrap(), "ABC");
-                println!("passed");
-            }
-
             _ => panic!("unknown argument: {}", arg),
         }
         return
@@ -79,9 +72,4 @@ fn main() {
     assert!(output.status.success());
     assert!(output.stderr.is_empty());
     assert_eq!(output.stdout, b"passed\n");
-
-    let output = Command::new(&me).arg("exec-test5").output().unwrap();
-    assert!(output.status.success());
-    assert!(output.stderr.is_empty());
-    assert_eq!(output.stdout, b"passed\n");
 }