Cleaned up the sftp parsing logic.

Simplified and cleaned up the sftp parsing logic. Added support to
path.Clean the directory. Added additional tests.

src/restic/backend/sftp/config.go

@@ -3,6 +3,7 @@ package sftp
 import (
 	"errors"
 	"net/url"
+	"path"
 	"strings"
 )
 
@@ -11,58 +12,49 @@ type Config struct {
 	User, Host, Dir string
 }
 
-// ParseConfig extracts all information for the sftp connection from the string s.
+// ParseConfig parses the string s and extracts the sftp config. The
+// supported configuration formats are sftp://user@host/directory
+// (with an optional port sftp://user@host:port/directory) and
+// sftp:user@host:directory.  The directory will be path Cleaned and
+// can be an absolute path if it starts with a '/'
+// (e.g. sftp://user@host//absolute and sftp:user@host:/absolute).
 func ParseConfig(s string) (interface{}, error) {
-	if strings.HasPrefix(s, "sftp://") {
+	var user, host, dir string
-		return parseFormat1(s)
+	switch {
+	case strings.HasPrefix(s, "sftp://"):
+		// parse the "sftp://user@host/path" url format
+		url, err := url.Parse(s)
+		if err != nil {
+			return nil, err
+		}
+		if url.User != nil {
+			user = url.User.Username()
+		}
+		host = url.Host
+		dir = url.Path[1:]
+	case strings.HasPrefix(s, "sftp:"):
+		// parse the sftp:user@host:path format, which means we'll get
+		// "user@host:path" in s
+		s = s[5:]
+		// split user@host and path at the colon
+		data := strings.SplitN(s, ":", 2)
+		if len(data) < 2 {
+			return nil, errors.New("sftp: invalid format, hostname or path not found")
+		}
+		host = data[0]
+		dir = data[1]
+		// split user and host at the "@"
+		data = strings.SplitN(host, "@", 2)
+		if len(data) == 2 {
+			user = data[0]
+			host = data[1]
+		}
+	default:
+		return nil, errors.New(`invalid format, does not start with "sftp:"`)
 	}
-
+	return Config{
-	// otherwise parse in the sftp:user@host:path format, which means we'll get
+		User: user,
-	// "user@host:path" in s
+		Host: host,
-	return parseFormat2(s)
+		Dir:  path.Clean(dir),
-}
+	}, nil
-
-// parseFormat1 parses the first format, starting with a slash, so the user
-// either specified "sftp://host/path", so we'll get everything after the first
-// colon character
-func parseFormat1(s string) (Config, error) {
-	url, err := url.Parse(s)
-	if err != nil {
-		return Config{}, err
-	}
-
-	cfg := Config{
-		Host: url.Host,
-		Dir:  url.Path[1:],
-	}
-	if url.User != nil {
-		cfg.User = url.User.Username()
-	}
-	return cfg, nil
-}
-
-// parseFormat2 parses the second format, sftp:user@host:path
-func parseFormat2(s string) (cfg Config, err error) {
-	// split user/host and path at the second colon
-	data := strings.SplitN(s, ":", 3)
-	if len(data) < 3 {
-		return Config{}, errors.New("sftp: invalid format, hostname or path not found")
-	}
-
-	if data[0] != "sftp" {
-		return Config{}, errors.New(`invalid format, does not start with "sftp:"`)
-	}
-
-	userhost := data[1]
-	cfg.Dir = data[2]
-
-	data = strings.SplitN(userhost, "@", 2)
-	if len(data) == 2 {
-		cfg.User = data[0]
-		cfg.Host = data[1]
-	} else {
-		cfg.Host = userhost
-	}
-
-	return cfg, nil
 }

src/restic/backend/sftp/config_test.go

@@ -3,7 +3,7 @@ package sftp
 import "testing"
 
 var configTests = []struct {
-	s   string
+	in  string
 	cfg Config
 }{
 	// first form, user specified sftp://user@host/dir
@@ -27,33 +27,49 @@ var configTests = []struct {
 		"sftp://user@host:10022//dir/subdir",
 		Config{User: "user", Host: "host:10022", Dir: "/dir/subdir"},
 	},
+	{
+		"sftp://user@host/dir/subdir/../other",
+		Config{User: "user", Host: "host", Dir: "dir/other"},
+	},
+	{
+		"sftp://user@host/dir///subdir",
+		Config{User: "user", Host: "host", Dir: "dir/subdir"},
+	},
 
 	// second form, user specified sftp:user@host:/dir
 	{
-		"sftp:foo@bar:/baz/quux",
+		"sftp:user@host:/dir/subdir",
-		Config{User: "foo", Host: "bar", Dir: "/baz/quux"},
+		Config{User: "user", Host: "host", Dir: "/dir/subdir"},
 	},
 	{
-		"sftp:bar:../baz/quux",
+		"sftp:host:../dir/subdir",
-		Config{Host: "bar", Dir: "../baz/quux"},
+		Config{Host: "host", Dir: "../dir/subdir"},
 	},
 	{
-		"sftp:fux@bar:baz/qu:ux",
+		"sftp:user@host:dir/subdir:suffix",
-		Config{User: "fux", Host: "bar", Dir: "baz/qu:ux"},
+		Config{User: "user", Host: "host", Dir: "dir/subdir:suffix"},
+	},
+	{
+		"sftp:user@host:dir/subdir/../other",
+		Config{User: "user", Host: "host", Dir: "dir/other"},
+	},
+	{
+		"sftp:user@host:dir///subdir",
+		Config{User: "user", Host: "host", Dir: "dir/subdir"},
 	},
 }
 
 func TestParseConfig(t *testing.T) {
 	for i, test := range configTests {
-		cfg, err := ParseConfig(test.s)
+		cfg, err := ParseConfig(test.in)
 		if err != nil {
-			t.Errorf("test %d failed: %v", i, err)
+			t.Errorf("test %d:%s failed: %v", i, test.in, err)
 			continue
 		}
 
 		if cfg != test.cfg {
-			t.Errorf("test %d: wrong config, want:\n  %v\ngot:\n  %v",
+			t.Errorf("test %d:\ninput:\n  %s\n wrong config, want:\n  %v\ngot:\n  %v",
-				i, test.cfg, cfg)
+				i, test.in, test.cfg, cfg)
 			continue
 		}
 	}