Make ByteString support any CBOR byte string

This commit makes ByteString support CBOR byte string (major type 2)
without being limited to map keys.

ByteString can be used when using a []byte is not possible or
convenient.  For example, Go doesn't allow []byte as map key, so
ByteString can be used to support data formats having CBOR map with byte
string keys. ByteString can also be used to encode invalid UTF-8 string
as CBOR byte string.

- Modified ByteString to use string type.
- Implemented cbor.Marshaller and cbor.Unmarshaller for ByteString.
- Simplified ByteString encoding and decoding.
- Renamed MapKeyByteStringFail to MapKeyByteStringForbidden.
- Renamed MapKeyByteStringWrap to MapKeyByteStringAllowed.
- Added more tests for ByteString.

bytestring.go

@@ -4,31 +4,59 @@
 package cbor
 
 import (
-	"encoding/hex"
-	"reflect"
+	"errors"
 )
 
-var (
-	typeByteString = reflect.TypeOf(NewByteString(nil))
-)
+// ByteString represents CBOR byte string (major type 2). ByteString can be used
+// when using a Go []byte is not possible or convenient. For example, Go doesn't
+// allow []byte as map key, so ByteString can be used to support data formats
+// having CBOR map with byte string keys. ByteString can also be used to
+// encode invalid UTF-8 string as CBOR byte string.
+// See DecOption.MapKeyByteStringMode for more details.
+type ByteString string
 
