From ea8d13d841d28c5e313d439db1117017c52ade13 Mon Sep 17 00:00:00 2001
From: Nick Craig-Wood <nick@craig-wood.com>
Date: Sun, 13 Dec 2020 10:26:13 +0000
Subject: [PATCH] fs: Fix parsing of .. when joining remotes - Fixes #4862

Before this fix setting an alias of `s3:bucket` then using `alias:..`
would use the current working directory!

This fix corrects the path parsing. This parsing is also used in
wrapping backends like crypt, chunker, union etc.

It does not allow looking above the root of the alias, so `alias:..`
now lists `s3:bucket` as you might expect if you did `cd /` then
`ls ..`.
---
 fs/fspath/path.go      | 52 ++++++++++++++++++--------
 fs/fspath/path_test.go | 83 +++++++++++++++++++++++++++++-------------
 2 files changed, 95 insertions(+), 40 deletions(-)

diff --git a/fs/fspath/path.go b/fs/fspath/path.go
index a07b7ceea..a6a4843a4 100644
--- a/fs/fspath/path.go
+++ b/fs/fspath/path.go
@@ -102,25 +102,47 @@ func Split(remote string) (parent string, leaf string, err error) {
 	return remoteName + parent, leaf, nil
 }
 
-// JoinRootPath joins any number of path elements into a single path, adding a
-// separating slash if necessary. The result is Cleaned; in particular,
-// all empty strings are ignored.
+// Make filePath absolute so it can't read above the root
+func makeAbsolute(filePath string) string {
+	leadingSlash := strings.HasPrefix(filePath, "/")
+	filePath = path.Join("/", filePath)
+	if !leadingSlash && strings.HasPrefix(filePath, "/") {
+		filePath = filePath[1:]
+	}
+	return filePath
+}
+
+// JoinRootPath joins filePath onto remote
 //
-// If the first non empty element has a leading "//" this is preserved.
+// If the remote has a leading "//" this is preserved to allow Windows
+// network paths to be used as remotes.
+//
+// If filePath is empty then remote will be returned.
 //
 // If the path contains \ these will be converted to / on Windows.
-func JoinRootPath(elem ...string) string {
-	es := make([]string, len(elem))
-	for i := range es {
-		es[i] = filepath.ToSlash(elem[i])
+func JoinRootPath(remote, filePath string) string {
+	remote = filepath.ToSlash(remote)
+	if filePath == "" {
+		return remote
 	}
-	for i, e := range es {
-		if e != "" {
-			if strings.HasPrefix(e, "//") {
-				return "/" + path.Clean(strings.Join(es[i:], "/"))
-			}
-			return path.Clean(strings.Join(es[i:], "/"))
+	filePath = filepath.ToSlash(filePath)
+	filePath = makeAbsolute(filePath)
+	if strings.HasPrefix(remote, "//") {
+		return "/" + path.Join(remote, filePath)
+	}
+	remoteName, remotePath, err := Parse(remote)
+	if err != nil {
+		// Couldn't parse so assume it is a path
+		remoteName = ""
+		remotePath = remote
+	}
+	remotePath = path.Join(remotePath, filePath)
+	if remoteName != "" {
+		remoteName += ":"
+		// if have remote: then normalise the remotePath
+		if remotePath == "." {
+			remotePath = ""
 		}
 	}
-	return ""
+	return remoteName + remotePath
 }
diff --git a/fs/fspath/path_test.go b/fs/fspath/path_test.go
index c07379030..39a3058d6 100644
--- a/fs/fspath/path_test.go
+++ b/fs/fspath/path_test.go
@@ -130,33 +130,66 @@ func TestSplit(t *testing.T) {
 		}
 	}
 }
-func TestJoinRootPath(t *testing.T) {
+
+func TestMakeAbsolute(t *testing.T) {
 	for _, test := range []struct {
-		elements []string
-		want     string
+		in   string
+		want string
 	}{
-		{nil, ""},
-		{[]string{""}, ""},
-		{[]string{"/"}, "/"},
-		{[]string{"/", "/"}, "/"},
-		{[]string{"/", "//"}, "/"},
-		{[]string{"/root", ""}, "/root"},
-		{[]string{"/root", "/"}, "/root"},
-		{[]string{"/root", "//"}, "/root"},
-		{[]string{"/a/b"}, "/a/b"},
-		{[]string{"//", "/"}, "//"},
-		{[]string{"//server", "path"}, "//server/path"},
-		{[]string{"//server/sub", "path"}, "//server/sub/path"},
-		{[]string{"//server", "//path"}, "//server/path"},
-		{[]string{"//server/sub", "//path"}, "//server/sub/path"},
-		{[]string{"", "//", "/"}, "//"},
-		{[]string{"", "//server", "path"}, "//server/path"},
-		{[]string{"", "//server/sub", "path"}, "//server/sub/path"},
-		{[]string{"", "//server", "//path"}, "//server/path"},
-		{[]string{"", "//server/sub", "//path"}, "//server/sub/path"},
-		{[]string{"", filepath.FromSlash("//server/sub"), filepath.FromSlash("//path")}, "//server/sub/path"},
+		{"", ""},
+		{".", ""},
+		{"/.", "/"},
+		{"../potato", "potato"},
+		{"/../potato", "/potato"},
+		{"./../potato", "potato"},
+		{"//../potato", "/potato"},
+		{"././../potato", "potato"},
+		{"././potato/../../onion", "onion"},
 	} {
-		got := JoinRootPath(test.elements...)
-		assert.Equal(t, test.want, got)
+		got := makeAbsolute(test.in)
+		assert.Equal(t, test.want, got, test)
+	}
+}
+
+func TestJoinRootPath(t *testing.T) {
+	for _, test := range []struct {
+		remote   string
+		filePath string
+		want     string
+	}{
+		{"", "", ""},
+		{"", "/", "/"},
+		{"/", "", "/"},
+		{"/", "/", "/"},
+		{"/", "//", "/"},
+		{"/root", "", "/root"},
+		{"/root", "/", "/root"},
+		{"/root", "//", "/root"},
+		{"/a/b", "", "/a/b"},
+		{"//", "/", "//"},
+		{"//server", "path", "//server/path"},
+		{"//server/sub", "path", "//server/sub/path"},
+		{"//server", "//path", "//server/path"},
+		{"//server/sub", "//path", "//server/sub/path"},
+		{"//", "/", "//"},
+		{"//server", "path", "//server/path"},
+		{"//server/sub", "path", "//server/sub/path"},
+		{"//server", "//path", "//server/path"},
+		{"//server/sub", "//path", "//server/sub/path"},
+		{filepath.FromSlash("//server/sub"), filepath.FromSlash("//path"), "//server/sub/path"},
+		{"s3:", "", "s3:"},
+		{"s3:", ".", "s3:"},
+		{"s3:.", ".", "s3:"},
+		{"s3:", "..", "s3:"},
+		{"s3:dir", "sub", "s3:dir/sub"},
+		{"s3:dir", "/sub", "s3:dir/sub"},
+		{"s3:dir", "./sub", "s3:dir/sub"},
+		{"s3:/dir", "/sub/", "s3:/dir/sub"},
+		{"s3:dir", "..", "s3:dir"},
+		{"s3:dir", "/..", "s3:dir"},
+		{"s3:dir", "/../", "s3:dir"},
+	} {
+		got := JoinRootPath(test.remote, test.filePath)
+		assert.Equal(t, test.want, got, test)
 	}
 }