Skip to content
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

std::process::Command accepting arbitrary file extensions when env() specified #50870

Closed
AndrewGaspar opened this issue May 18, 2018 · 9 comments
Labels
C-bug Category: This is a bug. O-windows Operating system: Windows T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@AndrewGaspar
Copy link
Contributor

AndrewGaspar commented May 18, 2018

Description

On Windows, std::process::Command accepts arbitrary file extensions in place of the correct file extension (typically .exe) for a command when at least one environment variable is specified (e.g. command.env("FOO", "BAR")). The issue does not reproduce when no environment variables are specified.

Rust Version

> rustc --version
rustc 1.26.0 (a77568041 2018-05-07)

Minimum repro

use std::process::Command;

fn main() {
    Command::new("cmd.bannana")
        .env("FOO", "BAR")
        .args(&["/c", "echo", "hello"])
        .status()
        .unwrap();
}

Expected Result

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Os { code: 2, kind: NotFound, message: "The system cannot find the file specified." }', libcore\result.rs:945:5
note: Run with `RUST_BACKTRACE=1` for a backtrace.
error: process didn't exit successfully: `target\debug\command-ext-err.exe` (exit code: 101)

Actual Result

hello

Negative Reprodution

The following code snippet does NOT reproduce the issue.

use std::process::Command;

fn main() {
    Command::new("cmd.bannana")
        .args(&["/c", "echo", "hello"])
        .status()
        .unwrap();
}
@cuviper
Copy link
Member

cuviper commented May 18, 2018

I believe it's this chunk that does it:

// To have the spawning semantics of unix/windows stay the same, we need
// to read the *child's* PATH if one is provided. See #15149 for more
// details.
let program = maybe_env.as_ref().and_then(|env| {
if let Some(v) = env.get(OsStr::new("PATH")) {
// Split the value and test each path to see if the
// program exists.
for path in split_paths(&v) {
let path = path.join(self.program.to_str().unwrap())
.with_extension(env::consts::EXE_EXTENSION);
if fs::metadata(&path).is_ok() {
return Some(path.into_os_string())
}
}
}
None
});

Specifically that .with_extension(env::consts::EXE_EXTENSION) changes the extension. I expect the intent is to add .exe only when there's no extension, so a bare cmd still finds cmd.exe.
(This should really be using PATHEXT though, rather than just "exe".)

@cuviper cuviper added O-windows Operating system: Windows T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. C-bug Category: This is a bug. labels May 18, 2018
@cuviper
Copy link
Member

cuviper commented May 18, 2018

Nevermind about PATHEXT -- that's used by cmd.exe, but CreateProcess only adds "exe".

If the file name does not contain an extension, .exe is appended. Therefore, if the file name extension is .com, this parameter must include the .com extension. If the file name ends in a period (.) with no extension, or if the file name contains a path, .exe is not appended.

@ollie27
Copy link
Member

ollie27 commented May 18, 2018

Duplicate of #37519. That chunk of code is very broken.

@AndrewGaspar
Copy link
Contributor Author

It does sound perhaps that this can't be fixed because it's a long standing issue and would break backwards compatibility. Please close this if so.

@jokeyrhyme
Copy link

@AndrewGaspar it seems a shame to have something in the core standard library that is known broken for all perpetuity

Are there any crates that implement a cross-platform std::process with the desired behaviour on Windows?

I've been using which for some of this functionality, but I still have to pair it with a call to cmd.exe /c example.com in order to execute, because std;:process::Command is insufficient on Windows for anything but ".exe" files

@retep998
Copy link
Member

The problem with being unable to run other things isn't the extension but rather that CreateProcess just won't work correctly with them. The only thing you can reliably CreateProcess are normal PE binaries, regardless of extension (although you do have to explicitly specify the extension if it isn't .exe).

The code in libstd definitely needs to be fixed though. I'll have to run some tests to determine how CreateProcess handles appending .exe though, to ensure behavior is consistent.

@BasixKOR
Copy link

BasixKOR commented Mar 6, 2020

I just found a documentation about how file extensions work in Windows, and it might help since CMD finds the application to open a file with %PATHEXT% using this information (I found this on Stack Overflow answer, but it may be wrong.).

@ChrisDenton
Copy link
Member

The issue in the OP has been fixed as a side-effect of #87704. So I think this issue can be closed.

Supporting %PATHEXT% could be a new feature request if there's demand for it. As noted above, it's not supported by the CreateProcess function that Rust currently uses.

@ChrisDenton
Copy link
Member

ChrisDenton commented Jul 4, 2022

Closing as resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. O-windows Operating system: Windows T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

7 participants