-type ByteString struct {
-	// XXX: replace with interface{} storing fixed-length byte array?
-	// We use a string because []byte isn't comparable
-	data string
+// Bytes returns bytes representing ByteString.
+func (bs ByteString) Bytes() []byte {
+	return []byte(bs)
 }
 
-func NewByteString(data []byte) ByteString {
-	bs := ByteString{
-		data: string(data),
-	}
-	return bs
-}
+// MarshalCBOR encodes ByteString as CBOR byte string (major type 2).
+func (bs ByteString) MarshalCBOR() ([]byte, error) {
+	e := getEncoderBuffer()
+	defer putEncoderBuffer(e)
 
-func (bs ByteString) Bytes() []byte {
-	return []byte(bs.data)
+	// Encode length
+	encodeHead(e, byte(cborTypeByteString), uint64(len(bs)))
+
+	// Encode data
+	buf := make([]byte, e.Len()+len(bs))
+	n := copy(buf, e.Bytes())
+	copy(buf[n:], bs)
+
+	return buf, nil
 }
 
-func (bs ByteString) String() string {
-	return hex.EncodeToString([]byte(bs.data))
+// UnmarshalCBOR decodes CBOR byte string (major type 2) to ByteString.
+// Decoding CBOR null and CBOR undefined sets ByteString to be empty.
+func (bs *ByteString) UnmarshalCBOR(data []byte) error {
+	if bs == nil {
+		return errors.New("cbor.ByteString: UnmarshalCBOR on nil pointer")
+	}
+
+	// Decoding CBOR null and CBOR undefined to ByteString resets data.
+	// This behavior is similar to decoding CBOR null and CBOR undefined to []byte.
+	if len(data) == 1 && (data[0] == 0xf6 || data[0] == 0xf7) {
+		*bs = ""
+		return nil
+	}
+
+	d := decoder{data: data, dm: defaultDecMode}
+
+	// Check if CBOR data type is byte string
+	if typ := d.nextCBORType(); typ != cborTypeByteString {
+		return &UnmarshalTypeError{CBORType: typ.String(), GoType: typeByteString.String()}
+	}
+
+	*bs = ByteString(d.parseByteString())
+	return nil
 }

bytestring_test.go

@@ -0,0 +1,101 @@
+// Copyright (c) Faye Amacker. All rights reserved.
+// Licensed under the MIT License. See LICENSE in the project root for license information.
+
+package cbor
+
+import "testing"
+
+func TestByteString(t *testing.T) {
+	type s1 struct {
+		A ByteString `cbor:"a"`
+	}
+	type s2 struct {
+		A *ByteString `cbor:"a"`
+	}
+	type s3 struct {
+		A ByteString `cbor:"a,omitempty"`
+	}
+	type s4 struct {
+		A *ByteString `cbor:"a,omitempty"`
+	}
+
+	emptybs := ByteString("")
+	bs := ByteString("\x01\x02\x03\x04")
+
+	testCases := []roundTripTest{
+		{
+			name:         "empty",
+			obj:          emptybs,
+			wantCborData: hexDecode("40"),
+		},
+		{
+			name:         "not empty",
+			obj:          bs,
+			wantCborData: hexDecode("4401020304"),
+		},
+		{
+			name:         "array",
+			obj:          []ByteString{bs},
+			wantCborData: hexDecode("814401020304"),
+		},
+		{
+			name:         "map with ByteString key",
+			obj:          map[ByteString]bool{bs: true},
+			wantCborData: hexDecode("a14401020304f5"),
+		},
+		{
+			name:         "empty ByteString field",
+			obj:          s1{},
+			wantCborData: hexDecode("a1616140"),
+		},
+		{
+			name:         "not empty ByteString field",
+			obj:          s1{A: bs},
+			wantCborData: hexDecode("a161614401020304"),
+		},
+		{
+			name:         "nil *ByteString field",
+			obj:          s2{},
+			wantCborData: hexDecode("a16161f6"),
+		},
+		{
+			name:         "empty *ByteString field",
+			obj:          s2{A: &emptybs},
+			wantCborData: hexDecode("a1616140"),
+		},
+		{
+			name:         "not empty *ByteString field",
+			obj:          s2{A: &bs},
+			wantCborData: hexDecode("a161614401020304"),
+		},
+		{
+			name:         "empty ByteString field with omitempty option",
+			obj:          s3{},
+			wantCborData: hexDecode("a0"),
+		},
+		{
+			name:         "not empty ByteString field with omitempty option",
+			obj:          s3{A: bs},
+			wantCborData: hexDecode("a161614401020304"),
+		},
+		{
+			name:         "nil *ByteString field with omitempty option",
+			obj:          s4{},
+			wantCborData: hexDecode("a0"),
+		},
+		{
+			name:         "empty *ByteString field with omitempty option",
+			obj:          s4{A: &emptybs},
+			wantCborData: hexDecode("a1616140"),
+		},
+		{
+			name:         "not empty *ByteString field with omitempty option",
+			obj:          s4{A: &bs},
+			wantCborData: hexDecode("a161614401020304"),
+		},
+	}
+
+	em, _ := EncOptions{}.EncMode()
+	dm, _ := DecOptions{}.DecMode()
+	testRoundTrip(t, testCases, em, dm)
+}

decode.go

@@ -239,24 +239,27 @@ func (idm IntDecMode) valid() bool {
 	return idm < maxIntDec
 }
 
-// MapKeyByteStringMode specifies whether to use a wrapper object for byte strings.
+// MapKeyByteStringMode specifies how to decode CBOR byte string (major type 2)
+// as Go map key when decoding CBOR map with byte string keys into an empty
+// Go interface value. Since Go doesn't allow []byte as map key, this option
+// can be used to make CBOR byte string decodable as a Go map key.
 type MapKeyByteStringMode int
 
 const (
-	// MapKeyByteStringFail affects how a CBOR byte string as a map key is decoded.
-	// It makes the parsing fail as Go cannot use []byte as a map key.
-	MapKeyByteStringFail MapKeyByteStringMode = iota
+	// MapKeyByteStringForbidden forbids CBOR byte string being decoded as Go map key.
+	// This is the default setting because Go cannot use []byte as a map key.
+	MapKeyByteStringForbidden MapKeyByteStringMode = iota
 
-	// MapKeyByteStringWrap affects how a CBOR byte string as a map key is decoded.
-	// It allows the parsing to succeed by wrapping the byte string in a wrapper object
-	// that can be used as a map key in Go.
-	MapKeyByteStringWrap
+	// MapKeyByteStringAllowed allows CBOR byte string being decoded as Go map key.
+	// Since Go doesn't allow []byte as map key, CBOR byte string is decoded to
+	// ByteString, which is an alias for Go string.
+	MapKeyByteStringAllowed
 
-	maxMapKeyByteString
+	maxMapKeyByteStringMode
 )
 
-func (bsm MapKeyByteStringMode) valid() bool {
-	return bsm < maxMapKeyByteString
+func (mkbsm MapKeyByteStringMode) valid() bool {
+	return mkbsm < maxMapKeyByteStringMode
 }
 
 // ExtraDecErrorCond specifies extra conditions that should be treated as errors.
@@ -329,7 +332,10 @@ type DecOptions struct {
 	// when decoding CBOR int (major type 0 and 1) to Go interface{}.
 	IntDec IntDecMode
 
-	// MapKeyByteString specifies whether to use a wrapper object for byte strings.
+	// MapKeyByteString specifies how to decode CBOR byte string as map key
+	// when decoding CBOR map with byte string key into an empty interface value.
+	// By default, an error is returned when attempting to decode CBOR byte string
+	// as map key because Go doesn't allow []byte as map key.
 	MapKeyByteString MapKeyByteStringMode
 
 	// ExtraReturnErrors specifies extra conditions that should be treated as errors.
@@ -1236,7 +1242,6 @@ func (d *decoder) parseMap() (interface{}, error) {
 	var k, e interface{}
 	var err, lastErr error
 	keyCount := 0
-	typeByteSlice := reflect.TypeOf([]byte{})
 	for i := 0; (hasSize && i < count) || (!hasSize && !d.foundBreak()); i++ {
 		// Parse CBOR map key.
 		if k, lastErr = d.parse(true); lastErr != nil {
@@ -1249,17 +1254,17 @@ func (d *decoder) parseMap() (interface{}, error) {
 
 		// Detect if CBOR map key can be used as Go map key.
 		rv := reflect.ValueOf(k)
-		// Wrap byte string if enabled
-		if rv.IsValid() && rv.Type() == typeByteSlice && d.dm.mapKeyByteString == MapKeyByteStringWrap {
-			k = NewByteString(rv.Bytes())
-			rv = reflect.ValueOf(k)
-		}
 		if !isHashableValue(rv) {
-			if err == nil {
-				err = errors.New("cbor: invalid map key type: " + rv.Type().String())
+			if b, ok := k.([]byte); ok && d.dm.mapKeyByteString == MapKeyByteStringAllowed {
+				// Use decoded []byte as a ByteString map key.
+				k = ByteString(b)
+			} else {
+				if err == nil {
+					err = errors.New("cbor: invalid map key type: " + rv.Type().String())
+				}
+				d.skip()
+				continue
 			}
-			d.skip()
-			continue
 		}
 
 		// Parse CBOR map value.
@@ -1310,7 +1315,6 @@ func (d *decoder) parseMapToMap(v reflect.Value, tInfo *typeInfo) error { //noli
 	keyIsInterfaceType := keyType == typeIntf // If key type is interface{}, need to check if key value is hashable.
 	var err, lastErr error
 	keyCount := v.Len()
-	typeByteSlice := reflect.TypeOf([]byte{})
 	var existingKeys map[interface{}]bool // Store existing map keys, used for detecting duplicate map key.
 	if d.dm.dupMapKey == DupMapKeyEnforcedAPF {
 		existingKeys = make(map[interface{}]bool, keyCount)
@@ -1338,11 +1342,6 @@ func (d *decoder) parseMapToMap(v reflect.Value, tInfo *typeInfo) error { //noli
 			d.skip()
 			continue
 		}
-		// Wrap byte string if enabled
-		if keyType == typeByteSlice && d.dm.mapKeyByteString == MapKeyByteStringWrap {
-			keyValue = reflect.ValueOf(NewByteString(keyValue.Bytes()))
-			keyType = keyValue.Type()
-		}
 
 		// Detect if CBOR map key can be used as Go map key.
 		if keyIsInterfaceType && keyValue.Elem().IsValid() {