Skip to content

Add Command::resolve_in_parent_path #142035

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions library/std/src/process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1195,6 +1195,22 @@ impl Command {
pub fn get_current_dir(&self) -> Option<&Path> {
self.inner.get_current_dir()
}

/// Resolve the program given in [`new`](Self::new) using the parent's PATH.
///
/// Usually when a `env` is used to set `PATH` on a child process,
/// that new `PATH` will be used to discover the process.
/// With `resolve_in_parent_path` to `true`, the parent's `PATH` will be used instead.
///
/// # Platform-specific behavior
///
/// On Unix the process is executed after fork and after setting up the rest of the new environment.
/// So if `chroot`, `uid`, `gid` or `groups` are used then the way `PATH` is interpreted may be changed.
Comment on lines +1207 to +1208
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this was a point without agreement on the libs-api side. E.g. what should happen if chroot etc is set so the paths in PATH essentially point somewhere different. I decided to implement it the easy way for the sake of experimentation and hope that someone will bug me if they think we must do something different and explain their reasons.

#[unstable(feature = "command_resolve", issue = "none")]
pub fn resolve_in_parent_path(&mut self, use_parent: bool) -> &mut Self {
self.inner.resolve_in_parent_path(use_parent);
self
}
}

#[stable(feature = "rust1", since = "1.0.0")]
Expand Down
43 changes: 41 additions & 2 deletions library/std/src/process/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@ use super::{Command, Output, Stdio};
use crate::io::prelude::*;
use crate::io::{BorrowedBuf, ErrorKind};
use crate::mem::MaybeUninit;
use crate::str;
use crate::{fs, str};

fn known_command() -> Command {
if cfg!(windows) { Command::new("help") } else { Command::new("echo") }
if cfg!(windows) { Command::new("help.exe") } else { Command::new("echo") }
}

#[cfg(target_os = "android")]
Expand Down Expand Up @@ -603,3 +603,42 @@ fn terminate_exited_process() {
assert!(p.kill().is_ok());
assert!(p.kill().is_ok());
}

#[test]
fn resolve_in_parent_path() {
let tempdir = crate::test_helpers::tmpdir();
let mut cmd = if cfg!(windows) {
// On Windows std prepends the system paths when searching for executables
// but not if PATH is set on the command. Therefore adding a broken exe
// using PATH will cause spawn to fail if it's invoked.
let mut cmd = known_command();
fs::write(tempdir.join(cmd.get_program().to_str().unwrap()), "").unwrap();
cmd.env("PATH", tempdir.path());
cmd
} else {
let mut cmd = known_command();
cmd.env("PATH", "/a/b/c");
cmd
};

assert!(cmd.spawn().is_err());
cmd.resolve_in_parent_path(true);
cmd.spawn().unwrap();
cmd.resolve_in_parent_path(false);
assert!(cmd.spawn().is_err());

// If the platform is a unix and has posix_spawn then that will be used in the test above.
// So we also test the fork/exec path by setting a pre_exec function.
#[cfg(unix)]
{
use crate::os::unix::process::CommandExt;
unsafe {
cmd.pre_exec(|| Ok(()));
}
assert!(cmd.spawn().is_err());
cmd.resolve_in_parent_path(true);
assert!(cmd.status().unwrap().success());
cmd.resolve_in_parent_path(false);
assert!(cmd.spawn().is_err());
}
}
4 changes: 4 additions & 0 deletions library/std/src/sys/process/uefi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,10 @@ impl Command {
self.stderr = Some(stderr);
}

pub fn resolve_in_parent_path(&mut self, _use_parent: bool) -> &mut Self {
self
}

pub fn get_program(&self) -> &OsStr {
self.prog.as_ref()
}
Expand Down
9 changes: 9 additions & 0 deletions library/std/src/sys/process/unix/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ pub struct Command {
#[cfg(target_os = "linux")]
create_pidfd: bool,
pgroup: Option<pid_t>,
resolve_in_parent_path: bool,
}

// passed back to std::process with the pipes connected to the child, if any
Expand Down Expand Up @@ -185,6 +186,7 @@ impl Command {
#[cfg(target_os = "linux")]
create_pidfd: false,
pgroup: None,
resolve_in_parent_path: false,
}
}

Expand Down Expand Up @@ -220,6 +222,9 @@ impl Command {
self.cwd(&OsStr::new("/"));
}
}
pub fn resolve_in_parent_path(&mut self, use_parent: bool) {
self.resolve_in_parent_path = use_parent;
}

