Merge pull request #626 from orirawlings/master

packp/capability: Skip argument validations for unknown capabilities

3 files changed, 36 insertions, 11 deletions

diff --git a/plumbing/protocol/packp/capability/capability.go b/plumbing/protocol/packp/capability/capability.go
index 96d93f6..a129781 100644
--- a/plumbing/protocol/packp/capability/capability.go
+++ b/plumbing/protocol/packp/capability/capability.go
@@ -234,7 +234,7 @@ const (
 
 const DefaultAgent = "go-git/4.x"
 
-var valid = map[Capability]bool{
+var known = map[Capability]bool{
 	MultiACK: true, MultiACKDetailed: true, NoDone: true, ThinPack: true,
 	Sideband: true, Sideband64k: true, OFSDelta: true, Agent: true,
 	Shallow: true, DeepenSince: true, DeepenNot: true, DeepenRelative: true,
diff --git a/plumbing/protocol/packp/capability/list.go b/plumbing/protocol/packp/capability/list.go
index 3904a4e..26a79b6 100644
--- a/plumbing/protocol/packp/capability/list.go
+++ b/plumbing/protocol/packp/capability/list.go
@@ -108,7 +108,7 @@ func (l *List) Add(c Capability, values ...string) error {
 		return nil
 	}
 
-	if !multipleArgument[c] && len(l.m[c].Values) > 0 {
+	if known[c] && !multipleArgument[c] && len(l.m[c].Values) > 0 {
 		return ErrMultipleArguments
 	}
 
@@ -116,7 +116,19 @@ func (l *List) Add(c Capability, values ...string) error {
 	return nil
 }
 
+func (l *List) validateNoEmptyArgs(values []string) error {
+	for _, v := range values {
+		if v == "" {
+			return ErrEmtpyArgument
+		}
+	}
+	return nil
+}
+
 func (l *List) validate(c Capability, values []string) error {
+	if !known[c] {
+		return l.validateNoEmptyArgs(values)
+	}
 	if requiresArgument[c] && len(values) == 0 {
 		return ErrArgumentsRequired
 	}
@@ -128,14 +140,7 @@ func (l *List) validate(c Capability, values []string) error {
 	if !multipleArgument[c] && len(values) > 1 {
 		return ErrMultipleArguments
 	}
-
-	for _, v := range values {
-		if v == "" {
-			return ErrEmtpyArgument
-		}
-	}
-
-	return nil
+	return l.validateNoEmptyArgs(values)
 }
 
 // Supports returns true if capability is present
diff --git a/plumbing/protocol/packp/capability/list_test.go b/plumbing/protocol/packp/capability/list_test.go
index 9665e89..82dd63f 100644
--- a/plumbing/protocol/packp/capability/list_test.go
+++ b/plumbing/protocol/packp/capability/list_test.go
@@ -65,6 +65,26 @@ func (s *SuiteCapabilities) TestDecodeWithUnknownCapability(c *check.C) {
 	c.Assert(cap.Supports(Capability("foo")), check.Equals, true)
 }
 
+func (s *SuiteCapabilities) TestDecodeWithUnknownCapabilityWithArgument(c *check.C) {
+	cap := NewList()
+	err := cap.Decode([]byte("oldref=HEAD:refs/heads/v2 thin-pack"))
+	c.Assert(err, check.IsNil)
+
+	c.Assert(cap.m, check.HasLen, 2)
+	c.Assert(cap.Get("oldref"), check.DeepEquals, []string{"HEAD:refs/heads/v2"})
+	c.Assert(cap.Get(ThinPack), check.IsNil)
+}
+
+func (s *SuiteCapabilities) TestDecodeWithUnknownCapabilityWithMultipleArgument(c *check.C) {
+	cap := NewList()
+	err := cap.Decode([]byte("foo=HEAD:refs/heads/v2 foo=HEAD:refs/heads/v1 thin-pack"))
+	c.Assert(err, check.IsNil)
+
+	c.Assert(cap.m, check.HasLen, 2)
+	c.Assert(cap.Get("foo"), check.DeepEquals, []string{"HEAD:refs/heads/v2", "HEAD:refs/heads/v1"})
+	c.Assert(cap.Get(ThinPack), check.IsNil)
+}
+
 func (s *SuiteCapabilities) TestString(c *check.C) {
 	cap := NewList()
 	cap.Set(Agent, "bar")
@@ -153,7 +173,7 @@ func (s *SuiteCapabilities) TestAddErrArgumentsNotAllowed(c *check.C) {
 	c.Assert(err, check.Equals, ErrArguments)
 }
 
-func (s *SuiteCapabilities) TestAddErrArgumendts(c *check.C) {
+func (s *SuiteCapabilities) TestAddErrArguments(c *check.C) {
 	cap := NewList()
 	err := cap.Add(SymRef, "")
 	c.Assert(err, check.Equals, ErrEmtpyArgument)