Skip to content

path/filepath: EvalSymlinks failure #21511

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

Closed
alexbrainman opened this issue Aug 18, 2017 · 12 comments
Closed

path/filepath: EvalSymlinks failure #21511

alexbrainman opened this issue Aug 18, 2017 · 12 comments

Comments

@alexbrainman
Copy link
Member

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

go version devel +78984d3954 Wed Aug 16 22:22:19 2017 +0000 windows/amd64

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

set GOARCH=amd64
set GOBIN=
set GOEXE=.exe
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOOS=windows
set GOPATH=c:\users\alex\dev
set GORACE=
set GOROOT=c:\users\alex\dev\go
set GOTOOLDIR=c:\users\alex\dev\go\pkg\tool\windows_amd64
set GCCGO=gccgo
set CC=gcc
set GOGCCFLAGS=-m64 -mthreads -fmessage-length=0 -fdebug-prefix-map=C:\Users\Alex\AppData\Local\Temp\go-build996896503=/tmp/go-build -gno-record-gcc-switches
set CXX=g++
set CGO_ENABLED=1
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

What did you do?

c:\Users\Alex\dev\src\a2>type path_test.go
package filepath_test

import (
        "io/ioutil"
        "os"
        "path/filepath"
        "testing"
)

// simpleJoin builds a file name from the directory and path.
// It does not use Join because we don't want ".." to be evaluated.
func simpleJoin(dir, path string) string {
        return dir + string(filepath.Separator) + path
}

func TestEvalSymlink(t *testing.T) {
        tmpDir, err := ioutil.TempDir("", "TestEvalSymlink")
        if err != nil {
                t.Fatal(err)
        }
        defer os.RemoveAll(tmpDir)

        err = os.MkdirAll(simpleJoin(tmpDir, "test/dir"), 0755)
        if err != nil {
                t.Fatal(err)
        }
        err = os.Symlink("../test", simpleJoin(tmpDir, "test/link1"))
        if err != nil {
                t.Fatal(err)
        }

        cwd, err := os.Getwd()
        if err != nil {
                t.Fatal(err)
        }
        defer func() {
                err := os.Chdir(cwd)
                if err != nil {
                        t.Fatal(err)
                }
        }()

        wd := simpleJoin(tmpDir, "test/link1")
        err = os.Chdir(wd)
        if err != nil {
                t.Fatal(err)
        }

        path := simpleJoin("..", "test")
        have, err := filepath.EvalSymlinks(path)
        if err != nil {
                t.Errorf("EvalSymlinks(%q) in %q directory error: %v", path, wd, err)
                return
        }
        want := simpleJoin("..", "test")
        if filepath.Clean(have) != filepath.Clean(want) {
                t.Errorf("EvalSymlinks(%q) in %q directory returns %q, want %q", path, wd, have, want)
        }
}

c:\Users\Alex\dev\src\a2>go test
--- FAIL: TestEvalSymlink (0.00s)
        path_test.go:52: EvalSymlinks("..\\test") in "C:\\Users\\Alex\\AppData\\Local\\Temp\\TestEvalSymlink279508299\\test/link1" directory error: GetFileAttributesEx ..\test: The system cannot find the file specified.
FAIL
exit status 1
FAIL    a2      0.035s

c:\Users\Alex\dev\src\a2>

What did you expect to see?

I expect test to PASS

What did you see instead?

The test fails.

Alex

@hirochachacha
Copy link
Contributor

(Sorry, I was wrong. I deleted the comment.)

@alexbrainman
Copy link
Member Author

I think there are many more similar bugs. That just goes to show we should use Windows APIs as much as we can, instead of guessing ourself.

Alex

@hirochachacha
Copy link
Contributor

If I am not confusing, I can reproduce by inserting following code before EvalSymlinks check.

_, err = os.Stat(path)
if err != nil {
  t.Fatal(err)
}

So, the problem is coming from os.Lstat, os.Stat?

@alexbrainman
Copy link
Member Author

So, the problem is coming from os.Lstat, os.Stat?

