plumbing: packp, Avoid duplicate encoding when overriding a Capability value. (#521)

Previously, calling `Set($CAPABILITY, ...)` on a `capability.List` where `$CAPABILITY`
was already present would correctly replace the existing value of that capability, but
would also result in that capability being listed twice in the internal `l.sort` slice.
This manifested publicly when the `List` was encoded as the same capability appearing
twice with the same value in the encoded output.

2 files changed, 14 insertions, 1 deletions

diff --git a/plumbing/protocol/packp/capability/list.go b/plumbing/protocol/packp/capability/list.go
index f41ec79..553d81c 100644
--- a/plumbing/protocol/packp/capability/list.go
+++ b/plumbing/protocol/packp/capability/list.go
@@ -86,7 +86,9 @@ func (l *List) Get(capability Capability) []string {
 
 // Set sets a capability removing the previous values
 func (l *List) Set(capability Capability, values ...string) error {
-	delete(l.m, capability)
+	if _, ok := l.m[capability]; ok {
+		l.m[capability].Values = l.m[capability].Values[:0]
+	}
 	return l.Add(capability, values...)
 }
 
diff --git a/plumbing/protocol/packp/capability/list_test.go b/plumbing/protocol/packp/capability/list_test.go
index 61b0b13..71181cb 100644
--- a/plumbing/protocol/packp/capability/list_test.go
+++ b/plumbing/protocol/packp/capability/list_test.go
@@ -122,6 +122,17 @@ func (s *SuiteCapabilities) TestSetEmpty(c *check.C) {
 	c.Assert(cap.Get(Agent), check.HasLen, 1)
 }
 
+func (s *SuiteCapabilities) TestSetDuplicate(c *check.C) {
+	cap := NewList()
+	err := cap.Set(Agent, "baz")
+	c.Assert(err, check.IsNil)
+
+	err = cap.Set(Agent, "bar")
+	c.Assert(err, check.IsNil)
+
+	c.Assert(cap.String(), check.Equals, "agent=bar")
+}
+
 func (s *SuiteCapabilities) TestGetEmpty(c *check.C) {
 	cap := NewList()
 	c.Assert(cap.Get(Agent), check.HasLen, 0)