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>

5 files changed, 9 insertions, 118 deletions

diff --git a/src/path/filepath/path_test.go b/src/path/filepath/path_test.go
index 7a434a4292..709dccb61b 100644
--- 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 d1735d39bd..f7c454bf65 100644
--- 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 4b41039e25..a08b85a29c 100644
--- 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 b57e7f2277..d20e63a987 100644
--- 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 531dc26fc0..a799488c18 100644
--- 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
 }