Fix key.Remove removing 2 elements (#413)

* Add tests for #412

* fix off-by-one

* Update Changes

* slightly tweak test

Author: lestrrat

Changes

@@ -1,6 +1,13 @@
 Changes
 =======
 
+v1.2.3
+[Buf fixes]
+  * jwk.Set incorrectly removed 2 elements instead of one.
+
+[Miscellaneous]
+  * github.com/goccy/go-json has been upgraded to v0.7.4
+
 v1.2.2 13 Jul 2021
 [Deprecation notice]
   * `(jwe.Message).Decrypt()` will be removed from the API upon the next

jwk/jwk_test.go

@@ -9,6 +9,7 @@ import (
 	"fmt"
 	"math/big"
 	"reflect"
+	"strconv"
 	"strings"
 	"testing"
 	"time"
@@ -1567,3 +1568,44 @@ func TestTypedFields(t *testing.T) {
 		}
 	})
 }
+
+func TestGH412(t *testing.T) {
+	set := jwk.NewSet()
+
+	const max = 5
+	for i := 0; i < max; i++ {
+		k, err := jwxtest.GenerateRsaJwk()
+		if !assert.NoError(t, err, `jwxttest.GenerateRsaJwk() should succeed`) {
+			return
+		}
+
+		k.Set(jwk.KeyIDKey, strconv.Itoa(i))
+		set.Add(k)
+	}
+
+	if !assert.Equal(t, max, set.Len(), `set.Len should be %d`, max) {
+		return
+	}
+
+	k, ok := set.Get(max / 2)
+	if !assert.True(t, ok, `set.Get should succeed`) {
+		return
+	}
+
+	if !assert.True(t, set.Remove(k), `set.Remove should succeed`) {
+		return
+	}
+
+	if !assert.Equal(t, max-1, set.Len(), `set.Len should be %d`, max-1) {
+		return
+	}
+
+	ctx := context.Background()
+	for iter := set.Iterate(ctx); iter.Next(ctx); {
+		pair := iter.Pair()
+		key := pair.Value.(jwk.Key)
+		if !assert.NotEqual(t, k.KeyID(), key.KeyID(), `key id should not match`) {
+			return
+		}
+	}
+}

jwk/set.go

@@ -71,7 +71,7 @@ func (s *set) Remove(key Key) bool {
 			case len(s.keys) - 1:
 				s.keys = s.keys[:i-1]
 			default:
-				s.keys = append(s.keys[:i-1], s.keys[i+1:]...)
+				s.keys = append(s.keys[:i], s.keys[i+1:]...)
 			}
 			return true
 		}