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

process::Command on Windows doesn't work with canonicalized paths #133553

Open
jbiason opened this issue Nov 27, 2024 · 4 comments
Open

process::Command on Windows doesn't work with canonicalized paths #133553

jbiason opened this issue Nov 27, 2024 · 4 comments
Labels
C-bug Category: This is a bug. O-windows Operating system: Windows T-libs Relevant to the library team, which will review and decide on the PR/issue.

Comments

@jbiason
Copy link

jbiason commented Nov 27, 2024

I found a "weird" issue with std::process::Command and canonicalized paths on Windows. Something like (which assumes lsd is installed on the machine):

let p1 = Path::new(".");
let p2 = p1.canonicalize().unwrap();

let e = std::process::Command::new("lsd").current_dir(&p1).spawn().wait_for_output().unwrap();
println!("P1 = {e:?}");

let e = std::process::Command::new("lsd").current_dir(&p2).spawn().wait_for_output().unwrap();
println!("P2 = {e:?}");

... works fine for the first execution, showing the current directory contents; but the second execution fails with

Unhandled Exception: System.ArgumentException: Illegal characters in path.
   at System.Security.Permissions.FileIOPermission.EmulateFileIOPermissionChecks(String fullPath)
   at System.IO.Directory.InternalGetCurrentDirectory(Boolean checkHost)
   at shim.ShimProgram.Main(String[] args)

... which looks like it is caused due .canonicalize() prefixing the path with "\\?\" -- which, for what I can understand, correct, telling Windows that the path must be taken as-in, without any processing.

Meta

rustc --version --verbose:

rustc 1.82.0 (f6e511eec 2024-10-15)
binary: rustc
commit-hash: f6e511eec7342f59a25f7c0534f1dbea00d01b14
commit-date: 2024-10-15
host: x86_64-unknown-linux-gnu
release: 1.82.0
LLVM version: 19.1.1

Also note that I'm cross-compiling from Linux to Windows, using "x86_64-pc-windows-gnu" as target triple.

@jbiason jbiason added the C-bug Category: This is a bug. label Nov 27, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Nov 27, 2024
@ehuss
Copy link
Contributor

ehuss commented Nov 27, 2024

I think this is fundamentally another case of #42869. Some software just does not like \\?\ paths.
I don't know what your use case is, but there is an alternative of std::path::absolute.

@ChrisDenton
Copy link
Member

I should also note that current_dir doesn't really work with \\?\ paths. E.g., from the Windows docs for SetCurrentDirectory

Each process has a single current directory made up of two parts:

  • A disk designator that is either a drive letter followed by a colon, or a server name and share name (\servername\sharename)
  • A directory on the disk designator

Essentially only paths like C:\ or \\server\share are supported. It's possible to set other path types but they will often cause breakages elsewhere in the Windows API.

What Command could do is attempt to remove the \\?\ from the current directory path, if present.

@jieyouxu jieyouxu added O-windows Operating system: Windows T-libs Relevant to the library team, which will review and decide on the PR/issue. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Nov 28, 2024
@jbiason
Copy link
Author

jbiason commented Nov 28, 2024

What Command could do is attempt to remove the \\?\ from the current directory path, if present.

Wouldn't this change the semantics of what the path is? As far as I could understand, \\?\ changes the meaning of the path itself, preventing a bunch of weird processing (like CON) and allows long paths. The last part is important 'cause canonicalize may work fine, but then everything else will fail (ok, it is already failing, it would just fail in a different way).

My guess is that, as far as Rustc is concerned, everything is okay (unless there is something more in #42869 mentioned by @ehuss) and rustc .canonicalize() shouldn't be adding the prefix in the first place.

@ChrisDenton
Copy link
Member

ChrisDenton commented Nov 29, 2024

Sure, simply removing \\?\ would be wrong (or simply replacing it with \\ in the case of \\?\UNC\).

However, what I've done in other projects is to test if the it's possible to round trip paths without change. E.g. if after converting the path, GetFullPathNameW returns the same path then the path will be the same with or without the \\?\.

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 Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants