From bfcaa814f5deb987f3ed6ae0bdfb3c1ccc42ff6c Mon Sep 17 00:00:00 2001 From: Faye Amacker <33205765+fxamacker@users.noreply.github.com> Date: Mon, 1 Jan 2024 18:35:14 -0600 Subject: [PATCH] Fix cbor.SimpleValue encoding and decoding This commit resolves two issues: 1. Encoding cbor.SimpleValue with values 24..31 should fail because CBOR simple values 24..31 are reserved and they MUST NOT be encoded according to RFC 8949. This commit makes encoder return UnsupportedValueError when encoding cbor.SimpleValue with values 24..31 because that would not be a well-formed CBOR data item. 2. Decoding other CBOR types to cbor.SimpleValue should fail because cbor.SimpleValue represents CBOR simple value (major type 7) which is different from CBOR integers and shouldn't be used interchangeably. This commit makes decoder return UnmarshalTypeError when decoding other CBOR types to cbor.SimpleValue. --- decode.go | 5 -- decode_test.go | 60 ++++++++++++++- encode.go | 20 ++--- simplevalue.go | 53 ++++++++++++- simplevalue_test.go | 177 ++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 299 insertions(+), 16 deletions(-) create mode 100644 simplevalue_test.go diff --git a/decode.go b/decode.go index 6102de69..ffb62e82 100644 --- a/decode.go +++ b/decode.go @@ -1002,11 +1002,6 @@ func (d *decoder) parseToValue(v reflect.Value, tInfo *typeInfo) error { //nolin f := math.Float64frombits(val) return fillFloat(t, f, v) default: // ai <= 24 - // Decode simple values (including false, true, null, and undefined) - if tInfo.nonPtrType == typeSimpleValue { - v.SetUint(val) - return nil - } switch ai { case 20, 21: return fillBool(t, ai == 21, v) diff --git a/decode_test.go b/decode_test.go index 5c655eea..c029f50e 100644 --- a/decode_test.go +++ b/decode_test.go @@ -79,6 +79,7 @@ var unmarshalTests = []unmarshalTest{ typeTag, typeRawTag, typeByteString, + typeSimpleValue, }, }, { @@ -108,6 +109,7 @@ var unmarshalTests = []unmarshalTest{ typeTag, typeRawTag, typeByteString, + typeSimpleValue, }, }, { @@ -137,6 +139,7 @@ var unmarshalTests = []unmarshalTest{ typeTag, typeRawTag, typeByteString, + typeSimpleValue, }, }, { @@ -166,6 +169,7 @@ var unmarshalTests = []unmarshalTest{ typeTag, typeRawTag, typeByteString, + typeSimpleValue, }, }, { @@ -195,6 +199,7 @@ var unmarshalTests = []unmarshalTest{ typeTag, typeRawTag, typeByteString, + typeSimpleValue, }, }, { @@ -224,6 +229,7 @@ var unmarshalTests = []unmarshalTest{ typeTag, typeRawTag, typeByteString, + typeSimpleValue, }, }, { @@ -253,6 +259,7 @@ var unmarshalTests = []unmarshalTest{ typeTag, typeRawTag, typeByteString, + typeSimpleValue, }, }, { @@ -282,6 +289,7 @@ var unmarshalTests = []unmarshalTest{ typeTag, typeRawTag, typeByteString, + typeSimpleValue, }, }, { @@ -311,6 +319,7 @@ var unmarshalTests = []unmarshalTest{ typeTag, typeRawTag, typeByteString, + typeSimpleValue, }, }, { @@ -340,6 +349,7 @@ var unmarshalTests = []unmarshalTest{ typeTag, typeRawTag, typeByteString, + typeSimpleValue, }, }, { @@ -367,6 +377,7 @@ var unmarshalTests = []unmarshalTest{ typeTag, typeRawTag, typeByteString, + typeSimpleValue, }, }, @@ -397,6 +408,7 @@ var unmarshalTests = []unmarshalTest{ typeTag, typeRawTag, typeByteString, + typeSimpleValue, }, }, { @@ -425,6 +437,7 @@ var unmarshalTests = []unmarshalTest{ typeTag, typeRawTag, typeByteString, + typeSimpleValue, }, }, { @@ -453,6 +466,7 @@ var unmarshalTests = []unmarshalTest{ typeTag, typeRawTag, typeByteString, + typeSimpleValue, }, }, { @@ -481,6 +495,7 @@ var unmarshalTests = []unmarshalTest{ typeTag, typeRawTag, typeByteString, + typeSimpleValue, }, }, { @@ -506,6 +521,7 @@ var unmarshalTests = []unmarshalTest{ typeTag, typeRawTag, typeByteString, + typeSimpleValue, }, }, // CBOR value -18446744073709551616 overflows Go's int64, see TestNegIntOverflow @@ -537,6 +553,7 @@ var unmarshalTests = []unmarshalTest{ typeTag, typeRawTag, typeBigInt, + typeSimpleValue, }, }, { @@ -566,6 +583,7 @@ var unmarshalTests = []unmarshalTest{ typeTag, typeRawTag, typeBigInt, + typeSimpleValue, }, }, { @@ -596,6 +614,7 @@ var unmarshalTests = []unmarshalTest{ typeTag, typeRawTag, typeBigInt, + typeSimpleValue, }, }, @@ -623,6 +642,7 @@ var unmarshalTests = []unmarshalTest{ typeRawTag, typeBigInt, typeByteString, + typeSimpleValue, }, }, { @@ -648,6 +668,7 @@ var unmarshalTests = []unmarshalTest{ typeRawTag, typeBigInt, typeByteString, + typeSimpleValue, }, }, { @@ -673,6 +694,7 @@ var unmarshalTests = []unmarshalTest{ typeRawTag, typeBigInt, typeByteString, + typeSimpleValue, }, }, { @@ -698,6 +720,7 @@ var unmarshalTests = []unmarshalTest{ typeRawTag, typeBigInt, typeByteString, + typeSimpleValue, }, }, { @@ -723,6 +746,7 @@ var unmarshalTests = []unmarshalTest{ typeRawTag, typeBigInt, typeByteString, + typeSimpleValue, }, }, { @@ -748,6 +772,7 @@ var unmarshalTests = []unmarshalTest{ typeRawTag, typeBigInt, typeByteString, + typeSimpleValue, }, }, { @@ -773,6 +798,7 @@ var unmarshalTests = []unmarshalTest{ typeRawTag, typeBigInt, typeByteString, + typeSimpleValue, }, }, { @@ -798,6 +824,7 @@ var unmarshalTests = []unmarshalTest{ typeRawTag, typeBigInt, typeByteString, + typeSimpleValue, }, }, @@ -834,6 +861,7 @@ var unmarshalTests = []unmarshalTest{ typeRawTag, typeBigInt, typeByteString, + typeSimpleValue, }, }, { @@ -871,6 +899,7 @@ var unmarshalTests = []unmarshalTest{ typeRawTag, typeBigInt, typeByteString, + typeSimpleValue, }, }, { @@ -900,6 +929,7 @@ var unmarshalTests = []unmarshalTest{ typeRawTag, typeBigInt, typeByteString, + typeSimpleValue, }, }, { @@ -929,6 +959,7 @@ var unmarshalTests = []unmarshalTest{ typeRawTag, typeBigInt, typeByteString, + typeSimpleValue, }, }, { @@ -958,6 +989,7 @@ var unmarshalTests = []unmarshalTest{ typeRawTag, typeBigInt, typeByteString, + typeSimpleValue, }, }, { @@ -994,6 +1026,7 @@ var unmarshalTests = []unmarshalTest{ typeRawTag, typeBigInt, typeByteString, + typeSimpleValue, }, }, { @@ -1028,6 +1061,7 @@ var unmarshalTests = []unmarshalTest{ typeRawTag, typeBigInt, typeByteString, + typeSimpleValue, }, }, { @@ -1057,6 +1091,7 @@ var unmarshalTests = []unmarshalTest{ typeRawTag, typeBigInt, typeByteString, + typeSimpleValue, }, }, { @@ -1086,6 +1121,7 @@ var unmarshalTests = []unmarshalTest{ typeRawTag, typeBigInt, typeByteString, + typeSimpleValue, }, }, { @@ -1122,6 +1158,7 @@ var unmarshalTests = []unmarshalTest{ typeRawTag, typeBigInt, typeByteString, + typeSimpleValue, }, }, { @@ -1151,8 +1188,8 @@ var unmarshalTests = []unmarshalTest{ typeTag, typeRawTag, typeBigInt, - typeByteString, + typeSimpleValue, }, }, { @@ -1183,6 +1220,7 @@ var unmarshalTests = []unmarshalTest{ typeRawTag, typeBigInt, typeByteString, + typeSimpleValue, }, }, @@ -1216,6 +1254,7 @@ var unmarshalTests = []unmarshalTest{ typeTag, typeRawTag, typeByteString, + typeSimpleValue, }, }, { @@ -1245,6 +1284,7 @@ var unmarshalTests = []unmarshalTest{ typeTag, typeRawTag, typeByteString, + typeSimpleValue, }, }, { @@ -1274,6 +1314,7 @@ var unmarshalTests = []unmarshalTest{ typeTag, typeRawTag, typeByteString, + typeSimpleValue, }, }, { @@ -1304,6 +1345,7 @@ var unmarshalTests = []unmarshalTest{ typeTag, typeRawTag, typeByteString, + typeSimpleValue, }, }, { @@ -1333,6 +1375,7 @@ var unmarshalTests = []unmarshalTest{ typeTag, typeRawTag, typeByteString, + typeSimpleValue, }, }, { @@ -1362,6 +1405,7 @@ var unmarshalTests = []unmarshalTest{ typeTag, typeRawTag, typeByteString, + typeSimpleValue, }, }, @@ -1392,6 +1436,7 @@ var unmarshalTests = []unmarshalTest{ typeMapStringInt, typeBigInt, typeByteString, + typeSimpleValue, }, }, // 0: standard date/time { @@ -1420,6 +1465,7 @@ var unmarshalTests = []unmarshalTest{ typeIntSlice, typeMapStringInt, typeByteString, + typeSimpleValue, }, }, // 1: epoch-based date/time { @@ -1455,6 +1501,7 @@ var unmarshalTests = []unmarshalTest{ typeIntSlice, typeMapStringInt, typeByteString, + typeSimpleValue, }, }, // 2: positive bignum: 18446744073709551616 { @@ -1490,6 +1537,7 @@ var unmarshalTests = []unmarshalTest{ typeIntSlice, typeMapStringInt, typeByteString, + typeSimpleValue, }, }, // 3: negative bignum: -18446744073709551617 { @@ -1519,6 +1567,7 @@ var unmarshalTests = []unmarshalTest{ typeMapStringInt, typeBigInt, typeByteString, + typeSimpleValue, }, }, // 1: epoch-based date/time { @@ -1550,6 +1599,7 @@ var unmarshalTests = []unmarshalTest{ typeMapStringInt, typeBigInt, typeByteString, + typeSimpleValue, }, }, // 23: expected conversion to base16 encoding { @@ -1581,6 +1631,7 @@ var unmarshalTests = []unmarshalTest{ typeMapStringInt, typeBigInt, typeByteString, + typeSimpleValue, }, }, // 24: encoded cborBytes data item { @@ -1608,6 +1659,7 @@ var unmarshalTests = []unmarshalTest{ typeMapStringInt, typeBigInt, typeByteString, + typeSimpleValue, }, }, // 32: URI @@ -1859,6 +1911,7 @@ var unmarshalTests = []unmarshalTest{ typeTag, typeRawTag, typeBigInt, + typeSimpleValue, }, }, { @@ -1884,6 +1937,7 @@ var unmarshalTests = []unmarshalTest{ typeRawTag, typeBigInt, typeByteString, + typeSimpleValue, }, }, { @@ -1915,6 +1969,7 @@ var unmarshalTests = []unmarshalTest{ typeTag, typeRawTag, typeByteString, + typeSimpleValue, }, }, @@ -1945,6 +2000,7 @@ var unmarshalTests = []unmarshalTest{ typeIntSlice, typeMapStringInt, typeByteString, + typeSimpleValue, }, }, { @@ -1982,6 +2038,7 @@ var unmarshalTests = []unmarshalTest{ typeMapStringInt, reflect.TypeOf([3]string{}), typeByteString, + typeSimpleValue, }, }, } @@ -2012,6 +2069,7 @@ var unmarshalFloatWrongTypes = []reflect.Type{ typeRawTag, typeBigInt, typeByteString, + typeSimpleValue, } // unmarshalFloatTests includes test values for float16, float32, and float64. diff --git a/encode.go b/encode.go index f329f5da..d762ebb1 100644 --- a/encode.go +++ b/encode.go @@ -110,6 +110,16 @@ func (e *UnsupportedTypeError) Error() string { return "cbor: unsupported type: " + e.Type.String() } +// UnsupportedValueError is returned by Marshal when attempting to encode an +// unsupported value. +type UnsupportedValueError struct { + msg string +} + +func (e *UnsupportedValueError) Error() string { + return "cbor: unsupported value: " + e.msg +} + // SortMode identifies supported sorting order. type SortMode int @@ -1269,14 +1279,6 @@ func encodeTag(e *encoderBuffer, em *encMode, v reflect.Value) error { return encode(e, em, reflect.ValueOf(t.Content)) } -func encodeSimpleValue(e *encoderBuffer, em *encMode, v reflect.Value) error { - if b := em.encTagBytes(v.Type()); b != nil { - e.Write(b) - } - encodeHead(e, byte(cborTypePrimitives), v.Uint()) - return nil -} - func encodeHead(e *encoderBuffer, t byte, n uint64) { if n <= 23 { e.WriteByte(t | byte(n)) @@ -1319,7 +1321,7 @@ func getEncodeFuncInternal(t reflect.Type) (encodeFunc, isEmptyFunc) { } switch t { case typeSimpleValue: - return encodeSimpleValue, isEmptyUint + return encodeMarshalerType, isEmptyUint case typeTag: return encodeTag, alwaysNotEmpty case typeTime: diff --git a/simplevalue.go b/simplevalue.go index 14fd147e..6f93f67c 100644 --- a/simplevalue.go +++ b/simplevalue.go @@ -1,6 +1,10 @@ package cbor -import "reflect" +import ( + "errors" + "fmt" + "reflect" +) // SimpleValue represents CBOR simple value. // CBOR simple value is: @@ -16,3 +20,50 @@ type SimpleValue uint8 var ( typeSimpleValue = reflect.TypeOf(SimpleValue(0)) ) + +// MarshalCBOR encodes SimpleValue as CBOR simple value (major type 7). +func (sv SimpleValue) MarshalCBOR() ([]byte, error) { + // RFC 8949 3.3. Floating-Point Numbers and Values with No Content says: + // "An encoder MUST NOT issue two-byte sequences that start with 0xf8 + // (major type 7, additional information 24) and continue with a byte + // less than 0x20 (32 decimal). Such sequences are not well-formed. + // (This implies that an encoder cannot encode false, true, null, or + // undefined in two-byte sequences and that only the one-byte variants + // of these are well-formed; more generally speaking, each simple value + // only has a single representation variant)." + + switch { + case sv <= 23: + return []byte{byte(cborTypePrimitives) | byte(sv)}, nil + + case sv >= 32: + return []byte{byte(cborTypePrimitives) | byte(24), byte(sv)}, nil + + default: + return nil, &UnsupportedValueError{msg: fmt.Sprintf("SimpleValue(%d)", sv)} + } +} + +// UnmarshalCBOR decodes CBOR simple value (major type 7) to SimpleValue. +func (sv *SimpleValue) UnmarshalCBOR(data []byte) error { + if sv == nil { + return errors.New("cbor.SimpleValue: UnmarshalCBOR on nil pointer") + } + + d := decoder{data: data, dm: defaultDecMode} + + typ, ai, val := d.getHead() + + if typ != cborTypePrimitives { + return &UnmarshalTypeError{CBORType: typ.String(), GoType: "SimpleValue"} + } + if ai > 24 { + return &UnmarshalTypeError{CBORType: typ.String(), GoType: "SimpleValue", errorMsg: "not simple values"} + } + + // It is safe to cast val to uint8 here because + // - data is already verified to be well-formed CBOR simple value and + // - val is <= math.MaxUint8. + *sv = SimpleValue(val) + return nil +} diff --git a/simplevalue_test.go b/simplevalue_test.go new file mode 100644 index 00000000..67b11661 --- /dev/null +++ b/simplevalue_test.go @@ -0,0 +1,177 @@ +// Copyright (c) Faye Amacker. All rights reserved. +// Licensed under the MIT License. See LICENSE in the project root for license information. + +package cbor + +import ( + "bytes" + "reflect" + "testing" +) + +func TestUnmarshalSimpleValue(t *testing.T) { + t.Run("0..23", func(t *testing.T) { + for i := 0; i <= 23; i++ { + data := []byte{byte(cborTypePrimitives) | byte(i)} + want := SimpleValue(i) + + switch i { + case 20: // false + testUnmarshalSimpleValueToEmptyInterface(t, data, false) + case 21: // true + testUnmarshalSimpleValueToEmptyInterface(t, data, true) + case 22: // null + testUnmarshalSimpleValueToEmptyInterface(t, data, nil) + case 23: // undefined + testUnmarshalSimpleValueToEmptyInterface(t, data, nil) + default: + testUnmarshalSimpleValueToEmptyInterface(t, data, want) + } + + testUnmarshalSimpleValue(t, data, want) + } + }) + + t.Run("24..31", func(t *testing.T) { + for i := 24; i <= 31; i++ { + data := []byte{byte(cborTypePrimitives) | byte(24), byte(i)} + + testUnmarshalInvalidSimpleValueToEmptyInterface(t, data) + testUnmarshalInvalidSimpleValue(t, data) + } + }) + + t.Run("32..255", func(t *testing.T) { + for i := 32; i <= 255; i++ { + data := []byte{byte(cborTypePrimitives) | byte(24), byte(i)} + want := SimpleValue(i) + testUnmarshalSimpleValueToEmptyInterface(t, data, want) + testUnmarshalSimpleValue(t, data, want) + } + }) +} + +func testUnmarshalInvalidSimpleValueToEmptyInterface(t *testing.T, data []byte) { + var v interface{} + if err := Unmarshal(data, v); err == nil { + t.Errorf("Unmarshal(0x%x) didn't return an error", data) + } else if _, ok := err.(*SyntaxError); !ok { + t.Errorf("Unmarshal(0x%x) returned wrong error type %T, want (*SyntaxError)", data, err) + } +} + +func testUnmarshalInvalidSimpleValue(t *testing.T, data []byte) { + var v SimpleValue + if err := Unmarshal(data, v); err == nil { + t.Errorf("Unmarshal(0x%x) didn't return an error", data) + } else if _, ok := err.(*SyntaxError); !ok { + t.Errorf("Unmarshal(0x%x) returned wrong error type %T, want (*SyntaxError)", data, err) + } +} + +func testUnmarshalSimpleValueToEmptyInterface(t *testing.T, data []byte, want interface{}) { + var v interface{} + if err := Unmarshal(data, &v); err != nil { + t.Errorf("Unmarshal(0x%x) returned error %v", data, err) + return + } + if !reflect.DeepEqual(v, want) { + t.Errorf("Unmarshal(0x%x) = %v (%T), want %v (%T)", data, v, v, want, want) + } +} + +func testUnmarshalSimpleValue(t *testing.T, data []byte, want SimpleValue) { + cborNil := isCBORNil(data) + + // Decode to SimpleValue + var v SimpleValue + err := Unmarshal(data, &v) + if err != nil { + t.Errorf("Unmarshal(0x%x) returned error %v", data, err) + return + } + if !reflect.DeepEqual(v, want) { + t.Errorf("Unmarshal(0x%x) = %v (%T), want %v (%T)", data, v, v, want, want) + } + + // Decode to uninitialized *SimpleValue + var pv *SimpleValue + err = Unmarshal(data, &pv) + if err != nil { + t.Errorf("Unmarshal(0x%x) returned error %v", data, err) + return + } + if cborNil { + if pv != nil { + t.Errorf("Unmarshal(0x%x) returned %v, want nil *SimpleValue", data, *pv) + } + } else { + if !reflect.DeepEqual(*pv, want) { + t.Errorf("Unmarshal(0x%x) = %v (%T), want %v (%T)", data, *pv, *pv, want, want) + } + } + + // Decode to initialized *SimpleValue + v = SimpleValue(0) + pv = &v + err = Unmarshal(data, &pv) + if err != nil { + t.Errorf("Unmarshal(0x%x) returned error %v", data, err) + return + } + if cborNil { + if pv != nil { + t.Errorf("Unmarshal(0x%x) returned %v, want nil *SimpleValue", data, *pv) + } + } else { + if !reflect.DeepEqual(v, want) { + t.Errorf("Unmarshal(0x%x) = %v (%T), want %v (%T)", data, v, v, want, want) + } + } +} + +func TestMarshalSimpleValue(t *testing.T) { + t.Run("0..23", func(t *testing.T) { + for i := 0; i <= 23; i++ { + wantData := []byte{byte(cborTypePrimitives) | byte(i)} + v := SimpleValue(i) + + data, err := Marshal(v) + if err != nil { + t.Errorf("Marshal(%v) returned error %v", v, err) + continue + } + if !bytes.Equal(data, wantData) { + t.Errorf("Marshal(%v) = 0x%x, want 0x%x", v, data, wantData) + } + } + }) + + t.Run("24..31", func(t *testing.T) { + for i := 24; i <= 31; i++ { + v := SimpleValue(i) + + if data, err := Marshal(v); err == nil { + t.Errorf("Marshal(%v) didn't return an error", data) + } else if _, ok := err.(*UnsupportedValueError); !ok { + t.Errorf("Marshal(%v) returned wrong error type %T, want (*UnsupportedValueError)", data, err) + } + } + }) + + t.Run("32..255", func(t *testing.T) { + for i := 32; i <= 255; i++ { + wantData := []byte{byte(cborTypePrimitives) | byte(24), byte(i)} + v := SimpleValue(i) + + data, err := Marshal(v) + if err != nil { + t.Errorf("Marshal(%v) returned error %v", v, err) + continue + } + if !bytes.Equal(data, wantData) { + t.Errorf("Marshal(%v) = 0x%x, want 0x%x", v, data, wantData) + } + } + }) +}