path/filepath: rewrite walkSymlinks

Rather than try to work around Clean and Join on intermediate steps,
which can remove ".." components unexpectedly, just do everything in
walkSymlinks. Use a single loop over path components.

Fixes #23444

Change-Id: I4f15e50d0df32349cc4fd55e3d224ec9ab064379
Reviewed-on: https://go-review.googlesource.com/121676
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>

src/path/filepath/path_test.go

@@ -771,6 +771,18 @@ var EvalSymlinksTestDirs = []EvalSymlinksTest{
 	{"test/link1", "../test"},
 	{"test/link2", "dir"},
 	{"test/linkabs", "/"},
+	{"test/link4", "../test2"},
+	{"test2", "test/dir"},
+	// Issue 23444.
+	{"src", ""},
+	{"src/pool", ""},
+	{"src/pool/test", ""},
+	{"src/versions", ""},
+	{"src/versions/current", "../../version"},
+	{"src/versions/v1", ""},
+	{"src/versions/v1/modules", ""},
+	{"src/versions/v1/modules/test", "../../../pool/test"},
+	{"version", "src/versions/v1"},
 }
 
 var EvalSymlinksTests = []EvalSymlinksTest{
@@ -784,6 +796,8 @@ var EvalSymlinksTests = []EvalSymlinksTest{
 	{"test/dir/link3", "."},
 	{"test/link2/link3/test", "test"},
 	{"test/linkabs", "/"},
+	{"test/link4/..", "test"},
+	{"src/versions/current/modules/test", "src/pool/test"},
 }
 
 // simpleJoin builds a file name from the directory and path.

src/path/filepath/symlink.go

@@ -10,109 +10,125 @@ import (
 	"runtime"
 )
 
-// isRoot returns true if path is root of file system
-// (`/` on unix and `/`, `\`, `c:\` or `c:/` on windows).
-func isRoot(path string) bool {
-	if runtime.GOOS != "windows" {
-		return path == "/"
-	}
-	switch len(path) {
-	case 1:
-		return os.IsPathSeparator(path[0])
-	case 3:
-		return path[1] == ':' && os.IsPathSeparator(path[2])
+func walkSymlinks(path string) (string, error) {
+	volLen := volumeNameLen(path)
+	if volLen < len(path) && os.IsPathSeparator(path[volLen]) {
+		volLen++
 	}
-	return false
-}
+	vol := path[:volLen]
+	dest := vol
+	linksWalked := 0
+	for start, end := volLen, volLen; start < len(path); start = end {
+		for start < len(path) && os.IsPathSeparator(path[start]) {
+			start++
+		}
+		end = start
+		for end < len(path) && !os.IsPathSeparator(path[end]) {
+			end++
+		}
 
-// isDriveLetter returns true if path is Windows drive letter (like "c:").
-func isDriveLetter(path string) bool {
-	if runtime.GOOS != "windows" {
-		return false
-	}
-	return len(path) == 2 && path[1] == ':'
-}
+		// On Windows, "." can be a symlink.
+		// We look it up, and use the value if it is absolute.
+		// If not, we just return ".".
+		isWindowsDot := runtime.GOOS == "windows" && path[volumeNameLen(path):] == "."
 
-func walkLink(path string, linksWalked *int) (newpath string, islink bool, err error) {
-	if *linksWalked > 255 {
-		return "", false, errors.New("EvalSymlinks: too many links")
-	}
-	fi, err := os.Lstat(path)
-	if err != nil {
-		return "", false, err
-	}
-	if fi.Mode()&os.ModeSymlink == 0 {
-		return path, false, nil
-	}
-	newpath, err = os.Readlink(path)
-	if err != nil {
-		return "", false, err
-	}
-	*linksWalked++
-	return newpath, true, nil
-}
-
-func walkLinks(path string, linksWalked *int) (string, error) {
-	switch dir, file := Split(path); {
-	case dir == "":
-		newpath, _, err := walkLink(file, linksWalked)
-		return newpath, err
-	case file == "":
-		if isDriveLetter(dir) {
-			return dir, nil
-		}
-		if os.IsPathSeparator(dir[len(dir)-1]) {
-			if isRoot(dir) {
-				return dir, nil
+		// The next path component is in path[start:end].
+		if end == start {
+			// No more path components.
+			break
+		} else if path[start:end] == "." && !isWindowsDot {
+			// Ignore path component ".".
+			continue
+		} else if path[start:end] == ".." {
+			// Back up to previous component if possible.
+			var r int
+			for r = len(dest) - 1; r >= 0; r-- {
+				if os.IsPathSeparator(dest[r]) {
+					break
+				}
+			}
+			if r < 0 {
+				if len(dest) > 0 {
+					dest += string(os.PathSeparator)
+				}
+				dest += ".."
+			} else {
+				dest = dest[:r]
 			}
-			return walkLinks(dir[:len(dir)-1], linksWalked)
+			continue
 		}
-		newpath, _, err := walkLink(dir, linksWalked)
-		return newpath, err
-	default:
-		newdir, err := walkLinks(dir, linksWalked)
-		if err != nil {
-			return "", err
+
+		// Ordinary path component. Add it to result.
+
+		if len(dest) > volumeNameLen(dest) && !os.IsPathSeparator(dest[len(dest)-1]) {
+			dest += string(os.PathSeparator)
 		}
-		newpath, islink, err := walkLink(Join(newdir, file), linksWalked)
+
+		dest += path[start:end]
+
+		// Resolve symlink.
+
+		fi, err := os.Lstat(dest)
 		if err != nil {
 			return "", err
 		}
-		if !islink {
-			return newpath, nil
+
+		if fi.Mode()&os.ModeSymlink == 0 {
+			if !fi.Mode().IsDir() && end < len(path) {
+				return "", os.ErrNotExist
+			}
+			continue
 		}
-		if IsAbs(newpath) || os.IsPathSeparator(newpath[0]) {
-			return newpath, nil
+
+		// Found symlink.
+
+		linksWalked++
+		if linksWalked > 255 {
+			return "", errors.New("EvalSymlinks: too many links")
 		}
-		return Join(newdir, newpath), nil
-	}
-}
 
-func walkSymlinks(path string) (string, error) {
-	if path == "" {
-		return path, nil
-	}
-	var linksWalked int // to protect against cycles
-	for {
-		i := linksWalked
-		newpath, err := walkLinks(path, &linksWalked)
+		link, err := os.Readlink(dest)
 		if err != nil {
 			return "", err
 		}
-		if runtime.GOOS == "windows" {
-			// walkLinks(".", ...) always returns "." on unix.
-			// But on windows it returns symlink target, if current
-			// directory is a symlink. Stop the walk, if symlink
-			// target is not absolute path, and return "."
-			// to the caller (just like unix does).
-			// Same for "C:.".
-			if path[volumeNameLen(path):] == "." && !IsAbs(newpath) {
-				return path, nil
-			}
+
+		if isWindowsDot && !IsAbs(link) {
+			// On Windows, if "." is a relative symlink,
+			// just return ".".
+			break
 		}
-		if i == linksWalked {
-			return Clean(newpath), nil
+
+		path = link + path[end:]
+
+		v := volumeNameLen(link)
+		if v > 0 {
+			// Symlink to drive name is an absolute path.
+			if v < len(link) && os.IsPathSeparator(link[v]) {
+				v++
+			}
+			vol = link[:v]
+			dest = vol
+			end = len(vol)
+		} else if len(link) > 0 && os.IsPathSeparator(link[0]) {
+			// Symlink to absolute path.
+			dest = link[:1]
+			end = 1
+		} else {
+			// Symlink to relative path; replace last
+			// path component in dest.
+			var r int
+			for r = len(dest) - 1; r >= 0; r-- {
+				if os.IsPathSeparator(dest[r]) {
+					break
+				}
+			}
+			if r < 0 {
+				dest = vol
+			} else {
+				dest = dest[:r]
+			}
+			end = 0
 		}
-		path = newpath
 	}
+	return Clean(dest), nil
 }