ssh/config: fix setting the default values

The field default value should be set on Get, after all the Host or
Match fields merged.
In this way, if the field key already set, its not overridden by the
default value or subsequent Host or Match vaue.

8 files changed, 41 insertions, 26 deletions

diff --git a/lib/ssh/config/config.go b/lib/ssh/config/config.go
index 25dd9b3b..21116d44 100644
--- a/lib/ssh/config/config.go
+++ b/lib/ssh/config/config.go
@@ -78,14 +78,12 @@ func Load(file string) (cfg *Config, err error) {
 		switch key {
 		case keyHost:
 			if section != nil {
-				section.init(p.workDir, p.homeDir)
 				cfg.sections = append(cfg.sections, section)
 				section = nil
 			}
 			section = newSectionHost(value)
 		case keyMatch:
 			if section != nil {
-				section.init(p.workDir, p.homeDir)
 				cfg.sections = append(cfg.sections, section)
 				section = nil
 			}
@@ -105,7 +103,6 @@ func Load(file string) (cfg *Config, err error) {
 		}
 	}
 	if section != nil {
-		section.init(p.workDir, p.homeDir)
 		cfg.sections = append(cfg.sections, section)
 		section = nil
 	}
@@ -123,7 +120,7 @@ func (cfg *Config) Get(s string) (section *Section) {
 			section.mergeField(hostMatch)
 		}
 	}
-	section.init(cfg.workDir, cfg.homeDir)
+	section.setDefaults(cfg.workDir, cfg.homeDir)
 
 	if s != `` && section.Field[KeyHostname] == `` {
 		section.Set(KeyHostname, s)
diff --git a/lib/ssh/config/config_test.go b/lib/ssh/config/config_test.go
index c7e479ce..f31e5050 100644
--- a/lib/ssh/config/config_test.go
+++ b/lib/ssh/config/config_test.go
@@ -26,7 +26,7 @@ func TestMain(m *testing.M) {
 		log.Fatal(err)
 	}
 
-	testDefaultSection.init(testParser.workDir, testParser.homeDir)
+	testDefaultSection.setDefaults(testParser.workDir, testParser.homeDir)
 
 	os.Exit(m.Run())
 }
diff --git a/lib/ssh/config/parser_test.go b/lib/ssh/config/parser_test.go
index 3e7d0a48..dea0db82 100644
--- a/lib/ssh/config/parser_test.go
+++ b/lib/ssh/config/parser_test.go
@@ -78,6 +78,7 @@ func TestReadLines(t *testing.T) {
 			`Include sub/config`,
 			`Host foo.local`,
 			`Hostname 127.0.0.3`,
+			`Port 28022`,
 			`User foo`,
 			`IdentityFile ~/.ssh/foo`,
 			`Host *foo.local`,
@@ -123,6 +124,7 @@ func TestConfigParser_load(t *testing.T) {
 			`IdentityFile ~/.ssh/notexist`,
 			`Host foo.local`,
 			`Hostname 127.0.0.3`,
+			`Port 28022`,
 			`User foo`,
 			`IdentityFile ~/.ssh/foo`,
 			`Host *foo.local`,
diff --git a/lib/ssh/config/section.go b/lib/ssh/config/section.go
index 8e3c3f00..dc0b9654 100644
--- a/lib/ssh/config/section.go
+++ b/lib/ssh/config/section.go
@@ -153,6 +153,15 @@ const (
 	DefXAuthLocation      = `/usr/X11R6/bin/xauth`
 )
 
+// defaultFields define default field value for known key.
+var defaultFields = map[string]string{
+	KeyChallengeResponseAuthentication: ValueYes,
+	KeyCheckHostIP:                     ValueYes,
+	KeyConnectionAttempts:              DefConnectionAttempts,
+	KeyPort:                            DefPort,
+	KeyXAuthLocation:                   DefXAuthLocation,
+}
+
 // defaultCASignatureAlgorithms return list of default signature algorithms
 // that client supported.
 func defaultCASignatureAlgorithms() []string {
@@ -222,7 +231,7 @@ type Section struct {
 	useCriteria bool
 }
 
-// NewSection create new Host or Match with default values.
+// NewSection create an empty Host or Match section.
 func NewSection(name string) *Section {
 	name = strings.TrimSpace(name)
 	if len(name) == 0 {
@@ -230,15 +239,9 @@ func NewSection(name string) *Section {
 	}
 
 	return &Section{
-		Field: map[string]string{
-			KeyChallengeResponseAuthentication: ValueYes,
-			KeyCheckHostIP:                     ValueYes,
-			KeyConnectionAttempts:              DefConnectionAttempts,
-			KeyPort:                            DefPort,
-			KeyXAuthLocation:                   DefXAuthLocation,
-		},
-		env:  map[string]string{},
-		name: name,
+		Field: map[string]string{},
+		env:   map[string]string{},
+		name:  name,
 	}
 }
 
@@ -461,8 +464,8 @@ func (section *Section) isMatch(s string) bool {
 	return false
 }
 
-// init check, parse, and expand all of the fields values.
-func (section *Section) init(workDir, homeDir string) {
+// setDefaults set and expand all of the fields values if its not set.
+func (section *Section) setDefaults(workDir, homeDir string) {
 	section.homeDir = homeDir
 	section.WorkingDir = workDir
 
@@ -475,6 +478,18 @@ func (section *Section) init(workDir, homeDir string) {
 			section.IdentityFile[x] = strings.Replace(identFile, "~", section.homeDir, 1)
 		}
 	}
+
+	var (
+		key   string
+		val   string
+		exist bool
+	)
+	for key, val = range defaultFields {
+		_, exist = section.Field[key]
+		if !exist {
+			section.Field[key] = val
+		}
+	}
 }
 
 // mergeField merge the Field from other Section.
diff --git a/lib/ssh/config/section_match_test.go b/lib/ssh/config/section_match_test.go
index 5a40051f..d2742698 100644
--- a/lib/ssh/config/section_match_test.go
+++ b/lib/ssh/config/section_match_test.go
@@ -29,7 +29,7 @@ func TestNewSectionMatch(t *testing.T) {
 			}
 			continue
 		}
-		got.init(testParser.workDir, testParser.homeDir)
+		got.setDefaults(testParser.workDir, testParser.homeDir)
 
 		test.Assert(t, c.raw, *c.exp, *got)
 	}
@@ -92,7 +92,7 @@ func TestParseCriteriaAll(t *testing.T) {
 			}
 			continue
 		}
-		got.init(testParser.workDir, testParser.homeDir)
+		got.setDefaults(testParser.workDir, testParser.homeDir)
 
 		exp := c.exp(*testDefaultSection)
 		test.Assert(t, c.raw, *exp, *got)
@@ -137,7 +137,7 @@ func TestNewSectionMatch_ParseCriteriaExec(t *testing.T) {
 			}
 			continue
 		}
