Repository: fxamacker/cbor (go)

Issue:
bug: Registering tag (TagSet.Add) with already registered tag number should be rejected
### What version of fxamacker/cbor are you using?
commit 3f759bc5919a222f159e5a9d5f715520d931cf3e

### Does this issue reproduce with the latest release?
Yes.

### What OS and CPU architecture are you using (`go env`)?

<details><summary><code>go env</code> Output</summary><br><pre>
$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/user/.cache/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/user/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/go"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build813183223=/tmp/go-build -gno-record-gcc-switches"
</pre></details>

### What did you do?
Use `TagSet.Add` to register two tags.  Both tags have the same tag number, but different content type.

```
func main() {
	type myBool bool
	type myInt int

	tags := cbor.NewTagSet()

	// Register tag number 100 and myBool content type.
	err := tags.Add(
		cbor.TagOptions{DecTag: cbor.DecTagRequired, EncTag: cbor.EncTagRequired},
		reflect.TypeOf(myBool(false)),
		100)
	if err != nil {
		fmt.Printf("Expected no error, got error %v\n", err)
	}

	// Register tag number 100 (again) and myInt content type.
	err = tags.Add(
		cbor.TagOptions{DecTag: cbor.DecTagRequired, EncTag: cbor.EncTagRequired},
		reflect.TypeOf(myInt(0)),
		100)
	if err == nil {
		fmt.Printf("Expected error, got no error\n")
	}
}
```

### What did you expect to see?
Registering tag with already registered tag number should return error.


### What did you see instead?
Registering tag with already registered tag number doesn't return error.

Pull request description:
Reject registering tag with duplicate tag number
Make TagSet.Add return error when new tag number is already registered
with the same TagSet.

Closes #218


