Merge branch 'master' of github.com:src-d/go-git into annotated

5 files changed, 68 insertions, 14 deletions

diff --git a/config/config.go b/config/config.go
index c730015..a637f6d 100644
--- a/config/config.go
+++ b/config/config.go
@@ -40,6 +40,9 @@ type Config struct {
 		IsBare bool
 		// Worktree is the path to the root of the working tree.
 		Worktree string
+		// CommentChar is the character indicating the start of a
+		// comment for commands like commit and tag
+		CommentChar string
 	}
 
 	Pack struct {
@@ -113,6 +116,7 @@ const (
 	urlKey           = "url"
 	bareKey          = "bare"
 	worktreeKey      = "worktree"
+	commentCharKey   = "commentChar"
 	windowKey        = "window"
 	mergeKey         = "merge"
 
@@ -135,7 +139,7 @@ func (c *Config) Unmarshal(b []byte) error {
 	if err := c.unmarshalPack(); err != nil {
 		return err
 	}
-	c.unmarshalSubmodules()
+	unmarshalSubmodules(c.Raw, c.Submodules)
 
 	if err := c.unmarshalBranches(); err != nil {
 		return err
@@ -151,6 +155,7 @@ func (c *Config) unmarshalCore() {
 	}
 
 	c.Core.Worktree = s.Options.Get(worktreeKey)
+	c.Core.CommentChar = s.Options.Get(commentCharKey)
 }
 
 func (c *Config) unmarshalPack() error {
@@ -182,13 +187,17 @@ func (c *Config) unmarshalRemotes() error {
 	return nil
 }
 
-func (c *Config) unmarshalSubmodules() {
-	s := c.Raw.Section(submoduleSection)
+func unmarshalSubmodules(fc *format.Config, submodules map[string]*Submodule) {
+	s := fc.Section(submoduleSection)
 	for _, sub := range s.Subsections {
 		m := &Submodule{}
 		m.unmarshal(sub)
 
-		c.Submodules[m.Name] = m
+		if m.Validate() == ErrModuleBadPath {
+			continue
+		}
+
+		submodules[m.Name] = m
 	}
 }
 
diff --git a/config/config_test.go b/config/config_test.go
index 5cd713e..db0932c 100644
--- a/config/config_test.go
+++ b/config/config_test.go
@@ -13,6 +13,7 @@ func (s *ConfigSuite) TestUnmarshall(c *C) {
 	input := []byte(`[core]
         bare = true
 		worktree = foo
+		commentchar = bar
 [pack]
 		window = 20
 [remote "origin"]
@@ -23,6 +24,8 @@ func (s *ConfigSuite) TestUnmarshall(c *C) {
 		url = git@github.com:src-d/go-git.git
 		fetch = +refs/heads/*:refs/remotes/origin/*
 		fetch = +refs/pull/*:refs/remotes/origin/pull/*
+[remote "win-local"]
+		url = X:\\Git\\
 [submodule "qux"]
         path = qux
         url = https://github.com/foo/qux.git
@@ -38,14 +41,17 @@ func (s *ConfigSuite) TestUnmarshall(c *C) {
 
 	c.Assert(cfg.Core.IsBare, Equals, true)
 	c.Assert(cfg.Core.Worktree, Equals, "foo")
+	c.Assert(cfg.Core.CommentChar, Equals, "bar")
 	c.Assert(cfg.Pack.Window, Equals, uint(20))
-	c.Assert(cfg.Remotes, HasLen, 2)
+	c.Assert(cfg.Remotes, HasLen, 3)
 	c.Assert(cfg.Remotes["origin"].Name, Equals, "origin")
 	c.Assert(cfg.Remotes["origin"].URLs, DeepEquals, []string{"git@github.com:mcuadros/go-git.git"})
 	c.Assert(cfg.Remotes["origin"].Fetch, DeepEquals, []RefSpec{"+refs/heads/*:refs/remotes/origin/*"})
 	c.Assert(cfg.Remotes["alt"].Name, Equals, "alt")
 	c.Assert(cfg.Remotes["alt"].URLs, DeepEquals, []string{"git@github.com:mcuadros/go-git.git", "git@github.com:src-d/go-git.git"})
 	c.Assert(cfg.Remotes["alt"].Fetch, DeepEquals, []RefSpec{"+refs/heads/*:refs/remotes/origin/*", "+refs/pull/*:refs/remotes/origin/pull/*"})
+	c.Assert(cfg.Remotes["win-local"].Name, Equals, "win-local")
+	c.Assert(cfg.Remotes["win-local"].URLs, DeepEquals, []string{"X:\\Git\\"})
 	c.Assert(cfg.Submodules, HasLen, 1)
 	c.Assert(cfg.Submodules["qux"].Name, Equals, "qux")
 	c.Assert(cfg.Submodules["qux"].URL, Equals, "https://github.com/foo/qux.git")
@@ -67,6 +73,8 @@ func (s *ConfigSuite) TestMarshall(c *C) {
 	fetch = +refs/pull/*:refs/remotes/origin/pull/*
 [remote "origin"]
 	url = git@github.com:mcuadros/go-git.git
+[remote "win-local"]
+	url = "X:\\Git\\"
 [submodule "qux"]
 	url = https://github.com/foo/qux.git
 [branch "master"]
@@ -89,6 +97,11 @@ func (s *ConfigSuite) TestMarshall(c *C) {
 		Fetch: []RefSpec{"+refs/heads/*:refs/remotes/origin/*", "+refs/pull/*:refs/remotes/origin/pull/*"},
 	}
 
+	cfg.Remotes["win-local"] = &RemoteConfig{
+		Name: "win-local",
+		URLs: []string{"X:\\Git\\"},
+	}
+
 	cfg.Submodules["qux"] = &Submodule{
 		Name: "qux",
 		URL:  "https://github.com/foo/qux.git",
@@ -117,6 +130,8 @@ func (s *ConfigSuite) TestUnmarshallMarshall(c *C) {
 	url = git@github.com:mcuadros/go-git.git
 	fetch = +refs/heads/*:refs/remotes/origin/*
 	mirror = true
+[remote "win-local"]
+	url = "X:\\Git\\"
 [branch "master"]
 	remote = origin
 	merge = refs/heads/master
diff --git a/config/modules.go b/config/modules.go
index b208984..90758d9 100644
--- a/config/modules.go
+++ b/config/modules.go
@@ -3,6 +3,7 @@ package config
 import (
 	"bytes"
 	"errors"
+	"regexp"
 
 	format "gopkg.in/src-d/go-git.v4/plumbing/format/config"
 )
@@ -10,6 +11,12 @@ import (
 var (
 	ErrModuleEmptyURL  = errors.New("module config: empty URL")
 	ErrModuleEmptyPath = errors.New("module config: empty path")
+	ErrModuleBadPath   = errors.New("submodule has an invalid path")
+)
+
+var (
+	// Matches module paths with dotdot ".." components.
+	dotdotPath = regexp.MustCompile(`(^|[/\\])\.\.([/\\]|$)`)
 )
 
 // Modules defines the submodules properties, represents a .gitmodules file
@@ -44,14 +51,7 @@ func (m *Modules) Unmarshal(b []byte) error {
 		return err
 	}
 
-	s := m.raw.Section(submoduleSection)
-	for _, sub := range s.Subsections {
-		mod := &Submodule{}
-		mod.unmarshal(sub)
-
-		m.Submodules[mod.Path] = mod
-	}
-
+	unmarshalSubmodules(m.raw, m.Submodules)
 	return nil
 }
 
@@ -102,6 +102,10 @@ func (m *Submodule) Validate() error {
 		return ErrModuleEmptyURL
 	}
 
+	if dotdotPath.MatchString(m.Path) {
+		return ErrModuleBadPath
+	}
+
 	return nil
 }
 
diff --git a/config/modules_test.go b/config/modules_test.go
index 36cd93f..8e10d70 100644
--- a/config/modules_test.go
+++ b/config/modules_test.go
@@ -11,6 +11,29 @@ func (s *ModulesSuite) TestValidateMissingURL(c *C) {
 	c.Assert(m.Validate(), Equals, ErrModuleEmptyURL)
 }
 
+func (s *ModulesSuite) TestValidateBadPath(c *C) {
+	input := []string{
+		`..`,
+		`../`,
+		`../bar`,
+
+		`/..`,
+		`/../bar`,
+
+		`foo/..`,
+		`foo/../`,
+		`foo/../bar`,
+	}
+
+	for _, p := range input {
+		m := &Submodule{
+			Path: p,
+			URL:  "https://example.com/",
+		}
+		c.Assert(m.Validate(), Equals, ErrModuleBadPath)
+	}
+}
+
 func (s *ModulesSuite) TestValidateMissingName(c *C) {
 	m := &Submodule{URL: "bar"}
 	c.Assert(m.Validate(), Equals, ErrModuleEmptyPath)
@@ -39,6 +62,9 @@ func (s *ModulesSuite) TestUnmarshall(c *C) {
         path = foo/bar
         url = https://github.com/foo/bar.git
 		branch = dev
+[submodule "suspicious"]
+        path = ../../foo/bar
+        url = https://github.com/foo/bar.git
 `)
 
 	cfg := NewModules()
diff --git a/config/refspec.go b/config/refspec.go
index c9b9d52..391705c 100644
--- a/config/refspec.go
+++ b/config/refspec.go
@@ -15,7 +15,7 @@ const (
 
 var (
 	ErrRefSpecMalformedSeparator = errors.New("malformed refspec, separators are wrong")
-	ErrRefSpecMalformedWildcard  = errors.New("malformed refspec, missmatched number of wildcards")
+	ErrRefSpecMalformedWildcard  = errors.New("malformed refspec, mismatched number of wildcards")
 )
 
 // RefSpec is a mapping from local branches to remote references