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

Automatically insert \\?\ where needed on Windows #32689

Closed
DemiMarie opened this issue Apr 2, 2016 · 14 comments · Fixed by #89174
Closed

Automatically insert \\?\ where needed on Windows #32689

DemiMarie opened this issue Apr 2, 2016 · 14 comments · Fixed by #89174
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. O-windows Operating system: Windows

Comments

@DemiMarie
Copy link
Contributor

libstd should automatically insert \\?\ when necessary. The path must be canonicalized first; this can be done using the Windows API function GetFullPathNameW.

A necessary condition for needing to call GetFullPathNameW is that the path does not begin with \\?\. Afterwards, if the path begins with \\ but not \\?\, the \\ must be replaced with \\?\UNC\.

@retep998
Copy link
Member

retep998 commented Apr 3, 2016

There has already been a desire for something like this, and even an attempt at implementing this. However, doing something like this is somewhat complicated due to all the weird edge cases in how paths are handled on Windows. The current recommended solution is to create an external crate that handles normalizing paths and once all the kinks have been worked out of it and all the edge cases tested, then we will be able to consider whether to add something like that to libstd.

@dgrunwald
Copy link
Contributor

The documentation says that for long filenames, you need to prepend \\?\ before you're allowed to call GetFullPathNameW. So it seems to me that we'd have to re-implement that function without that limitation.

@retep998
Copy link
Member

retep998 commented Apr 3, 2016

@dgrunwald Which is exactly why it is so complicated because that means we have to handle all the weird edge cases ourselves! Hence the desire to have it worked out in an external crate first.

@nodakai
Copy link
Contributor

nodakai commented Apr 5, 2016

It would be great if we could properly implement such a feature in Rust stdlib, but the benefit might be marginal because many Windows tools either don't understand \\?\-prefixed paths at all, or cannot properly handle those paths enabled by the \\?\ prefix, including those longer than MAX_PATH.

  • The (Windows-standard) file open dialog from notepad.exe can suggest me what files are available under a \\?\-prefixed path but notepad.exe itself cannot open it.
  • When your cwd is as long as ~250 bytes, Mingw rustc.exe can still call cc.exe to produce yourcode.metadata.o but rustc.exe cannot remove it. More oddities can happen regarding the intereaction with the child cc.exe processs. Lengthy file names from make check taught me the lesson :-(
  • System32\msvcrt.dll has a fundamental bug with the \\?\ prefix and Mingw gcc.exe is affected by it: https://github.com/rust-lang/rust/blob/master/src/librustc/util/fs.rs
  • SetCurrentDirectory() doesn't accept paths longer than MAX_PATH regardless of the \\?\ prefix, or paths with the \\?\ prefix but without a trailing \.

@DemiMarie
Copy link
Contributor Author

@nodakai That is still not a good enough reason to implement the best support possible. Don't take my word for it – the CoreCLR team had the same decision! See this CoreFX issue which was fixed using essentially the same technique I am proposing, and was the inspiration for my suggestion.

@DemiMarie
Copy link
Contributor Author

@dgrunwald This is false. GetFullPathNameW does not care about MAX_PATH, and since CoreFX – and, through shared code, future versions of the full .NET Framework – relies on this behavior, it is not likely to change.

It is the fact that a Windows API call can be used to do all of the hard work for us that makes this practical. Otherwise, it would be too error-prone.

@retep998
Copy link
Member

retep998 commented Aug 9, 2016

I did a quick test and GetFullPathNameW does indeed work fine when supplied with paths that are longer than MAX_PATH. Therefore I would feel confident in having libstd automatically invoke GetFullPathNameW on any non-verbatim paths that are over MAX_PATH and add the verbatim prefix before passing the path to any OS functions.

@DemiMarie
Copy link
Contributor Author

Should I try to implement this?

On Aug 8, 2016 10:40 PM, "Peter Atashian" [email protected] wrote:

I did a quick test and GetFullPathNameW does indeed work fine when
supplied with paths that are longer than MAX_PATH. Therefore I would feel
confident in having libstd automatically invoke GetFullPathNameW on any
non-verbatim paths that are over MAX_PATH and add the verbatim prefix
before passing the path to any OS functions.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#32689 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AGGWBytKmh-Octe8mvxwZW3hAc0xVakSks5qd-iagaJpZM4H-aHL
.

@retep998
Copy link
Member

retep998 commented Aug 9, 2016

@DemiMarie I don't see why not. Feel free to give it a shot.

@DemiMarie
Copy link
Contributor Author

I just realized a problem: GetFullPathName has undefined behavior if it is passed a relative path name and another thread is concurrently calling SetCurrentDirectory (data race on the current directory). So does every other Windows API function that takes a relative path name.

This means that Rust must either

  • use its own version of the current directory, with proper synchronization
  • make all functions that take relative paths unsafe to call.

Indeed, the MSDN documentation states that multithreaded programs and all shared libraries should not use relative path names at all. Presumably, such code should fail if given a relative path name. MSDN says that relative paths provided by the user should be converted to absolute paths while the program is still single-threaded.

On the other hand, CoreFX does call this function, so I guess it is okay.

@retep998
Copy link
Member

retep998 commented Aug 13, 2016

@DemiMarie What do you mean by undefined behavior? The way it is implemented, the current directory is stored in the PEB, and all access to the PEB goes through the FastPebLock, which is a mutex basically. Memory unsafe data races do not occur. The reason it recommends not using relative paths is because if you set the current directory and intend to do some stuff in that given directory, another thread might come along and change it on you causing your code to behave incorrectly.

@DemiMarie
Copy link
Contributor Author

Sorry, I did not know that. The MSDN docs seemed to say otherwise.

On Aug 13, 2016 7:45 PM, "Peter Atashian" [email protected] wrote:

@DemiMarie https://github.com/DemiMarie What do you mean by undefined
behavior? The way it is implemented, all access to the current directory
goes through a critical section, specifically FastPebLock. Memory unsafe
data races do not occur. The reason it is recommended not to use it is
because if you set the current directory and intend to do some stuff in
that given directory, another thread might come along and change it on you
causing your code to behave incorrectly.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#32689 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AGGWB0v8lycoE-UQX4fwZS0crev41CoVks5qflcrgaJpZM4H-aHL
.

@Yomodo
Copy link

Yomodo commented Oct 8, 2016

On Windows platform, AlphaFS can help you out.

@steveklabnik
Copy link
Member

Triage: no changes that I'm aware of

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. O-windows Operating system: Windows
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants