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

Windows: Support running script files using process::Command #94743

Closed
ChrisDenton opened this issue Mar 8, 2022 · 8 comments
Closed

Windows: Support running script files using process::Command #94743

ChrisDenton opened this issue Mar 8, 2022 · 8 comments
Labels
A-process Area: std::process and std::env C-feature-request Category: A feature request, i.e: not implemented / a PR. O-windows Operating system: Windows T-libs Relevant to the library team, which will review and decide on the PR/issue.

Comments

@ChrisDenton
Copy link
Member

ChrisDenton commented Mar 8, 2022

On Windows, currently only running .exe files is supported and script files aren't. Running .bat or .cmd (batch) files may work due to an undocumented feature of CreateProcessW (the underlying Windows API call) but it would be good to have proper, documented support rather than relying on undocumented features.

Implementing support for batch files would require finding cmd.exe ourselves and making sure the arguments passed to the batch file are interpreted as intended.

Supporting other types of script file may also be desirable. This can be done by examining the PATHEXT environment variable for a list of script file extensions. If the given script file has an extension that matches an entry in this list, then the registry can be used to look up the program to run it.

For example, getting the default value from HKEY_CLASSES_ROOT\.js will give the default id of the .js extension ("JSFile"). This id can be used to find the command by getting the default value from HKEY_CLASSES_ROOT\JSFile\Shell\open\command. This will return something like C:\Windows\System32\WScript.exe "%1" %* where %1 should be replaced with the full path to the script file and %* should be replaced with any arguments being passed to the script.

MSDN documentation

@rustbot rustbot added O-windows Operating system: Windows T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Mar 8, 2022
@AronParker
Copy link
Contributor

AronParker commented Apr 24, 2022

Finding cmd.exe can be done by inspecting the ComSpec Environment variable.

We don't have any existing machinery to deal with the Windows Registry in the Rust standard library at the moment do we? So to access it we would need to import more libraries or use a different program such as REG. Using the registry APIs would be the cleanest approach though.

@ChrisDenton
Copy link
Member Author

Finding cmd.exe can be done by inspecting the ComSpec Environment variable.

True. Though we may need a fallback if that's not available for some reason. Also we'd need to consider if using the environment for this could be a security risk in some scenarios. Possibly not, but it's worth considering before implementing.

We don't have any existing machinery to deal with the Windows Registry in the Rust standard library at the moment do we? So to access it we would need to import more libraries or use a different program such as REG. Using the registry APIs would be the cleanest approach though.

Yeah, we don't currently use any registry APIs so those would need to be added. And there would also be a fair amount of effort to ensure the logic for resolving the script's application is all implemented correctly.

Another question is if running scripts does belong in the standard library, given the added complexity involved, or whether it's enough for a third party crate to implement it. There is at least some demand for it though (which is why I made this issue).

@workingjubilee
Copy link
Member

workingjubilee commented Feb 26, 2023

I would like to see implementations for scripts and registry handling in external crates first. When it's something we already have started doing, like running executables, then adding more ways to talk about that is one thing, since it's merely extending what we do. But I don't think we should add a lot of API surface for a particular OS and its nonstandard interfaces if we don't have some experimentation first to see what a good Rust interface would look like. Indeed, such things may be particular to the OS version as well.

@thomcc
Copy link
Member

thomcc commented Feb 26, 2023

I don't think the cost of adding code to talk to the registry is really the decider here. It's a few functions, and they're relatively straightforward to use, and fairly portable across versions.

That said, I don't think we should do this — it's not really std's place to implement this kind of support IMO. We also don't support running scripts in all cases on Unix, for example in the case of shebang-less scripts; see #101511, and in particular this comment #101511 (comment), where we decided not to fall back to trying sh, which seems just about like the Unix equivalent of this.

@ChrisDenton
Copy link
Member Author

ChrisDenton commented Feb 26, 2023

Just to be clear, this isn't proposing any new public API per se, just that the existing API works with non-.exe (aka non-PE) files. However, this would complicate the implementation. And that complexity comes not from the extra OS APIs (although that does add some) but from the logic needed to do this all correctly. There is a function, ShellExecuteExW, that can do a lot of the work for us but this comes with major limitations (e.g. you can't redirect stdio).

The reason I made this issue is this is a common request. People are often surprised that this doesn't work already. Even Windows users may expect it to work (depending on where they're coming from). So I wanted to walkthrough what it would mean for us to support it.

@workingjubilee
Copy link
Member

Oh okay! I guess I didn't quite parse what you meant. So those are just internal changes, and people want us to be able to run scripts.

Hmm.
Yeah, I'm kinda with Thom: I'm still inclined to say "nah". I feel like Command::spawn morally should be "please chuck this directly into the program loader in the way that makes this an independent program-object on this OS". If this happens to run a script because that's how programs get loaded on this particular OS (as with shebangs), that's fine, but it feels doubtful we should try to do extra work. It may have... negative security implications if we try to "help" too much.

@workingjubilee workingjubilee added the A-process Area: std::process and std::env label Mar 19, 2023
@ChrisDenton
Copy link
Member Author

Ok given the feel expressed here I'll close this issue. Running scripts, etc can be left to a third party crate.

@RalfJung
Copy link
Member

I can add myself to the list of users surprised by this. In Miri we rewrote our ./miri script in Rust and now we are realizing that this regressed its functionality on Windows; one aspect of that script relied on it spawning itself which no longer works.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-process Area: std::process and std::env C-feature-request Category: A feature request, i.e: not implemented / a PR. O-windows Operating system: Windows T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

6 participants