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

path/filepath: EvalSymlinks fails on Windows when passed a UNC share root path #42079

Closed
kevpar opened this issue Oct 19, 2020 · 4 comments
Closed
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Windows
Milestone

Comments

@kevpar
Copy link
Contributor

kevpar commented Oct 19, 2020

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

$ go version
go version go1.15.3 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=
set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\kevpar\AppData\Local\go-build
set GOENV=C:\Users\kevpar\AppData\Roaming\go\env
set GOEXE=.exe
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOINSECURE=
set GOMODCACHE=C:\Users\kevpar\go\pkg\mod
set GONOPROXY=
set GONOSUMDB=
set GOOS=windows
set GOPATH=C:\Users\kevpar\go
set GOPRIVATE=
set GOPROXY=https://proxy.golang.org,direct
set GOROOT=c:\go
set GOSUMDB=sum.golang.org
set GOTMPDIR=
set GOTOOLDIR=c:\go\pkg\tool\windows_amd64
set GCCGO=gccgo
set AR=ar
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set GOMOD=C:\Users\kevpar\Source\repos\test\go.mod
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 -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=C:\Users\kevpar\AppData\Local\Temp\go-build377379998=/tmp/go-build -gno-record-gcc-switches
GOROOT/bin/go version: go version go1.15.3 windows/amd64
GOROOT/bin/go tool compile -V: compile version go1.15.3

What did you do?

main.go

package main

import "path/filepath"

func main() {
	p, err := filepath.EvalSymlinks(`\\localhost\c$`)
	if err != nil {
		panic(err)
	}
	println(p)
}

Repro:

> go run .

What did you expect to see?

\\localhost\c$

What did you see instead?

panic: The system cannot find the file specified.

goroutine 1 [running]:
main.main()
        C:/Users/kevpar/Source/repos/test/main.go:8 +0xad
exit status 2

Details

This is due to EvalSymlinks using the Windows API FindFirstFile to attempt to resolve each path component after the volume name. Since \\localhost\c$ is parsed as the volume name with an empty path following it, this results in a call to FindFirstFile for the path of \\localhost\c$, which fails with The system cannot find the file specified..

The failure from FindFirstFile is expected. Per the documentation for that function:

On network shares, you can use an lpFileName in the form of the following: "\\Server\Share*". However, you cannot use an lpFileName that points to the share itself; for example, "\\Server\Share" is not valid.

One important occurrence of this bug is with Kubernetes. The containerd/containerd runtime uses EvalSymlinks to resolve each container mount path on the host. When a mount path is the root of a UNC share (or a symlink to the share root), then EvalSymlinks fails, preventing the container from starting. More discussion on this in kubernetes/kubernetes#94213.

As a workaround, if the path to EvalSymlinks has a \ appended, the function works properly, as it hits a conditional that causes an early return, before calling FindFirstFile. However, it would be good to fix the source of this issue as well.

Note: This repros for all UNC share roots passed to EvalSymlinks. I used the admin share (\\localhost\c$) in the example, as that allows a repro without needing access to an external network share, which makes testing easier.

@kevpar
Copy link
Contributor Author

kevpar commented Oct 19, 2020

It looks like this can be fixed by just changing the line here to also have a case for len(path) == 0. This will effectively skip the loop where it tries to resolve later path components using FindFirstFile, which is fine, since if path is empty, there are no later path components.

However, I'm currently getting an error when trying to run tests in src/path/filepath, even without making any changes. So I am probably doing something wrong :)

> go test .
# std/path/filepath
package std/path/filepath_test
        match_test.go:9:2: use of internal package internal/testenv not allowed
FAIL    std/path/filepath [setup failed]
FAIL

kevpar added a commit to kevpar/go that referenced this issue Oct 20, 2020
Fixes golang#42079

Previously, EvalSymlinks returned an error when called with the root of
a UNC share (e.g. \\server\share). This was due to Windows's
FindFirstFile function not supporting a share root path.

To resolve this, now return early from toNorm in the case where the path
after the volume name is empty. Skipping the later path component
resolution shouldn't have any negative impact in this case, as if the
path is empty, there aren't any path components to resolve anyways.

Signed-off-by: Kevin Parsons <[email protected]>
kevpar added a commit to kevpar/go that referenced this issue Oct 20, 2020
Fixes golang#42079

Previously, EvalSymlinks returned an error when called with the root of
a UNC share (e.g. \\server\share). This was due to Windows's
FindFirstFile function not supporting a share root path.

To resolve this, now return early from toNorm in the case where the path
after the volume name is empty. Skipping the later path component
resolution shouldn't have any negative impact in this case, as if the
path is empty, there aren't any path components to resolve anyways.
kevpar added a commit to kevpar/go that referenced this issue Oct 20, 2020
Fixes golang#42079

Previously, EvalSymlinks returned an error when called with the root of
a UNC share (e.g. \\server\share). This was due to Windows's
FindFirstFile function not supporting a share root path.

To resolve this, now return early from toNorm in the case where the path
after the volume name is empty. Skipping the later path component
resolution shouldn't have any negative impact in this case, as if the
path is empty, there aren't any path components to resolve anyways.

The test case uses the localhost admin share (c$), as it should be
present in most situations. This allows testing without setting up an
external file share. However, this fix applies to all UNC share root
paths.
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/263917 mentions this issue: path/filepath: allow EvalSymlinks to work on UNC share roots on Windows

@cagedmantis cagedmantis added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Windows labels Oct 20, 2020
@cagedmantis cagedmantis added this to the Backlog milestone Oct 20, 2020
@kevpar
Copy link
Contributor Author

kevpar commented Oct 20, 2020

I got the tests to work, and have a fix PR out now.

#42096

@networkimprov
Copy link

EvalSymlinks is hopelessly broken on Windows (documented within #40180), a replacement in in the works: #37113.

And I believe there's an open issue for this specific problem.

@golang golang locked and limited conversation to collaborators Oct 25, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Windows
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants