os/exec: unexpected ErrDot returned if there is an empty path in PATH

[LawyZheng]

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

$ go version

go1.20.2

Does this issue reproduce with the latest release?

no tested yet

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

go env Output

$ go env
GO111MODULE="on"

GOARCH="arm64"

GOBIN=""

GOCACHE="~/Library/Caches/go-build"

GOENV="~/Library/Application Support/go/env"

GOEXE=""

GOEXPERIMENT=""

GOFLAGS=""

GOHOSTARCH="arm64"

GOHOSTOS="darwin"

GOINSECURE=""

GOMODCACHE="~/go/pkg/mod"

GONOPROXY=""

GONOSUMDB=""

GOOS="darwin"

GOPATH="~/go"

GOPRIVATE=""

GOPROXY="https://goproxy.cn,direct"

GOROOT="~/.g/go"

GOSUMDB="sum.golang.org"

GOTMPDIR=""

GOTOOLDIR="~/.g/go/pkg/tool/darwin_arm64"

GOVCS=""

GOVERSION="go1.20.2"

GCCGO="gccgo"

AR="ar"

CC="clang"

CXX="clang++"

CGO_ENABLED="1"

GOMOD="~/Library/Application Support/JetBrains/GoLand2023.1/scratches/go.mod"

GOWORK=""

CGO_CFLAGS="-O2 -g"

CGO_CPPFLAGS=""

CGO_CXXFLAGS="-O2 -g"

CGO_FFLAGS="-O2 -g"

CGO_LDFLAGS="-O2 -g"

PKG_CONFIG="pkg-config"

GOGCCFLAGS="-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/bz/g8kmk1fn0xj0rnprc0v0h33r0000gn/T/go-build1655805848=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

As an apology, although my build env is Mac M1, I cross-compiled the application into the amd64 windows.
So this is a description of the Windows system.

I missed to set my PATH with a empty path, like path1;;path2;
So when I wanted to use exec.Command to open the cmd.exe (my working directory is C:/Windows/System32, just the same as cmd.exe lies), I got aErrDot.

In this situation, I got 2 ways to find the cmd.exe in PATH. The one is to find in the absolute path C:/Windows/System32, and the other one is the empty path which I entered by mistake. And the empty one comes first.

go/src/os/exec/lp_windows.go

Lines 103 to 139 in 8abc6e2

	if _, found := syscall.Getenv("NoDefaultCurrentDirectoryInExePath"); !found {
	if f, err := findExecutable(filepath.Join(".", file), exts); err == nil {
	if execerrdot.Value() == "0" {
	execerrdot.IncNonDefault()
	return f, nil
	}
	dotf, dotErr = f, &Error{file, ErrDot}
	}
	}
	path := os.Getenv("path")
	for _, dir := range filepath.SplitList(path) {
	if f, err := findExecutable(filepath.Join(dir, file), exts); err == nil {
	if dotErr != nil {
	// https://go.dev/issue/53536: if we resolved a relative path implicitly,
	// and it is the same executable that would be resolved from the explicit %PATH%,
	// prefer the explicit name for the executable (and, likely, no error) instead
	// of the equivalent implicit name with ErrDot.
	//
	// Otherwise, return the ErrDot for the implicit path as soon as we find
	// out that the explicit one doesn't match.
	dotfi, dotfiErr := os.Lstat(dotf)
	fi, fiErr := os.Lstat(f)
	if dotfiErr != nil || fiErr != nil || !os.SameFile(dotfi, fi) {
	return dotf, dotErr
	}
	}
	if !filepath.IsAbs(f) {
	if execerrdot.Value() != "0" {
	return f, &Error{file, ErrDot}
	}
	execerrdot.IncNonDefault()
	}
	return f, nil
	}
	}

In line 114, it enters the empty path which is considered as the . path, and finds cmd.exe in this path.
Without a doubt, it is treated as a RELATIVE PATH and returned with an ErrDot.

I know the relative path is forbidden now.
And I think this is similar to the problem mentioned in the #53536, but not exactly the same.

What did you expect to see?