Patch:
diff --git a/decode.go b/decode.go
index d3855fb..66549f9 100644
--- a/decode.go
+++ b/decode.go
@@ -1415,7 +1415,7 @@ func (d *decodeState) validRegisteredTagNums(t reflect.Type, registeredTagNums [
 
 func (d *decodeState) getRegisteredTagItem(vt reflect.Type) *tagItem {
 	if d.dm.tags != nil {
-		return d.dm.tags.get(vt)
+		return d.dm.tags.getTagItemFromType(vt)
 	}
 	return nil
 }
diff --git a/encode.go b/encode.go
index de52b54..0098db5 100644
--- a/encode.go
+++ b/encode.go
@@ -485,7 +485,7 @@ func (em *encMode) EncOptions() EncOptions {
 
 func (em *encMode) encTagBytes(t reflect.Type) []byte {
 	if em.tags != nil {
-		if tagItem := em.tags.get(t); tagItem != nil {
+		if tagItem := em.tags.getTagItemFromType(t); tagItem != nil {
 			return tagItem.cborTagNum
 		}
 	}
diff --git a/tag.go b/tag.go
index a812182..defc337 100644
--- a/tag.go
+++ b/tag.go
@@ -123,7 +123,7 @@ type TagSet interface {
 }
 
 type tagProvider interface {
-	get(t reflect.Type) *tagItem
+	getTagItemFromType(t reflect.Type) *tagItem
 }
 
 type tagItem struct {
@@ -133,6 +133,25 @@ type tagItem struct {
 	opts        TagOptions
 }
 
+func (t *tagItem) equalTagNum(num []uint64) bool {
+	// Fast path to compare 1 tag number
+	if len(t.num) == 1 && len(num) == 1 && t.num[0] == num[0] {
+		return true
+	}
+
+	if len(t.num) != len(num) {
+		return false
+	}
+
+	for i := 0; i < len(t.num); i++ {
+		if t.num[i] != num[i] {
+			return false
+		}
+	}
+
+	return true
+}
+
 type (
 	tagSet map[reflect.Type]*tagItem
 
@@ -142,7 +161,7 @@ type (
 	}
 )
 
-func (t tagSet) get(typ reflect.Type) *tagItem {
+func (t tagSet) getTagItemFromType(typ reflect.Type) *tagItem {
 	return t[typ]
 }
 
@@ -165,8 +184,13 @@ func (t *syncTagSet) Add(opts TagOptions, contentType reflect.Type, num uint64,
 	}
 	t.Lock()
 	defer t.Unlock()
-	if _, ok := t.t[contentType]; ok {
-		return errors.New("cbor: content type " + contentType.String() + " already exists in TagSet")
+	for typ, ti := range t.t {
+		if typ == contentType {
+			return errors.New("cbor: content type " + contentType.String() + " already exists in TagSet")
+		}
+		if ti.equalTagNum(tag.num) {
+			return fmt.Errorf("cbor: tag number %v already exists in TagSet", tag.num)
+		}
 	}
 	t.t[contentType] = tag
 	return nil
@@ -182,7 +206,7 @@ func (t *syncTagSet) Remove(contentType reflect.Type) {
 	t.Unlock()
 }
 
-func (t *syncTagSet) get(typ reflect.Type) *tagItem {
+func (t *syncTagSet) getTagItemFromType(typ reflect.Type) *tagItem {
 	t.RLock()
 	ti := t.t[typ]
 	t.RUnlock()


Test patch:
diff --git a/tag_test.go b/tag_test.go
index 2184473..cba9a87 100644
--- a/tag_test.go
+++ b/tag_test.go
@@ -379,7 +379,7 @@ func TestAddTagError(t *testing.T) {
 	}
 }
 
-func TestAddDuplicateTagError(t *testing.T) {
+func TestAddDuplicateTagContentTypeError(t *testing.T) {
 	type myInt int
 	myIntType := reflect.TypeOf(myInt(0))
 	wantErrorMsg := "cbor: content type cbor.myInt already exists in TagSet"
@@ -393,7 +393,49 @@ func TestAddDuplicateTagError(t *testing.T) {
 	if err := tags.Add(TagOptions{DecTag: DecTagRequired, EncTag: EncTagRequired}, myIntType, 101); err == nil {
 		t.Errorf("TagSet.Add(%s, %d) didn't return an error", myIntType.String(), 101)
 	} else if err.Error() != wantErrorMsg {
-		t.Errorf("TagSet.Add(%s, %d) returned error msg %q, want %q", myIntType, 100, err, wantErrorMsg)
+		t.Errorf("TagSet.Add(%s, %d) returned error msg %q, want %q", myIntType, 101, err, wantErrorMsg)
+	}
+}
+
+func TestAddDuplicateTagNumError(t *testing.T) {
+	type myBool bool
+	type myInt int
+	myBoolType := reflect.TypeOf(myBool(false))
+	myIntType := reflect.TypeOf(myInt(0))
+	wantErrorMsg := "cbor: tag number [100] already exists in TagSet"
+
+	tags := NewTagSet()
+
+	// Add myIntType and 100 to tags
+	if err := tags.Add(TagOptions{DecTag: DecTagRequired, EncTag: EncTagRequired}, myIntType, 100); err != nil {
+		t.Errorf("TagSet.Add(%s, %d) returned error %v", myIntType.String(), 100, err)
+	}
+	// Add myBoolType and 100 to tags
+	if err := tags.Add(TagOptions{DecTag: DecTagRequired, EncTag: EncTagRequired}, myBoolType, 100); err == nil {
+		t.Errorf("TagSet.Add(%s, %d) didn't return an error", myBoolType.String(), 100)
+	} else if err.Error() != wantErrorMsg {
+		t.Errorf("TagSet.Add(%s, %d) returned error msg %q, want %q", myBoolType, 100, err, wantErrorMsg)
+	}
+}
+
+func TestAddDuplicateTagNumsError(t *testing.T) {
+	type myBool bool
+	type myInt int
+	myBoolType := reflect.TypeOf(myBool(false))
+	myIntType := reflect.TypeOf(myInt(0))
+	wantErrorMsg := "cbor: tag number [100 101] already exists in TagSet"
+
+	tags := NewTagSet()
+
+	// Add myIntType and [100, 101] to tags
+	if err := tags.Add(TagOptions{DecTag: DecTagRequired, EncTag: EncTagRequired}, myIntType, 100, 101); err != nil {
+		t.Errorf("TagSet.Add(%s, %d, %d) returned error %v", myIntType.String(), 100, 101, err)
+	}
+	// Add myBoolType and [100, 101] to tags
+	if err := tags.Add(TagOptions{DecTag: DecTagRequired, EncTag: EncTagRequired}, myBoolType, 100, 101); err == nil {
+		t.Errorf("TagSet.Add(%s, %d, %d) didn't return an error", myBoolType.String(), 100, 101)
+	} else if err.Error() != wantErrorMsg {
+		t.Errorf("TagSet.Add(%s, %d, %d) returned error msg %q, want %q", myBoolType, 100, 101, err, wantErrorMsg)
 	}
 }