path/filepath: do not call GetFinalPathNameByHandle from EvalSymlinks

EvalSymlinks is using GetFinalPathNameByHandle to handle symlinks with
unusual targets like \??\Volume{ABCD}\. But since CL 164201, os.Readlink
handles path like that too.

So remove all that extra code that EvalSymlinks calls when os.Readlink
fails - it is not needed any more.

Now that windows EvalSymlinks implementation is similar to unix
implementation, we can remove all slashAfterFilePathError related code
too. So do that.

This also makes TestIssue29372 pass even when TMP directory refers to
symlinks with target like \??\Volume{ABCD}\. So remove TestIssue29372
code that helped it pass on windows-arm. TestIssue29372 should pass as
is now.

Fixes #29746

Change-Id: I568d142c89d3297bff8513069bceaa6be51fe7e4
Reviewed-on: https://go-review.googlesource.com/c/164202
Run-TryBot: Alex Brainman <alex.brainman@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
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 7a434a429249354ead0022039e3030e7b8bc614a..709dccb61b7acca0d9376d8b988f466cbb3aab9e 100644 (file)
--- a/src/path/filepath/path_test.go
+++ b/src/path/filepath/path_test.go
@@ -1377,16 +1377,6 @@ func TestIssue29372(t *testing.T) {
        }
        defer os.RemoveAll(tmpDir)
 
-       if runtime.GOOS == "windows" {
-               // This test is broken on windows, if temporary directory
-               // is a symlink. See issue 29746.
-               // TODO(brainman): Remove this hack once issue #29746 is fixed.
-               tmpDir, err = filepath.EvalSymlinks(tmpDir)
-               if err != nil {
-                       t.Fatal(err)
-               }
-       }
-
        path := filepath.Join(tmpDir, "file.txt")
        err = ioutil.WriteFile(path, nil, 0644)
        if err != nil {

diff --git a/src/path/filepath/path_windows_test.go b/src/path/filepath/path_windows_test.go
index d1735d39bd8ccfb69a4b5bd706a99abbca7db8b4..f7c454bf65f7fc851cab079e14e4a07389fddedf 100644 (file)
--- a/src/path/filepath/path_windows_test.go
+++ b/src/path/filepath/path_windows_test.go
@@ -529,6 +529,12 @@ func TestNTNamespaceSymlink(t *testing.T) {
        }
        defer os.RemoveAll(tmpdir)
 
+       // Make sure tmpdir is not a symlink, otherwise tests will fail.
+       tmpdir, err = filepath.EvalSymlinks(tmpdir)
+       if err != nil {
+               t.Fatal(err)
+       }
+
        vol := filepath.VolumeName(tmpdir)
        output, err = exec.Command("cmd", "/c", "mountvol", vol, "/L").CombinedOutput()
        if err != nil {

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

diff --git a/src/path/filepath/symlink_unix.go b/src/path/filepath/symlink_unix.go
index b57e7f2277e3e9915f90022439cdfe64e38c2fb1..d20e63a987e9ad7a9986c5ecc05b4db5f92beb74 100644 (file)
--- a/src/path/filepath/symlink_unix.go
+++ b/src/path/filepath/symlink_unix.go
@@ -2,15 +2,6 @@
 
 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 531dc26fc0e4f2366026acc66ab1479d226b314e..a799488c1824019363d64c0bbd87a11244157be8 100644 (file)
--- a/src/path/filepath/symlink_windows.go
+++ b/src/path/filepath/symlink_windows.go
@@ -5,9 +5,6 @@
 package filepath
 
 import (
-       "errors"
-       "internal/syscall/windows"
-       "os"
        "strings"
        "syscall"
 )
@@ -109,108 +106,14 @@ func toNorm(path string, normBase func(string) (string, error)) (string, error)
        return volume + normPath, nil
 }
 
-// evalSymlinksUsingGetFinalPathNameByHandle uses Windows
-// GetFinalPathNameByHandle API to retrieve the final
-// path for the specified file.
-func evalSymlinksUsingGetFinalPathNameByHandle(path string) (string, error) {
-       err := windows.LoadGetFinalPathNameByHandle()
-       if err != nil {
-               // we must be using old version of Windows
-               return "", err
-       }
-
-       if path == "" {
-               return path, nil
-       }
-
-       // Use Windows I/O manager to dereference the symbolic link, as per
-       // https://blogs.msdn.microsoft.com/oldnewthing/20100212-00/?p=14963/
-       p, err := syscall.UTF16PtrFromString(path)
-       if err != nil {
-               return "", err
-       }
-       h, err := syscall.CreateFile(p, 0, 0, nil,
-               syscall.OPEN_EXISTING, syscall.FILE_FLAG_BACKUP_SEMANTICS, 0)
-       if err != nil {
-               return "", err
-       }
-       defer syscall.CloseHandle(h)
-
-       buf := make([]uint16, 100)
-       for {
-               n, err := windows.GetFinalPathNameByHandle(h, &buf[0], uint32(len(buf)), windows.VOLUME_NAME_DOS)
-               if err != nil {
-                       return "", err
-               }
-               if n < uint32(len(buf)) {
-                       break
-               }
-               buf = make([]uint16, n)
-       }
-       s := syscall.UTF16ToString(buf)
-       if len(s) > 4 && s[:4] == `\\?\` {
-               s = s[4:]
-               if len(s) > 3 && s[:3] == `UNC` {
-                       // return path like \\server\share\...
-                       return `\` + s[3:], nil
-               }
-               return s, nil
-       }
-       return "", errors.New("GetFinalPathNameByHandle returned unexpected path=" + s)
-}
-
-func samefile(path1, path2 string) bool {
-       fi1, err := os.Lstat(path1)
-       if err != nil {
-               return false
-       }
-       fi2, err := os.Lstat(path2)
-       if err != nil {
-               return false
-       }
-       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 {
-                       return toNorm(newpath2, normBase)
-               }
                return "", err
        }
        newpath, err = toNorm(newpath, normBase)
        if err != nil {
-               newpath2, err2 := evalSymlinksUsingGetFinalPathNameByHandle(path)
-               if err2 == nil {
-                       return toNorm(newpath2, normBase)
-               }
                return "", err
        }
-       if strings.ToUpper(newpath) == strings.ToUpper(path) {
-               // walkSymlinks did not actually walk any symlinks,
-               // so we don't need to try GetFinalPathNameByHandle.
-               return newpath, nil
-       }
-       newpath2, err2 := evalSymlinksUsingGetFinalPathNameByHandle(path)
-       if err2 != nil {
-               return newpath, nil
-       }
-       newpath2, err2 = toNorm(newpath2, normBase)
-       if err2 != nil {
-               return newpath, nil
-       }
-       if samefile(newpath, newpath2) {
-               return newpath, nil
-       }
-       return newpath2, nil
+       return newpath, nil
 }