I do not understand your question. This issue is that filepath.EvalSymlinks fails under certain conditions. On Windows only - the program above passes on Linux. I think it should pass on Windows too.

Alex

@hirochachacha
Copy link
Contributor

Please, apply following patch and see what wil happen.

diff --git a/filepath_test.go b/filepath2_test.go
index b859cd6..49cfa05 100644
--- a/filepath_test.go
+++ b/filepath2_test.go
@@ -44,10 +44,14 @@ func TestEvalSymlink(t *testing.T) {
        err = os.Chdir(wd)
        if err != nil {
                t.Fatal(err)
-               return
        }

        path := simpleJoin("..", "test")
+       _, err = os.Stat(path)
+       if err != nil {
+               t.Fatal(err)
+       }
+
        have, err := filepath.EvalSymlinks(path)
        if err != nil {
                t.Errorf("EvalSymlinks(%q) in %q directory error: %v", path, wd, err)

@hirochachacha
Copy link
Contributor

I think there are many more similar bugs. That just goes to show we should use Windows APIs as much as we can, instead of guessing ourself.

I feel that we are failing even in low levels, few guesses.

@hirochachacha
Copy link
Contributor

I feel that we are failing even in low levels, few guesses.

I mean, windows is too difficult to me.

@alexbrainman
Copy link
Member Author

I mean, windows is too difficult to me.

:-}

No worries. We will fix it regardless.

Alex

@alexbrainman
Copy link
Member Author

I just had some time to investigate this. And I can see why tests behave differently on Linux and Windows. The problem, I think, is actually os.Getwd on Linux. Linux os.Getwd follows symlink. If I run this:

package filepath_test

import (
	"io/ioutil"
	"os"
	"path/filepath"
	"testing"
)

// simpleJoin builds a file name from the directory and path.
// It does not use Join because we don't want ".." to be evaluated.
func simpleJoin(dir, path string) string {
	return dir + string(filepath.Separator) + path
}

func TestEvalSymlink(t *testing.T) {
	tmpDir, err := ioutil.TempDir("", "TestEvalSymlink")
	if err != nil {
		t.Fatal(err)
	}
	defer os.RemoveAll(tmpDir)

	err = os.MkdirAll(simpleJoin(tmpDir, "test/dir"), 0755)
	if err != nil {
		t.Fatal(err)
	}
	err = os.Symlink("../test", simpleJoin(tmpDir, "test/link1"))
	if err != nil {
		t.Fatal(err)
	}

	err = os.Chdir(simpleJoin(tmpDir, "test/link1"))
	if err != nil {
		t.Fatal(err)
	}

	cwd, err := os.Getwd()
	if err != nil {
		t.Fatal(err)
	}

	t.Logf("%v", cwd)
}

I get

=== RUN   TestEvalSymlink
--- PASS: TestEvalSymlink (0.00s)
        path_test.go:42: /tmp/TestEvalSymlink946037555/test
PASS

on Linux and

=== RUN   TestEvalSymlink
--- PASS: TestEvalSymlink (0.01s)
        path_test.go:42: C:\Users\Alex\AppData\Local\Temp\TestEvalSymlink144778175\test\link1
PASS

on Windows.

In both we change directory to test/link1 (which is a link), but after the change, we end-up in test on Linux (target of the link), and we remain in test/link1 on Windows.

But os.Getwd is documented to work this way: "... If the current directory can be reached via multiple paths (due to symbolic links), Getwd may return any one of them. ...".

So this is working as expected. Closing. Sorry for confusion.

Alex

@hirochachacha
Copy link
Contributor

wait, where does os.Getwd come from?

@alexbrainman
Copy link
Member Author

wait, where does os.Getwd come from?

It is really os.Chdir that follows symlink, but the effect is documented in os.Getwd.

Alex

@hirochachacha
Copy link
Contributor

I changed to call syscall.CreateFile and it didn't work. So I'm convinced.
Thank you for your response.

@golang golang locked and limited conversation to collaborators Sep 8, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants