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 <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Ian Lance Taylor <[email protected]>

Author: alexbrainman

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 {

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 {

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
 		}

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)
 }

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
 }