-		got.init(testParser.workDir, testParser.homeDir)
+		got.setDefaults(testParser.workDir, testParser.homeDir)
 
 		exp := c.exp(*testDefaultSection)
 		test.Assert(t, c.raw, *exp, *got)
@@ -206,7 +206,7 @@ func TestParseCriteriaWithArg(t *testing.T) {
 			}
 			continue
 		}
-		got.init(testParser.workDir, testParser.homeDir)
+		got.setDefaults(testParser.workDir, testParser.homeDir)
 
 		exp := c.exp(*testDefaultSection)
 		test.Assert(t, c.raw, *exp, *got)
diff --git a/lib/ssh/config/section_test.go b/lib/ssh/config/section_test.go
index a5c9c5b8..760d93d6 100644
--- a/lib/ssh/config/section_test.go
+++ b/lib/ssh/config/section_test.go
@@ -54,14 +54,14 @@ func TestNewSectionHost(t *testing.T) {
 
 	for _, c := range cases {
 		got := newSectionHost(c.rawPattern)
-		got.init(testParser.workDir, testParser.homeDir)
+		got.setDefaults(testParser.workDir, testParser.homeDir)
 
 		exp := c.exp(*testDefaultSection)
 		test.Assert(t, c.rawPattern, *exp, *got)
 	}
 }
 
-func TestSection_init(t *testing.T) {
+func TestSectionSetDefaults(t *testing.T) {
 	cases := []struct {
 		section func(def Section) *Section
 		exp     func(def Section) *Section
@@ -81,10 +81,10 @@ func TestSection_init(t *testing.T) {
 	}}
 	for _, c := range cases {
 		got := c.section(*testDefaultSection)
-		got.init(testParser.workDir, testParser.homeDir)
+		got.setDefaults(testParser.workDir, testParser.homeDir)
 
 		exp := c.exp(*testDefaultSection)
-		test.Assert(t, `init`, exp.IdentityFile, got.IdentityFile)
+		test.Assert(t, `setDefaults`, exp.IdentityFile, got.IdentityFile)
 	}
 }
 
diff --git a/lib/ssh/config/testdata/config b/lib/ssh/config/testdata/config
index 1716e1af..2f876502 100644
--- a/lib/ssh/config/testdata/config
+++ b/lib/ssh/config/testdata/config
@@ -12,6 +12,7 @@ Host *.example.local
 
 Host foo.local
 	Hostname 127.0.0.3
+	Port 28022
 	User foo
 	IdentityFile ~/.ssh/foo
 
diff --git a/lib/ssh/config/testdata/config_get_test.txt b/lib/ssh/config/testdata/config_get_test.txt
index 2fc364e2..e87444c8 100644
--- a/lib/ssh/config/testdata/config_get_test.txt
+++ b/lib/ssh/config/testdata/config_get_test.txt
@@ -38,7 +38,7 @@ Host foo.local
   hostname 127.0.0.3
   identityfile ~/.ssh/foo
   identityfile ~/.ssh/allfoo
-  port 22
+  port 28022
   user allfoo
   xauthlocation /usr/X11R6/bin/xauth