What version of Go are you using (go version)?

$ go version
go version go1.18.4 windows/amd64

Does this issue reproduce with the latest release?

yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
set GO111MODULE=on
set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\TEST\AppData\Local\go-build
set GOENV=C:\Users\TEST\AppData\Roaming\go\env
set GOEXE=.exe
set GOEXPERIMENT=
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOINSECURE=
set GOMODCACHE=C:\Users\TEST\go\pkg\mod
set GONOPROXY=
set GONOSUMDB=
set GOOS=windows
set GOPATH=C:\Users\TEST\go;C:\Daten\go
set GOPRIVATE=
set GOPROXY=https://proxy.golang.org,direct
set GOROOT=C:\Program Files\Go
set GOSUMDB=sum.golang.org
set GOTMPDIR=
set GOTOOLDIR=C:\Program Files\Go\pkg\tool\windows_amd64
set GOVCS=
set GOVERSION=go1.18.4
set GCCGO=gccgo
set GOAMD64=v1
set AR=ar
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set GOMOD=C:\Daten\go\src\megabildgo\go.mod
set GOWORK=
set CGO_CFLAGS=-g -O2
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-g -O2
set CGO_FFLAGS=-g -O2
set CGO_LDFLAGS=-g -O2
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -mthreads -fmessage-length=0 -fdebug-prefix-map=C:\Users\TEST\AppData\Local\Temp\go-build1829370843=/tmp/go-build -gno-record-gcc-switches

What did you do?

err := os.MkdirAll("C:\\temp\\folder \\this one fails", 0644) if err != nil { fmt.Println(err) }

What did you expect to see?

All folders being created

What did you see instead?

There is a folder "folder" without trailing space. The folder "this one fails" is not created. an os.PathError is thrown: mkdir C:\temp\folder \this one fails: Das System kann den angegebenen Pfad nicht finden.

Comment From: mengzhuo

cc @golang/windows

Comment From: qmuntal

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

Comment From: seankhliao

how much normalization should we do vs having the user specify \\?\ when they need weird paths?

Comment From: qmuntal

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.

Comment From: mattn

I am also against normalizing this. If Windows changes its behavior in the future, Go will have to follow the changes.

Comment From: megabild

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. dotnet/runtime#23292 (comment).

Creating the folder with \?\ works with MkdirAll. Can we make this the default behavior on windows? Otherwise the programmer needs to prefix the MkdirAll call if needed. I wonder if anyone even knows about that special \?\ thing.

Comment From: qmuntal

I wonder if anyone even knows about that special \\?\ thing.

From Windows docs:

For file I/O, the "\?\" prefix to a path string tells the Windows APIs to disable all string parsing and to send the string that follows it straight to the file system.

Comment From: bcmills

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

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

(Perhaps something in the call stack could notice the leading or trailing space in the intermediate paths and fail with a clearer error message.)

Comment From: qmuntal

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.

Comment From: gopherbot

Change https://go.dev/cl/458136 mentions this issue: os: handle trailing spaces case on MkdirAll

Comment From: vladtenlive

@bcmills @qmuntal I realized that syscall.ERROR_PATH_NOT_FOUND is Windows OS specific and it will not compile on different platforms. How could I avoid this or wrap it properly?

Comment From: ianlancetaylor

@vladtenlive Use os.IsNotExist(err).

Comment From: ianlancetaylor

Or I guess that we now recommend the equivalent errors.Is(err, fs.ErrNotExist).

Comment From: vladtenlive

@ianlancetaylor fixed, thank you https://go-review.googlesource.com/c/go/+/458136

Comment From: bcmills

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. … 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.

I think that leaves us back where we started: if we can't distinguish between “a concurrent process deleted the directory“ and “Windows did not honor the folder path in a previous CreateDirectory call”, then the existing ERROR_PATH_NOT_FOUND message seems like about the best we can do.

Comment From: bcmills

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)

Comment From: bcmills

I would suggest the following (https://go.dev/play/p/T63O30FOM8Y?v=gotip) as a regression test. It passes today on Linux.

// Regression test for https://go.dev/issue/54040:
// Mkdir for a path ending with a trailing ASCII space (or period)
// should either return a non-nil error or create a usable directory.
func TestMkdirTrailingSpace(t *testing.T) {
    parent := t.TempDir()

    dir := filepath.Join(parent, "ends in space ")
    err := os.Mkdir(dir, 0777)
    if err != nil {
        if runtime.GOOS == "windows" && !strings.HasPrefix(dir, `\\?\`) {
            t.Skipf("skipping: Windows does not support paths with trailing spaces")
        }

        t.Fatal(err)
    }

    t.Logf("Mkdir(%q, 0777)", dir)
    t.Cleanup(func() {
        err := os.Remove(dir)
        if err == nil {
            t.Logf("Remove(%q)", dir)
        } else {
            t.Error(err)
        }
    })

    fi, err := os.Lstat(dir)
    if err == nil {
        t.Logf("Lstat(%q).Name() = %q", dir, fi.Name())
    } else {
        t.Error(err)
    }

    fi, err = os.Stat(dir)
    if err == nil {
        t.Logf("Stat(%q).Name() = %q", dir, fi.Name())
    } else {
        t.Error(err)
    }

    ents, err := os.ReadDir(dir)
    if err == nil {
        t.Logf("ReadDir(%q) = %v", dir, ents)
    } else {
        t.Error(err)
    }

    sub := filepath.Join(dir, "sub")
    err = os.Mkdir(sub, 0777)
    if err == nil {
        t.Logf("Mkdir(%q, 0777)", sub)
        err := os.Remove(sub)
        if err == nil {
            t.Logf("Remove(%q)", sub)
        } else {
            t.Error(err)
        }
    } else {
        t.Error(err)
    }
}

Comment From: neild

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.

This option sounds reasonable to me. (Although rather than "for non-UNC paths", say "for paths not starting with \\?\--UNC paths are something different.)

Comment From: vladtenlive

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.

Comment From: gopherbot

Change https://go.dev/cl/618496 mentions this issue: syscall: check for valid Windows path in Open and Mkdir

Comment From: qmuntal

The more I think about the "option a" detailed in https://github.com/golang/go/issues/54040#issuecomment-1370242553 (and implemented in CL 618496), the less I like it, as it is a breaking change. Until today, one could do something like the following (pseudo)code snippet and assume Windows compatibility layer to chime by creating and opening a file without the trailing space.

const name = "test.txt "
os.Create(name)
os.Open(name)

After CL 618496, Go will error out in os.Create before letting Windows do its best effort.

IMO fixing the original issue (os.MkdirAll not working well when the path contains trailing spaces) doesn't worth introducing a breaking change, even if the new behavior would be more consistent and robust.

Also, thanks to the work done in #67002, we will soon have some additional primitives (mkdirat, openat, ...) that will allow us to reimplement Mkdirall in a way that won't trigger the issue reported in here. That is, os.MkdirAll("foo /bar") will successfully create "foo/bar" (which is weird, but it is what one expects on Windows).

Having said all this, I propose to revert CL 618496 and wait till Go 1.25 to implement a better fix.

@neild @gdams @golang/windows

Comment From: gdams

Having said all this, I propose to revert CL 618496 and wait till Go 1.25 to implement a better fix.

I agree, I'll revert the CL now via CL 629595

Comment From: dmitshur

@qmuntal If I understand right, when you applied the release-blocker label in Nov 2024 it was meant to be to track reverting CL 618496 in time for Go 1.24, is that right? Note that when using that label, the milestone needs to match the release that the issue intends to block, so it had no effect for the Unplanned milestone.

The revert CL 629595 did land in time for Go 1.24 though, so that worked out, and this can probably go back to being a normal non-blocking issue for Go 1.25 that needs investigation.