#[cfg(target_os = "linux")]
pub fn create_pidfd(&mut self, val: bool) {
Expand Down Expand Up @@ -298,6 +303,10 @@ impl Command {
pub fn get_chroot(&self) -> Option<&CStr> {
self.chroot.as_deref()
}
#[allow(dead_code)]
pub fn get_resolve_in_parent_path(&self) -> bool {
self.resolve_in_parent_path
}

pub fn get_closures(&mut self) -> &mut Vec<Box<dyn FnMut() -> io::Result<()> + Send + Sync>> {
&mut self.closures
Expand Down
84 changes: 74 additions & 10 deletions library/std/src/sys/process/unix/unix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -378,27 +378,89 @@ impl Command {
callback()?;
}

let mut _reset = None;
// Although we're performing an exec here we may also return with an
// error from this function (without actually exec'ing) in which case we
// want to be sure to restore the global environment back to what it
// once was, ensuring that our temporary override, when free'd, doesn't
// corrupt our process's environment.
let mut _reset = None;
if let Some(envp) = maybe_envp {
struct Reset(*const *const libc::c_char);
struct Reset(*const *const libc::c_char);

impl Drop for Reset {
fn drop(&mut self) {
unsafe {
*sys::env::environ() = self.0;
}
impl Drop for Reset {
fn drop(&mut self) {
unsafe {
*sys::env::environ() = self.0;
}
}
}

if let Some(envp) = maybe_envp
&& self.get_resolve_in_parent_path()
&& self.env_saw_path()
&& self.get_program_kind() == ProgramKind::PathLookup
{
use crate::ffi::CStr;
// execvpe is a gnu extension...
#[cfg(all(target_os = "linux", target_env = "gnu"))]
unsafe fn exec_with_env(
program: &CStr,
args: &CStringArray,
envp: &CStringArray,
) -> io::Error {
unsafe { libc::execvpe(program.as_ptr(), args.as_ptr(), envp.as_ptr()) };
io::Error::last_os_error()
}

// ...so if we're not gnu then use our own implementation.
#[cfg(not(all(target_os = "linux", target_env = "gnu")))]
unsafe fn exec_with_env(
program: &CStr,
args: &CStringArray,
envp: &CStringArray,
) -> io::Error {
unsafe {
let name = program.to_bytes();
let mut buffer =
[const { mem::MaybeUninit::<u8>::uninit() }; libc::PATH_MAX as usize];
let mut environ = *sys::env::environ();
// Search the environment for PATH and, if found,
// search the paths for the executable by trying to execve each candidate.
while !(*environ).is_null() {
let kv = CStr::from_ptr(*environ);
if let Some(value) = kv.to_bytes().strip_prefix(b"PATH=") {
for path in value.split(|&b| b == b':') {
if buffer.len() - 2 >= path.len().saturating_add(name.len()) {
let buf_ptr = buffer.as_mut_ptr().cast::<u8>();
let mut offset = 0;
if !path.is_empty() {
buf_ptr.copy_from(path.as_ptr(), path.len());
offset += path.len();
if path.last() != Some(&b'/') {
*buf_ptr.add(path.len()) = b'/';
offset += 1;
}
}
buf_ptr.add(offset).copy_from(name.as_ptr(), name.len());
offset += name.len();
*buf_ptr.add(offset) = 0;
libc::execve(buf_ptr.cast(), args.as_ptr(), envp.as_ptr());
}
}
break;
}
environ = environ.add(1);
}
// If execve is successful then it'll never return,
// thus we only reach this point on failure..
io::Error::from_raw_os_error(libc::ENOENT)
}
}
_reset = Some(Reset(*sys::env::environ()));
return Err(exec_with_env(self.get_program_cstr(), self.get_argv(), envp));
} else if let Some(envp) = maybe_envp {
_reset = Some(Reset(*sys::env::environ()));
*sys::env::environ() = envp.as_ptr();
}

libc::execvp(self.get_program_cstr().as_ptr(), self.get_argv().as_ptr());
Err(io::Error::last_os_error())
}
Expand Down Expand Up @@ -453,7 +515,9 @@ impl Command {

if self.get_gid().is_some()
|| self.get_uid().is_some()
|| (self.env_saw_path() && !self.program_is_path())
|| (!self.get_resolve_in_parent_path()
&& self.env_saw_path()
&& !self.program_is_path())
|| !self.get_closures().is_empty()
|| self.get_groups().is_some()
|| self.get_chroot().is_some()
Expand Down
6 changes: 6 additions & 0 deletions library/std/src/sys/process/unsupported.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ pub struct Command {
stdin: Option<Stdio>,
stdout: Option<Stdio>,
stderr: Option<Stdio>,
force_parent_path: bool,
}

// passed back to std::process with the pipes connected to the child, if any
Expand Down Expand Up @@ -52,6 +53,7 @@ impl Command {
stdin: None,
stdout: None,
stderr: None,
force_parent_path: false,
}
}

Expand Down Expand Up @@ -79,6 +81,10 @@ impl Command {
self.stderr = Some(stderr);
}

pub fn resolve_in_parent_path(&mut self, use_parent: bool) {
self.force_parent_path = use_parent;
}

pub fn get_program(&self) -> &OsStr {
&self.program
}
Expand Down
11 changes: 10 additions & 1 deletion library/std/src/sys/process/windows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@ pub struct Command {
startupinfo_fullscreen: bool,
startupinfo_untrusted_source: bool,
startupinfo_force_feedback: Option<bool>,
force_parent_path: bool,
}

pub enum Stdio {
Expand Down Expand Up @@ -192,6 +193,7 @@ impl Command {
startupinfo_fullscreen: false,
startupinfo_untrusted_source: false,
startupinfo_force_feedback: None,
force_parent_path: false,
}
}

Expand Down Expand Up @@ -224,6 +226,10 @@ impl Command {
self.force_quotes_enabled = enabled;
}

pub fn resolve_in_parent_path(&mut self, use_parent: bool) {
self.force_parent_path = use_parent;
}

pub fn raw_arg(&mut self, command_str_to_append: &OsStr) {
self.args.push(Arg::Raw(command_str_to_append.to_os_string()))
}
Expand Down Expand Up @@ -274,7 +280,10 @@ impl Command {
let env_saw_path = self.env.have_changed_path();
let maybe_env = self.env.capture_if_changed();

let child_paths = if env_saw_path && let Some(env) = maybe_env.as_ref() {
let child_paths = if env_saw_path
&& !self.force_parent_path
&& let Some(env) = maybe_env.as_ref()
{
env.get(&EnvKey::new("PATH")).map(|s| s.as_os_str())
} else {
None
Expand Down
Loading