I have to admit that this is my fault to set an empty path by mistake.
But I still wonder whether the logic in this code should be optimized or not.
Provided with my two opinions as below:

1. In the PATH list, skip the empty path or anything like the relative path which will function as the . path eventually. After all, there is no need to deal with the . path in this code, because it has been dealt with before.

2. We can sort the list before range it. Make the absolute paths come before the relative paths.

[ianlancetaylor]

Note that an empty path is meaningful on Unix systems, and means the current directory. So at least on Unix systems the behavior sounds correct. I don't know what an empty path means on Windows systems.

[seankhliao]

cc @golang/windows

[LawyZheng]

Note that an empty path is meaningful on Unix systems, and means the current directory. So at least on Unix systems the behavior sounds correct. I don't know what an empty path means on Windows systems.

empty path also means the current directory in Windows. However, the logic to deal with the current directory logic shouldn't be handled in line 103 already?

[ianlancetaylor]

The check at line 103, for the NoDefaultCurrentDirectoryInExePath environment variable, is about how to look up executables separately from PATH. Originally Windows always looked in the current directory. I don't know the details, but I guess that now that environment variable tells Windows to not look in the current directory. I don't think that says anything about what to do if PATH explicitly lists the current directory.

[ianlancetaylor]

But I could be wrong, and would be happy to see a pointer to some Windows docs explaining how empty strings should be handled in PATH.

[qmuntal]

I'm not aware of any Microsoft-vetted rule that says how to handle empty PATH entries, i.e. ;;, other than treat it as current directory.

Doing a quick search, I saw that Rust skips empty entries (source), so they are never used to evaluate potential executable locations.

IMO, standard lacking, our behavior is also good and consistent. If someone has added an empty entry, then we should honor it and return the binary at the working directory and ErrDot.

[LawyZheng]

Of course, we should honor that someone adds an empty entry.

But we should also think that running an application as a service in Windows will make the working directory in the C:/Window/System32, where many system executable binary lies, like cmd.exe and query.exe.

Unlike Unix, we can decide where is the working directory. Windows is set (or maybe I don't know how to change it.)
I don't know whether this will have an influence on CGO which might interact will Windows System API by some executable binaries.

[bcmills]

@LawyZheng, ErrDot is only returned for the implicit “current directory” search if there is not also an explicit %PATH% entry for the directory. (That was #53536, fixed in https://go.dev/cl/414054.)

[bcmills]

Hmm, I think I see what you are saying. If there is an explicit empty entry in %PATH%, perhaps that should update dotf and dotErr for that entry within the loop instead of returning ErrDot immediately. 🤔

[LawyZheng]

Hmm, I think I see what you are saying. If there is an explicit empty entry in %PATH%, perhaps that should update dotf and dotErr for that entry within the loop instead of returning ErrDot immediately. 🤔

Perhaps, updating dof and dofErr is a good idea. Honoring the empty entry, meanwhile dealing with the ErrDot.
But before that, maybe we need to make sure what is the meaning of empty entry in the Windows.

[qmuntal]

But before that, maybe we need to make sure what is the meaning of empty entry in the Windows.

In my previous comment I said that Rust was skipping empty entries. Well, I found out that PowerShell's Get-Command also skips empty entries. This behavior is not documented, but I've reproduced it locally and also found the code that does the skipping: https://github.com/PowerShell/PowerShell/blob/18350bc9ff28532d3d75cf4f07642d35fd8c490b/src/System.Management.Automation/engine/CommandDiscovery.cs#L1216

I'm starting to think that honoring ;; is not what is expected on Windows...

[neel-awn]

Can we get an update on this? Experiencing issues with this behaviour where empty/NIL PATH entries on Windows OS are resolving to relative path, making it impossible to use exec.Command('cmd.exe') (just an example) on machines which have PATH env variable with NIL entries since it returns ErrDot...

[gopherbot]

Change https://go.dev/cl/528037 mentions this issue: os/exec: ignore empty entries in PATH and treat an explicit "." the same as implicit

[gopherbot]

Change https://go.dev/cl/548481 mentions this issue: doc: document os/exec changes on Windows