-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Support long path names on all Windows versions #2188
Conversation
Required readingIf you want to be able to provide useful comments on this RFC, it is essential that you actually understand the various forms of paths on Windows. Therefore, please read this article before commenting: https://googleprojectzero.blogspot.com/2016/02/the-definitive-guide-on-win32-to-nt.html Thank you. |
Could we expose the transformation function publicly in (On the other hand, I don’t think we expose the UTF-16 conversion function that is also needed, and this isn’t quite so useful without that. I do know that I’ve copied code from the standard library to achieve these things before. But exposing that is definitely out of scope for this RFC.) |
I'm building a P2P filesharing application which would be deemed seriously crippled if it cannot support longer paths. So this is very much a needed fix. @chris-morgan I, for one, would vote for making the std APIs automatically and seemlessly work with longer paths. Requiring the caller to call extra functions for longer paths would be awkward. |
@gurry I do not describe a function required for normal use, but rather a low-level function for cases where the user is calling the NT API directly. |
Calling NT directly is very special because you're passing NT paths, not win32 paths. A path that starts with However I am still interested in these API additions:
|
I thought that CoreFX (.NET Core standard library) used `GetFullPathNameW`, but it doesn’t.
I once wrote the beginnings of an RFC for this, but never finished it. It goes a different route, doing the conversion ourselves. It is not very difficult, and also works for files that do not (yet) exist in the filesystem. for reference (I did not recently re-read it) |
|
As I am looking at implementation strategies, I am finding that: We don’t really want bug-for-bug compatibility with Windows.The Windows behavior:
So, I propose that Rust do its own normalization. This will require me to write LOTS of test cases! |
When I read that big statement, my initial expectation was that you were going to continue with a list of points in favour of using the Windows normalization function to avoid introducing bugs where Rust's behaviour diverges from the system behaviour. (ie. "We don't want to be responsible for having to match Windows bug-for-bug to avoid problems.") |
Regardless of what decision we end up taking, we will need a ridiculous number of test cases. I also recommend first putting the normalization implementation into an external crate and getting it tested in all sorts of zany setups to make sure it behaves as people expect in the most obscure of corner cases. |
@ssokolow I would much rather use the system implementation. However, parts of Windows pathname handling are such security footguns (such as DOS devices) that I really really don’t want Rust programs to inherit them. Personally, I think that we should act just like the Windows API, except for a certain list of special cases, each of which needs to be documented and justified. The ones that come to mind are:
2 and 4 can be accomplished by padding the path with a suitable suffix (such as |
I certainly agree. I was more expressing that, to someone like me who hasn't used Windows on anything other than retro-gaming PCs in over a decade and has only started to learn the depth of the rabbit hole more recently, the degree to which Windows pathname handling is broken boggles the mind. The part about trailing periods and spaces didn't even come to mind for me. (ie. It was more intended as a very sloppy attempt to indicate that more supporting detail for the rationale wouldn't go amiss.) |
Have I mentioned the sheer number of test cases we're going to need for this? |
You can add a suffix that begins with a backslash.
…On Nov 12, 2017 9:36 PM, "Peter Atashian" ***@***.***> wrote:
2 and 4 can be accomplished by padding the path with a suitable suffix
(such as rust or just a) before passing to GetFullPathNameW and then
stripping it afterwards.
X:\COM1.blah will still get transformed into \\.\COM1 or \??\COM1, so
simply tacking on a suffix won't work.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#2188 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AGGWB08AkDunk7WVHMXyHgGTByoBf6_Iks5s16s1gaJpZM4QFNcy>
.
|
@retep998 I agree with the huge number of test cases! Here is the algorithm I am leaning towards right now:
|
We don't pass paths directly to NT though. We always call win32 functions so stripping the |
I have thought more on this and done some testing, and the following seems to work for every case I have come across: Subroutine
|
Just leaving a comment so I can come back shortly and spend some more time digging in to this! For reference (mostly my own), looks like CoreFX (.NET's I like @retep998's suggestion of getting an implementation out in the wild and iterating on it there. |
When considering a change like this I'd personally be quite curious to explore what other language ecosystems are doing to see if we can learn about the pros/cons of various strategies. From some googling I've found:
Perhaps some more research could be done into these various ecosystems to see what they're doing and list out some pros/cons of the various strategies? |
ManifestsPros
Cons
|
Thanks @retep998! I think we should include your summary in some form in the Alternatives section of the RFC. The What do you all think? |
@rfcbot fcp close This RFC has been open for quite some time and I'd like to propose that we close this RFC. My previous findings seem to indicate that there's not necessarily broad consensus among language standard libraires about whether this is a good idea or not, and I'd personally prefer to err on the side of less "magic" in the standard library. While I think that it's always possible we could support the manifest option more easily in Rust I'd specifically like to propose closing this RFC with the idea of inserting expansion of paths on all system calls in the standard library. |
Team member @alexcrichton has proposed to close this. The next step is review by the rest of the tagged teams: No concerns currently listed. Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
🔔 This is now entering its final comment period, as per the review above. 🔔 |
The final comment period, with a disposition to close, as per the review above, is now complete. By the power vested in me by Rust, I hereby close this RFC. |
Rendered