From 6eb97ca6cc6477f29fae1457de9b4e97dcfa55b2 Mon Sep 17 00:00:00 2001 From: Christian Kemper Date: Mon, 15 Feb 2016 07:58:13 -0800 Subject: [PATCH] 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 | 96 ++++++++++++-------------- src/restic/backend/sftp/config_test.go | 38 +++++++--- 2 files changed, 71 insertions(+), 63 deletions(-) diff --git a/src/restic/backend/sftp/config.go b/src/restic/backend/sftp/config.go index ee241facb..d48525344 100644 --- a/src/restic/backend/sftp/config.go +++ b/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://") { - return parseFormat1(s) + var user, host, dir string + 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:"`) } - - // otherwise parse in the sftp:user@host:path format, which means we'll get - // "user@host:path" in s - return parseFormat2(s) -} - -// 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 + return Config{ + User: user, + Host: host, + Dir: path.Clean(dir), + }, nil } diff --git a/src/restic/backend/sftp/config_test.go b/src/restic/backend/sftp/config_test.go index 63a46ae3b..54ff3b91b 100644 --- a/src/restic/backend/sftp/config_test.go +++ b/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", - Config{User: "foo", Host: "bar", Dir: "/baz/quux"}, + "sftp:user@host:/dir/subdir", + Config{User: "user", Host: "host", Dir: "/dir/subdir"}, }, { - "sftp:bar:../baz/quux", - Config{Host: "bar", Dir: "../baz/quux"}, + "sftp:host:../dir/subdir", + Config{Host: "host", Dir: "../dir/subdir"}, }, { - "sftp:fux@bar:baz/qu:ux", - Config{User: "fux", Host: "bar", Dir: "baz/qu:ux"}, + "sftp:user@host:dir/subdir:suffix", + 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", - i, test.cfg, cfg) + t.Errorf("test %d:\ninput:\n %s\n wrong config, want:\n %v\ngot:\n %v", + i, test.in, test.cfg, cfg) continue } }