Skip to content

Commit 935f644

Browse files
mislavmislav
committed
Refactor ssh parser for format compatibility & testability
- Per ssh_config(5), keywords and arguments may be separated by an `=` sign as well as whitespace. - When following the `Include` directive, skip directories that were returned as the result of globbing. - Respect the `Host` context when recursing into `Include`s - Avoid having tests read from the actual filesystem. - Avoid repeatedly looking up the home directory.
1 parent dc8698e commit 935f644

File tree

7 files changed

+172
-134
lines changed

7 files changed

+172
-134
lines changed

‎git/remote_test.go‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,17 @@
11
package git
22

3-
import "testing"
3+
import (
4+
"reflect"
5+
"testing"
6+
)
7+
8+
// TODO: extract assertion helpers into a shared package
9+
func eq(t *testing.T, got interface{}, expected interface{}) {
10+
t.Helper()
11+
if !reflect.DeepEqual(got, expected) {
12+
t.Errorf("expected: %v, got: %v", expected, got)
13+
}
14+
}
415

516
func Test_parseRemotes(t *testing.T) {
617
remoteList := []string{

‎git/ssh_config.go‎

Lines changed: 76 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package git
22

33
import (
44
"bufio"
5+
"io"
56
"net/url"
67
"os"
78
"path/filepath"
@@ -12,13 +13,10 @@ import (
1213
)
1314

1415
var (
15-
sshTokenRE *regexp.Regexp
16+
sshConfigLineRE = regexp.MustCompile(`\A\s*(?P<keyword>[A-Za-z][A-Za-z0-9]*)(?:\s+|\s*=\s*)(?P<argument>.+)`)
17+
sshTokenRE = regexp.MustCompile(`%[%h]`)
1618
)
1719

18-
func init() {
19-
sshTokenRE = regexp.MustCompile(`%[%h]`)
20-
}
21-
2220
// SSHAliasMap encapsulates the translation of SSH hostname aliases
2321
type SSHAliasMap map[string]string
2422

@@ -42,42 +40,75 @@ func (m SSHAliasMap) Translator() func(*url.URL) *url.URL {
4240
}
4341
}
4442

45-
type parser struct {
43+
type sshParser struct {
44+
homeDir string
45+
4646
aliasMap SSHAliasMap
47+
hosts []string
48+
49+
open func(string) (io.Reader, error)
50+
glob func(string) ([]string, error)
4751
}
4852

49-
func (p *parser) read(fileName string) error {
50-
file, err := os.Open(fileName)
51-
if err != nil {
52-
return err
53+
func (p *sshParser) read(fileName string) error {
54+
var file io.Reader
55+
if p.open == nil {
56+
f, err := os.Open(fileName)
57+
if err != nil {
58+
return err
59+
}
60+
defer f.Close()
61+
file = f
62+
} else {
63+
var err error
64+
file, err = p.open(fileName)
65+
if err != nil {
66+
return err
67+
}
68+
}
69+
70+
if len(p.hosts) == 0 {
71+
p.hosts = []string{"*"}
5372
}
54-
defer file.Close()
5573

56-
hosts := []string{"*"}
5774
scanner := bufio.NewScanner(file)
5875
for scanner.Scan() {
59-
line := scanner.Text()
60-
fields := strings.Fields(line)
61-
62-
if len(fields) < 2 {
76+
m := sshConfigLineRE.FindStringSubmatch(scanner.Text())
77+
if len(m) < 3 {
6378
continue
6479
}
6580

66-
directive, params := fields[0], fields[1:]
67-
switch {
68-
case strings.EqualFold(directive, "Host"):
69-
hosts = params
70-
case strings.EqualFold(directive, "Hostname"):
71-
for _, host := range hosts {
72-
for _, name := range params {
81+
keyword, arguments := strings.ToLower(m[1]), m[2]
82+
switch keyword {
83+
case "host":
84+
p.hosts = strings.Fields(arguments)
85+
case "hostname":
86+
for _, host := range p.hosts {
87+
for _, name := range strings.Fields(arguments) {
88+
if p.aliasMap == nil {
89+
p.aliasMap = make(SSHAliasMap)
90+
}
7391
p.aliasMap[host] = sshExpandTokens(name, host)
7492
}
7593
}
76-
case strings.EqualFold(directive, "Include"):
77-
for _, path := range absolutePaths(fileName, params) {
78-
fileNames, err := filepath.Glob(path)
79-
if err != nil {
80-
continue
94+
case "include":
95+
for _, arg := range strings.Fields(arguments) {
96+
path := p.absolutePath(fileName, arg)
97+
98+
var fileNames []string
99+
if p.glob == nil {
100+
paths, _ := filepath.Glob(path)
101+
for _, p := range paths {
102+
if s, err := os.Stat(p); err == nil && !s.IsDir() {
103+
fileNames = append(fileNames, p)
104+
}
105+
}
106+
} else {
107+
var err error
108+
fileNames, err = p.glob(path)
109+
if err != nil {
110+
continue
111+
}
81112
}
82113

83114
for _, fileName := range fileNames {
@@ -90,38 +121,20 @@ func (p *parser) read(fileName string) error {
90121
return scanner.Err()
91122
}
92123

93-
func isSystem(path string) bool {
94-
return strings.HasPrefix(path, "/etc/ssh")
95-
}
96-
97-
func absolutePaths(parentFile string, paths []string) []string {
98-
absPaths := make([]string, len(paths))
99-
100-
for i, path := range paths {
101-
switch {
102-
case filepath.IsAbs(path):
103-
absPaths[i] = path
104-
case strings.HasPrefix(path, "~"):
105-
absPaths[i], _ = homedir.Expand(path)
106-
case isSystem(parentFile):
107-
absPaths[i] = filepath.Join("/etc", "ssh", path)
108-
default:
109-
dir, _ := homedir.Dir()
110-
absPaths[i] = filepath.Join(dir, ".ssh", path)
111-
}
124+
func (p *sshParser) absolutePath(parentFile, path string) string {
125+
if filepath.IsAbs(path) || strings.HasPrefix(filepath.ToSlash(path), "/") {
126+
return path
112127
}
113128

114-
return absPaths
115-
}
116-
117-
func parse(files ...string) SSHAliasMap {
118-
p := parser{aliasMap: make(SSHAliasMap)}
129+
if strings.HasPrefix(path, "~") {
130+
return filepath.Join(p.homeDir, strings.TrimPrefix(path, "~"))
131+
}
119132

120-
for _, file := range files {
121-
_ = p.read(file)
133+
if strings.HasPrefix(filepath.ToSlash(parentFile), "/etc/ssh") {
134+
return filepath.Join("/etc/ssh", path)
122135
}
123136

124-
return p.aliasMap
137+
return filepath.Join(p.homeDir, ".ssh", path)
125138
}
126139

127140
// ParseSSHConfig constructs a map of SSH hostname aliases based on user and
@@ -131,12 +144,19 @@ func ParseSSHConfig() SSHAliasMap {
131144
"/etc/ssh_config",
132145
"/etc/ssh/ssh_config",
133146
}
147+
148+
p := sshParser{}
149+
134150
if homedir, err := homedir.Dir(); err == nil {
135151
userConfig := filepath.Join(homedir, ".ssh", "config")
136152
configFiles = append([]string{userConfig}, configFiles...)
153+
p.homeDir = homedir
137154
}
138155

139-
return parse(configFiles...)
156+
for _, file := range configFiles {
157+
_ = p.read(file)
158+
}
159+
return p.aliasMap
140160
}
141161

142162
func sshExpandTokens(text, host string) string {

0 commit comments

Comments
 (0)