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

@@ -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:], "/"))
+	filePath = filepath.ToSlash(filePath)
+	filePath = makeAbsolute(filePath)
+	if strings.HasPrefix(remote, "//") {
+		return "/" + path.Join(remote, filePath)
 	}
-			return path.Clean(strings.Join(es[i:], "/"))
+	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
 }

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