go: os: MkdirAll fails when a parent folder contains trailing spaces

Normalizing this should be trivial, but I would argue against doing so. It would fix the MkdirAll, yet the underlying problem will still be there: the white-spaced path won’t exist even though MkdirAll succeed, so any other operation on that path is going to fail.

Note that many of the functions in the os package already call the fixLongPath helper function, which converts paths of length ≥ 248 to the equivalent \\?\ form provided that os.canUseLongPaths is also false (see #3358, #41734).

So under certain conditions today, Mkdir (and MkdirAll) will actually succeed in creating the space-containing paths as requested.

In other conditions, Mkdir silently creates a path that differs by a trailing character, and MkdirAll creates a path that differs from some parent directory by a trailing character, then fails. Neither of those behaviors seems like what the user intends, and neither is particularly useful.

I think that, rather than letting the OS do as it likes and possibly fail in ways that are subtle and difficult to predict, we should explicitly choose to either always succeed or always fail for these paths. That is, we should choose either:

a. Change os.Mkdir (and probably also os.OpenFile with O_CREATE as well?) to check path components for non-UNC paths for trailing ASCII spaces and periods, and explicitly reject paths that end in an ASCII space or period.

• As a side effect, that would cause os.MkdirAll to refuse to create such paths.
• That would be consistent with https://learn.microsoft.com/en-us/windows/win32/fileio/naming-a-file#naming-conventions, which says in no uncertain terms: “Do not end a file or directory name with a space or a period.”
• Users who intentionally create such files or directories could switch to using the \\?\ form to manipulate them.
• Opening such files and/or directories for reading would not be rejected explicitly, but other existing bugs may prevent their use. (For example, os.ReadDir at HEAD seems to fail for such a directory.)

or:

b. Change (and rename) fixLongPath to check for such paths and escape them to their corresponding \\?\ forms.

• This would allow Go programs to create files and directories with names with trailing space or period characters if the underlying filesystem permits them, which would make the behavior of Go programs on Windows more consistent with their behavior on Unix platforms.
  • However, it would not fix other functions like os.ReadDir that fail for such paths today, so the improved consistency would be limited at best. (Unless we fully open the can of worms and try to fix everything to work with these paths…)
• It would also not fix the trailing-space problem for relative paths until #41734 is also addressed.

On balance, I would prefer to make Mkdir and OpenFile reject these paths explicitly.

(CC @neild @zx2c4 for Windows path normalization, the gift that keeps on giving)

Normalizing this should be trivial, but I would argue against doing so. It would fix the MkdirAll, yet the underlying problem will still be there: the white-spaced path won’t exist even though MkdirAll succeed, so any other operation on that path is going to fail.

We either always normalize all paths or we don’t, else we will just be moving the failure to a later point, making it worst.

As @seankhliao mentioned, Windows already supports trailing whitespaces when the path is prefixed with \\?\, so the workaround is there.

Additional data point: .Net does not support this either for the sake of interoperability. https://github.com/dotnet/runtime/issues/23292#issuecomment-363237559.

Thank you, @bcmills, for such a great explanation with the possible options. I will start working on approach a., check it with the regression test you provided, and share the results here for further discussion.

a. Change os.Mkdir (and probably also os.OpenFile with O_CREATE as well?) to check path components for non-UNC paths for trailing ASCII spaces and periods, and explicitly reject paths that end in an ASCII space or period.

I agree that we should not normalize paths here, but could we at least improve the failure mode here?

Improving the error message seems like a good idea, but I would try to avoid adding logic to detect leading/trailing whitespaces or periods and instead rely on giving general tips based on windows error codes.

For example, MkdirAll logic ensures that intermediate directories exist by calling CreateDirectory on every non-existing intermediate directory. So, if syscall.CreateDirectory returns ERROR_PATH_NOT_FOUND (see error description in docs), it means that either a concurrent process deleted the directory or that Windows did not honor the folder path in a previous CreateDirectory call.

Windows docs have a special entry for dealing with whitespaces in files and folder names: https://docs.microsoft.com/en-us/troubleshoot/windows-client/shell-experience/file-folder-name-whitespace-characters.

The relevant part is this:

File and Folder names that begin or end with the ASCII Space (0x20) will be saved without these characters. File and Folder names that end with the ASCII Period (0x2E) character will also be saved without this character. All other trailing or leading whitespace characters are retained.

Looking at how MkdirAll is implemented on windows, it seems that we are not removing leading and trailing whitespaces from non-leaf folders. This leads to failure mode:

1. MkdirAll(“C:\temp\folder \this one fails”, 0644)
2. MkdirAll(“C:\temp”, 0644)
3. MkdirAll("C:\temp\folder ", 0644) // But Windows created “C:\temp\folder”!!
4. Mkdir(“C:\temp\folder \this one fails”, 0644) // Wops, parent folder does not exist