path/filepath: return special error from EvalSymlinks

CL 155597 attempted to fix #29372. But it failed to make all new
test cases pass. Also CL 155597 broke some existing code
(see #29449 for details).

Make small adjustment to CL 155597 that fixes both #29372 and #29449.

Suggested by Ian.

Updates #29372
Fixes #29449

Change-Id: I9777a615514d3f152af5acb65fb1239e696607b6
Reviewed-on: https://go-review.googlesource.com/c/156398
Run-TryBot: Alex Brainman <alex.brainman@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>

diff --git a/src/path/filepath/path_test.go b/src/path/filepath/path_test.go
index 1b9f286c4d87ad5f1058795700dc0946d54c1970..cbddda88b6693ddf3878993ffd9ea6b1c721a24a 100644 (file)
--- a/src/path/filepath/path_test.go
+++ b/src/path/filepath/path_test.go
@@ -1382,27 +1382,18 @@ func TestIssue29372(t *testing.T) {
        path := f.Name()
        defer os.Remove(path)
 
-       isWin := runtime.GOOS == "windows"
        pathSeparator := string(filepath.Separator)
-       tests := []struct {
-               path string
-               skip bool
-       }{
-               {path + strings.Repeat(pathSeparator, 1), false},
-               {path + strings.Repeat(pathSeparator, 2), false},
-               {path + strings.Repeat(pathSeparator, 1) + ".", false},
-               {path + strings.Repeat(pathSeparator, 2) + ".", false},
-               // windows.GetFinalPathNameByHandle return the directory part with trailing dot dot
-               // C:\path\to\existing_dir\existing_file\.. returns C:\path\to\existing_dir
-               {path + strings.Repeat(pathSeparator, 1) + "..", isWin},
-               {path + strings.Repeat(pathSeparator, 2) + "..", isWin},
+       tests := []string{
+               path + strings.Repeat(pathSeparator, 1),
+               path + strings.Repeat(pathSeparator, 2),
+               path + strings.Repeat(pathSeparator, 1) + ".",
+               path + strings.Repeat(pathSeparator, 2) + ".",
+               path + strings.Repeat(pathSeparator, 1) + "..",
+               path + strings.Repeat(pathSeparator, 2) + "..",
        }
 
        for i, test := range tests {
-               if test.skip {
-                       continue
-               }
-               _, err = filepath.EvalSymlinks(test.path)
+               _, err = filepath.EvalSymlinks(test)
                if err != syscall.ENOTDIR {
                        t.Fatalf("test#%d: want %q, got %q", i, syscall.ENOTDIR, err)
                }

diff --git a/src/path/filepath/path_windows_test.go b/src/path/filepath/path_windows_test.go
index 63eab18116cf5d44213cec00a8471c45309ce1d6..3fcccfab785c6725ed12def644d1e907142aca75 100644 (file)
--- a/src/path/filepath/path_windows_test.go
+++ b/src/path/filepath/path_windows_test.go
@@ -536,17 +536,39 @@ func TestNTNamespaceSymlink(t *testing.T) {
        }
        target := strings.Trim(string(output), " \n\r")
 
-       link := filepath.Join(tmpdir, "link")
-       output, err = exec.Command("cmd", "/c", "mklink", "/J", link, target).CombinedOutput()
+       dirlink := filepath.Join(tmpdir, "dirlink")
+       output, err = exec.Command("cmd", "/c", "mklink", "/J", dirlink, target).CombinedOutput()
        if err != nil {
-               t.Fatalf("failed to run mklink %v %v: %v %q", link, target, err, output)
+               t.Fatalf("failed to run mklink %v %v: %v %q", dirlink, target, err, output)
        }
 
-       got, err := filepath.EvalSymlinks(link)
+       got, err := filepath.EvalSymlinks(dirlink)
        if err != nil {
                t.Fatal(err)
        }
        if want := vol + `\`; got != want {
-               t.Errorf(`EvalSymlinks(%q): got %q, want %q`, link, got, want)
+               t.Errorf(`EvalSymlinks(%q): got %q, want %q`, dirlink, got, want)
+       }
+
+       file := filepath.Join(tmpdir, "file")
+       err = ioutil.WriteFile(file, []byte(""), 0666)
+       if err != nil {
+               t.Fatal(err)
+       }
+
+       target += file[len(filepath.VolumeName(file)):]
+
+       filelink := filepath.Join(tmpdir, "filelink")
+       output, err = exec.Command("cmd", "/c", "mklink", filelink, target).CombinedOutput()
+       if err != nil {
+               t.Fatalf("failed to run mklink %v %v: %v %q", filelink, target, err, output)
+       }
+
+       got, err = filepath.EvalSymlinks(filelink)
+       if err != nil {
+               t.Fatal(err)
+       }
+       if want := file; got != want {
+               t.Errorf(`EvalSymlinks(%q): got %q, want %q`, filelink, got, want)
        }
 }

diff --git a/src/path/filepath/symlink.go b/src/path/filepath/symlink.go
index a08b85a29cdb3c9dc7aea6d95e5088f2f26fb3bb..4b41039e25f7592f6a142e2dc372078f05b980a9 100644 (file)
--- a/src/path/filepath/symlink.go
+++ b/src/path/filepath/symlink.go
@@ -8,7 +8,6 @@ import (
        "errors"
        "os"
        "runtime"
-       "syscall"
 )
 
 func walkSymlinks(path string) (string, error) {
@@ -79,7 +78,7 @@ func walkSymlinks(path string) (string, error) {
 
                if fi.Mode()&os.ModeSymlink == 0 {
                        if !fi.Mode().IsDir() && end < len(path) {
-                               return "", syscall.ENOTDIR
+                               return "", slashAfterFilePathError
                        }
                        continue
                }

diff --git a/src/path/filepath/symlink_unix.go b/src/path/filepath/symlink_unix.go
index d20e63a987e9ad7a9986c5ecc05b4db5f92beb74..b57e7f2277e3e9915f90022439cdfe64e38c2fb1 100644 (file)
--- a/src/path/filepath/symlink_unix.go
+++ b/src/path/filepath/symlink_unix.go
@@ -2,6 +2,15 @@
 
 package filepath
 
+import (
+       "syscall"
+)
+
+// walkSymlinks returns slashAfterFilePathError error for paths like
+// //path/to/existing_file/ and /path/to/existing_file/. and /path/to/existing_file/..
+
+var slashAfterFilePathError = syscall.ENOTDIR
+
 func evalSymlinks(path string) (string, error) {
        return walkSymlinks(path)
 }

diff --git a/src/path/filepath/symlink_windows.go b/src/path/filepath/symlink_windows.go
index 1108b3ddff0bd70ddbbc3f95a8676797c03cf64a..531dc26fc0e4f2366026acc66ab1479d226b314e 100644 (file)
--- a/src/path/filepath/symlink_windows.go
+++ b/src/path/filepath/symlink_windows.go
@@ -159,18 +159,6 @@ func evalSymlinksUsingGetFinalPathNameByHandle(path string) (string, error) {
        return "", errors.New("GetFinalPathNameByHandle returned unexpected path=" + s)
 }
 
-func symlinkOrDir(path string) (string, error) {
-       fi, err := os.Lstat(path)
-       if err != nil {
-               return "", err
-       }
-
-       if fi.Mode()&os.ModeSymlink == 0 && !fi.Mode().IsDir() {
-               return "", syscall.ENOTDIR
-       }
-       return path, nil
-}
-
 func samefile(path1, path2 string) bool {
        fi1, err := os.Lstat(path1)
        if err != nil {
@@ -183,16 +171,20 @@ func samefile(path1, path2 string) bool {
        return os.SameFile(fi1, fi2)
 }
 
+// walkSymlinks returns slashAfterFilePathError error for paths like
+// //path/to/existing_file/ and /path/to/existing_file/. and /path/to/existing_file/..
+
+var slashAfterFilePathError = errors.New("attempting to walk past file path.")
+
 func evalSymlinks(path string) (string, error) {
        newpath, err := walkSymlinks(path)
+       if err == slashAfterFilePathError {
+               return "", syscall.ENOTDIR
+       }
        if err != nil {
                newpath2, err2 := evalSymlinksUsingGetFinalPathNameByHandle(path)
                if err2 == nil {
-                       normPath, toNormErr := toNorm(newpath2, normBase)
-                       if toNormErr != nil {
-                               return "", toNormErr
-                       }
-                       return symlinkOrDir(normPath)
+                       return toNorm(newpath2, normBase)
                }
                return "", err
        }