fs: Fix parsing of paths under Windows - fixes #2353

Before this copyto would parse windows paths incorrectly.

This change moves the parsing code into fspath and makes sure
fspath.Split calls fspath.Parse which does the parsing correctly for

This also renames fspath.RemoteParse to fspath.Parse for consistency

cmd/cmd.go

@@ -245,7 +245,7 @@ func NewFsSrcDstFiles(args []string) (fsrc fs.Fs, srcFileName string, fdst fs.Fs
 	// If file exists then srcFileName != "", however if the file
 	// doesn't exist then we assume it is a directory...
 	if srcFileName != "" {
-		dstRemote, dstFileName = fspath.RemoteSplit(dstRemote)
+		dstRemote, dstFileName = fspath.Split(dstRemote)
 		if dstRemote == "" {
 			dstRemote = "."
 		}
@@ -268,7 +268,7 @@ func NewFsSrcDstFiles(args []string) (fsrc fs.Fs, srcFileName string, fdst fs.Fs
 
 // NewFsDstFile creates a new dst fs with a destination file name from the arguments
 func NewFsDstFile(args []string) (fdst fs.Fs, dstFileName string) {
-	dstRemote, dstFileName := fspath.RemoteSplit(args[0])
+	dstRemote, dstFileName := fspath.Split(args[0])
 	if dstRemote == "" {
 		dstRemote = "."
 	}

fs/config/config.go

@@ -30,6 +30,7 @@ import (
 	"github.com/ncw/rclone/fs/config/obscure"
 	"github.com/ncw/rclone/fs/driveletter"
 	"github.com/ncw/rclone/fs/fshttp"
+	"github.com/ncw/rclone/fs/fspath"
 	"github.com/pkg/errors"
 	"golang.org/x/crypto/nacl/secretbox"
 	"golang.org/x/text/unicode/norm"
@@ -865,12 +866,12 @@ func NewRemoteName() (name string) {
 	for {
 		fmt.Printf("name> ")
 		name = ReadLine()
-		parts := fs.Matcher.FindStringSubmatch(name + ":")
+		parts := fspath.Matcher.FindStringSubmatch(name + ":")
 		switch {
 		case name == "":
 			fmt.Printf("Can't use empty name.\n")
 		case driveletter.IsDriveLetter(name):
-			fmt.Printf("Can't use %q as it can be confused a drive letter.\n", name)
+			fmt.Printf("Can't use %q as it can be confused with a drive letter.\n", name)
 		case parts == nil:
 			fmt.Printf("Can't use %q as it has invalid characters in it.\n", name)
 		default:

fs/fs.go

@@ -9,12 +9,11 @@ import (
 	"os"
 	"path/filepath"
 	"reflect"
-	"regexp"
 	"sort"
 	"strings"
 	"time"
 
-	"github.com/ncw/rclone/fs/driveletter"
+	"github.com/ncw/rclone/fs/fspath"
 	"github.com/ncw/rclone/fs/hash"
 	"github.com/pkg/errors"
 )
@@ -786,24 +785,20 @@ func MustFind(name string) *RegInfo {
 	return fs
 }
 
-// Matcher is a pattern to match an rclone URL
-var Matcher = regexp.MustCompile(`^([\w_ -]+):(.*)$`)
-
 // ParseRemote deconstructs a path into configName, fsPath, looking up
 // the fsName in the config file (returning NotFoundInConfigFile if not found)
 func ParseRemote(path string) (fsInfo *RegInfo, configName, fsPath string, err error) {
-	parts := Matcher.FindStringSubmatch(path)
+	configName, fsPath = fspath.Parse(path)
 	var fsName string
-	fsName, configName, fsPath = "local", "local", path
-	if parts != nil && !driveletter.IsDriveLetter(parts[1]) {
-		configName, fsPath = parts[1], parts[2]
+	if configName != "" {
 		fsName = ConfigFileGet(configName, "type")
 		if fsName == "" {
 			return nil, "", "", ErrorNotFoundInConfigFile
 		}
+	} else {
+		fsName = "local"
+		configName = "local"
 	}
-	// change native directory separators to / if there are any
-	fsPath = filepath.ToSlash(fsPath)
 	fsInfo, err = Find(fsName)
 	return fsInfo, configName, fsPath, err
 }

fs/fspath/path.go

@@ -3,27 +3,46 @@ package fspath
 
 import (
 	"path"
-	"strings"
+	"path/filepath"
+	"regexp"
+
+	"github.com/ncw/rclone/fs/driveletter"
 )
 
-// RemoteSplit splits a remote into a parent and a leaf
+// Matcher is a pattern to match an rclone URL
+var Matcher = regexp.MustCompile(`^([\w_ -]+):(.*)$`)
+
+// Parse deconstructs a remote path into configName and fsPath
+//
+// If the path is a local path then configName will be returned as "".
+//
+// So "remote:path/to/dir" will return "remote", "path/to/dir"
+// and "/path/to/local" will return ("", "/path/to/local")
+//
+// Note that this will turn \ into / in the fsPath on Windows
+func Parse(path string) (configName, fsPath string) {
+	parts := Matcher.FindStringSubmatch(path)
+	configName, fsPath = "", path
+	if parts != nil && !driveletter.IsDriveLetter(parts[1]) {
+		configName, fsPath = parts[1], parts[2]
+	}
+	// change native directory separators to / if there are any
+	fsPath = filepath.ToSlash(fsPath)
+	return configName, fsPath
+}
+
+// Split splits a remote into a parent and a leaf
 //
 // if it returns leaf as an empty string then remote is a directory
 //
 // if it returns parent as an empty string then that means the current directory
 //
 // The returned values have the property that parent + leaf == remote
-func RemoteSplit(remote string) (parent string, leaf string) {
-	// Split remote on :
-	i := strings.Index(remote, ":")
-	remoteName := ""
-	remotePath := remote
-	if i >= 0 {
-		remoteName = remote[:i+1]
-		remotePath = remote[i+1:]
-	} else if strings.HasSuffix(remotePath, "/") {
-		// if no : and ends with / must be directory
-		return remotePath, ""
+// (except under Windows where \ will be translated into /)
+func Split(remote string) (parent string, leaf string) {
+	remoteName, remotePath := Parse(remote)
+	if remoteName != "" {
+		remoteName += ":"
 	}
 	// Construct new remote name without last segment
 	parent, leaf = path.Split(remotePath)

fs/fspath/path_test.go

@@ -7,8 +7,23 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-func TestRemoteSplit(t *testing.T) {
+func TestParse(t *testing.T) {
+	for _, test := range []struct {
+		in, wantConfigName, wantFsPath string
+	}{
+		{"", "", ""},
+		{"/path/to/file", "", "/path/to/file"},
+		{"path/to/file", "", "path/to/file"},
+		{"remote:path/to/file", "remote", "path/to/file"},
+		{"remote:/path/to/file", "remote", "/path/to/file"},
+	} {
+		gotConfigName, gotFsPath := Parse(test.in)
+		assert.Equal(t, test.wantConfigName, gotConfigName)
+		assert.Equal(t, test.wantFsPath, gotFsPath)
+	}
+}
 
+func TestSplit(t *testing.T) {
 	for _, test := range []struct {
 		remote, wantParent, wantLeaf string
 	}{
@@ -27,7 +42,7 @@ func TestRemoteSplit(t *testing.T) {
 		{"root/", "root/", ""},
 		{"a/b/", "a/b/", ""},
 	} {
-		gotParent, gotLeaf := RemoteSplit(test.remote)
+		gotParent, gotLeaf := Split(test.remote)
 		assert.Equal(t, test.wantParent, gotParent, test.remote)
 		assert.Equal(t, test.wantLeaf, gotLeaf, test.remote)
 		assert.Equal(t, test.remote, gotParent+gotLeaf, fmt.Sprintf("%s: %q + %q != %q", test.remote, gotParent, gotLeaf, test.remote))