Skip to content

Commit

Permalink
Decouple time.Time parsing from empty interface behavior.
Browse files Browse the repository at this point in the history 
Due to an implementation detail, options controlling decodes into empty interface values could in
some cases affect the behavior of decoding into time.Time. That is fixed. This patch also clarifies
the documentation and reconciles a couple edge-case behaviors:

1. In TimeTag mode DecTagIgnored, decoding a null or undefined simple value enclosed in a tag other
than 0 or 1 to time.Time is now a no-op. This is the same as the existing behavior when decoding an
untagged null or undefined.

2. The content type enclosed by tags 0 and 1 were not being validated if enclosed within an
unrecognized tag. This has been fixed.

Signed-off-by: Ben Luddy <[email protected]>
  • Loading branch information
@benluddy
benluddy committed Mar 13, 2024
1 parent 4a6e6d1 commit 865ac96
Show file tree
Hide file tree
Showing 2 changed files with 146 additions and 55 deletions.
156 changes: 107 additions & 49 deletions decode.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,8 @@ import (
// To unmarshal a CBOR text string into a time.Time value, Unmarshal parses text
// string formatted in RFC3339. To unmarshal a CBOR integer/float into a
// time.Time value, Unmarshal creates an unix time with integer/float as seconds
// and fractional seconds since January 1, 1970 UTC.
// and fractional seconds since January 1, 1970 UTC. As a special case, Infinite
// and NaN float values decode to time.Time's zero value.
//
// To unmarshal CBOR null (0xf6) and undefined (0xf7) values into a
// slice/map/pointer, Unmarshal sets Go value to nil. Because null is often
Expand Down Expand Up @@ -476,8 +477,28 @@ type DecOptions struct {
// DupMapKey specifies whether to enforce duplicate map key.
DupMapKey DupMapKeyMode

// TimeTag specifies whether to check validity of time.Time (e.g. valid tag number and tag content type).
// For now, valid tag number means 0 or 1 as specified in RFC 7049 if the Go type is time.Time.
// TimeTag specifies whether or not untagged data items, or tags other
// than tag 0 and tag 1, can be decoded to time.Time. If tag 0 or tag 1
// appears in an input, the type of its content is always validated as
// specified in RFC 8949. That behavior is not controlled by this
// option. The behavior of the supported modes are:
//
// DecTagIgnored (default): Untagged text strings and text strings
// enclosed in tags other than 0 and 1 are decoded as though enclosed
// in tag 0. Untagged unsigned integers, negative integers, and
// floating-point numbers (or those enclosed in tags other than 0 and
// 1) are decoded as though enclosed in tag 1. Decoding a tag other
// than 0 or 1 enclosing simple values null or undefined into a
// time.Time does not modify the destination value.
//
// DecTagOptional: Untagged text strings are decoded as though
// enclosed in tag 0. Untagged unsigned integers, negative integers,
// and floating-point numbers are decoded as though enclosed in tag
// 1. Tags other than 0 and 1 will produce an error on attempts to
// decode them into a time.Time.
//
// DecTagRequired: Only tags 0 and 1 can be decoded to time.Time. Any
// other input will produce an error.
TimeTag DecTagMode

// MaxNestedLevels specifies the max nested levels allowed for any combination of CBOR array, maps, and tags.
Expand Down Expand Up @@ -1024,15 +1045,15 @@ func (d *decoder) parseToValue(v reflect.Value, tInfo *typeInfo) error { //nolin
}

// Check validity of supported built-in tags.
if d.nextCBORType() == cborTypeTag {
off := d.off
off := d.off
for d.nextCBORType() == cborTypeTag {
_, _, tagNum := d.getHead()
if err := validBuiltinTag(tagNum, d.data[d.off]); err != nil {
d.skip()
return err
}
d.off = off
}
d.off = off

if tInfo.spclType != specialTypeNone {
switch tInfo.spclType {
Expand All @@ -1050,11 +1071,13 @@ func (d *decoder) parseToValue(v reflect.Value, tInfo *typeInfo) error { //nolin
d.skip()
return nil
}
tm, err := d.parseToTime()
tm, ok, err := d.parseToTime()
if err != nil {
return err
}
v.Set(reflect.ValueOf(tm))
if ok {
v.Set(reflect.ValueOf(tm))
}
return nil
case specialTypeUnmarshalerIface:
return d.parseToUnmarshaler(v)
Expand Down Expand Up @@ -1239,64 +1262,98 @@ func (d *decoder) parseToTag(v reflect.Value) error {
return nil
}

func (d *decoder) parseToTime() (tm time.Time, err error) {
t := d.nextCBORType()

// parseToTime decodes the current data item as a time.Time. The bool return value is false if and
// only if the destination value should remain unmodified.
func (d *decoder) parseToTime() (time.Time, bool, error) {
// Verify that tag number or absence of tag number is acceptable to specified timeTag.
if t == cborTypeTag {
if t := d.nextCBORType(); t == cborTypeTag {
if d.dm.timeTag == DecTagIgnored {
// Skip tag number
// Skip all enclosing tags
for t == cborTypeTag {
d.getHead()
t = d.nextCBORType()
}
if d.nextCBORNil() {
d.skip()
return time.Time{}, false, nil
}
} else {
// Read tag number
_, _, tagNum := d.getHead()
if tagNum != 0 && tagNum != 1 {
d.skip()
err = errors.New("cbor: wrong tag number for time.Time, got " + strconv.Itoa(int(tagNum)) + ", expect 0 or 1")
return
d.skip() // skip tag content
return time.Time{}, false, errors.New("cbor: wrong tag number for time.Time, got " + strconv.Itoa(int(tagNum)) + ", expect 0 or 1")
}
}
} else {
if d.dm.timeTag == DecTagRequired {
d.skip()
err = &UnmarshalTypeError{CBORType: t.String(), GoType: typeTime.String(), errorMsg: "expect CBOR tag value"}
return
return time.Time{}, false, &UnmarshalTypeError{CBORType: t.String(), GoType: typeTime.String(), errorMsg: "expect CBOR tag value"}
}
}

var content interface{}
content, err = d.parse(false)
if err != nil {
return
}

switch c := content.(type) {
case nil:
return
case uint64:
return time.Unix(int64(c), 0), nil
case int64:
return time.Unix(c, 0), nil
case float64:
if math.IsNaN(c) || math.IsInf(c, 0) {
return
}
f1, f2 := math.Modf(c)
return time.Unix(int64(f1), int64(f2*1e9)), nil
case string:
tm, err = time.Parse(time.RFC3339, c)
switch t := d.nextCBORType(); t {
case cborTypeTextString:
s, err := d.parseTextString()
if err != nil {
tm = time.Time{}
err = errors.New("cbor: cannot set " + c + " for time.Time: " + err.Error())
return
return time.Time{}, false, err
}
return
t, err := time.Parse(time.RFC3339, string(s))
if err != nil {
return time.Time{}, false, errors.New("cbor: cannot set " + string(s) + " for time.Time: " + err.Error())
}
return t, true, nil
case cborTypePositiveInt:
_, _, val := d.getHead()
if val > math.MaxInt64 {
return time.Time{}, false, &UnmarshalTypeError{
CBORType: t.String(),
GoType: typeTime.String(),
errorMsg: fmt.Sprintf("%d overflows Go's int64", val),
}
}
return time.Unix(int64(val), 0), true, nil
case cborTypeNegativeInt:
_, _, val := d.getHead()
if val > math.MaxInt64 {
if val == math.MaxUint64 {
// Maximum absolute value representable by negative integer is 2^64,
// not 2^64-1, so it overflows uint64.
return time.Time{}, false, &UnmarshalTypeError{
CBORType: t.String(),
GoType: typeTime.String(),
errorMsg: "-18446744073709551616 overflows Go's int64",
}
}
return time.Time{}, false, &UnmarshalTypeError{
CBORType: t.String(),
GoType: typeTime.String(),
errorMsg: fmt.Sprintf("-%d overflows Go's int64", val+1),
}
}
return time.Unix(int64(-1)^int64(val), 0), true, nil
case cborTypePrimitives:
_, ai, val := d.getHead()
var f float64
switch ai {
case 25:
f = float64(float16.Frombits(uint16(val)).Float32())
case 26:
f = float64(math.Float32frombits(uint32(val)))
case 27:
f = math.Float64frombits(val)
default:
return time.Time{}, false, &UnmarshalTypeError{CBORType: t.String(), GoType: typeTime.String()}
}

if math.IsNaN(f) || math.IsInf(f, 0) {
// https://www.rfc-editor.org/rfc/rfc8949.html#section-3.4.2-6
return time.Time{}, true, nil
}
seconds, fractional := math.Modf(f)
return time.Unix(int64(seconds), int64(fractional*1e9)), true, nil
default:
err = &UnmarshalTypeError{CBORType: t.String(), GoType: typeTime.String()}
return
return time.Time{}, false, &UnmarshalTypeError{CBORType: t.String(), GoType: typeTime.String()}
}
}

Expand Down Expand Up @@ -1336,15 +1393,15 @@ func (d *decoder) parse(skipSelfDescribedTag bool) (interface{}, error) { //noli
}

// Check validity of supported built-in tags.
if d.nextCBORType() == cborTypeTag {
off := d.off
off := d.off
for d.nextCBORType() == cborTypeTag {
_, _, tagNum := d.getHead()
if err := validBuiltinTag(tagNum, d.data[d.off]); err != nil {
d.skip()
return nil, err
}
d.off = off
}
d.off = off

t := d.nextCBORType()
switch t {
Expand Down Expand Up @@ -1445,7 +1502,8 @@ func (d *decoder) parse(skipSelfDescribedTag bool) (interface{}, error) { //noli
switch tagNum {
case 0, 1:
d.off = tagOff
return d.parseToTime()
tm, _, err := d.parseToTime()
return tm, err
case 2:
b, _ := d.parseByteString()
bi := new(big.Int).SetBytes(b)
Expand Down
45 changes: 39 additions & 6 deletions decode_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3547,13 +3547,27 @@ func TestMapKeyNil(t *testing.T) {
}

func TestDecodeTime(t *testing.T) {
unmodified := time.Now()

testCases := []struct {
name string
cborRFC3339Time []byte
cborUnixTime []byte
wantTime time.Time
}{
// Decoding CBOR null/defined to time.Time is no-op. See TestUnmarshalNil.
// Decoding untagged CBOR null/defined to time.Time is no-op. See TestUnmarshalNil.
{
name: "null within unrecognized tag", // no-op in DecTagIgnored
cborRFC3339Time: hexDecode("dadeadbeeff6"),
cborUnixTime: hexDecode("dadeadbeeff6"),
wantTime: unmodified,
},
{
name: "undefined within unrecognized tag", // no-op in DecTagIgnored
cborRFC3339Time: hexDecode("dadeadbeeff7"),
cborUnixTime: hexDecode("dadeadbeeff7"),
wantTime: unmodified,
},
{
name: "NaN",
cborRFC3339Time: hexDecode("f97e00"),
Expand Down Expand Up @@ -3599,13 +3613,13 @@ func TestDecodeTime(t *testing.T) {
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
tm := time.Now()
tm := unmodified
if err := Unmarshal(tc.cborRFC3339Time, &tm); err != nil {
t.Errorf("Unmarshal(0x%x) returned error %v", tc.cborRFC3339Time, err)
} else if !tc.wantTime.Equal(tm) {
t.Errorf("Unmarshal(0x%x) = %v (%T), want %v (%T)", tc.cborRFC3339Time, tm, tm, tc.wantTime, tc.wantTime)
}
tm = time.Now()
tm = unmodified
if err := Unmarshal(tc.cborUnixTime, &tm); err != nil {
t.Errorf("Unmarshal(0x%x) returned error %v", tc.cborUnixTime, err)
} else if !tc.wantTime.Equal(tm) {
Expand Down Expand Up @@ -3675,6 +3689,7 @@ func TestDecodeTimeWithTag(t *testing.T) {
func TestDecodeTimeError(t *testing.T) {
testCases := []struct {
name string
opts DecOptions
data []byte
wantErrorMsg string
}{
Expand Down Expand Up @@ -3703,11 +3718,29 @@ func TestDecodeTimeError(t *testing.T) {
data: hexDecode("3bffffffffffffffff"),
wantErrorMsg: "cbor: cannot unmarshal negative integer into Go value of type time.Time",
},
{
name: "untagged byte string content cannot be decoded into time.Time with DefaultByteStringType string",
opts: DecOptions{
TimeTag: DecTagOptional,
DefaultByteStringType: reflect.TypeOf(""),
},
data: hexDecode("54323031332d30332d32315432303a30343a30305a"),
wantErrorMsg: "cbor: cannot unmarshal byte string into Go value of type time.Time",
},
{
name: "time tag is validated when enclosed in unrecognized tag",
data: hexDecode("dadeadbeefc001"),
wantErrorMsg: "cbor: tag number 0 must be followed by text string, got positive integer",
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
dm, err := tc.opts.DecMode()
if err != nil {
t.Fatal(err)
}
tm := time.Now()
if err := Unmarshal(tc.data, &tm); err == nil {
if err := dm.Unmarshal(tc.data, &tm); err == nil {
t.Errorf("Unmarshal(0x%x) didn't return an error, want error msg %q", tc.data, tc.wantErrorMsg)
} else if !strings.Contains(err.Error(), tc.wantErrorMsg) {
t.Errorf("Unmarshal(0x%x) returned error %q, want %q", tc.data, err.Error(), tc.wantErrorMsg)
Expand Down Expand Up @@ -3759,7 +3792,7 @@ func TestDecodeInvalidTagTime(t *testing.T) {
name: "Tag 1 with negative integer overflow",
data: hexDecode("c13bffffffffffffffff"),
decodeToTypes: []reflect.Type{typeIntf, typeTime},
wantErrorMsg: "cbor: cannot unmarshal tag into Go value of type time.Time",
wantErrorMsg: "cbor: cannot unmarshal negative integer into Go value of type time.Time (-18446744073709551616 overflows Go's int64)",
},
{
name: "Tag 1 with string content",
Expand Down Expand Up @@ -3912,7 +3945,7 @@ func TestDecodeTimeStreaming(t *testing.T) {
},
{
data: hexDecode("c13bffffffffffffffff"),
wantErrorMsg: "cbor: cannot unmarshal tag into Go value of type time.Time",
wantErrorMsg: "cbor: cannot unmarshal negative integer into Go value of type time.Time (-18446744073709551616 overflows Go's int64)",
},
{
data: hexDecode("c11a514b67b0"),
Expand Down

0 comments on commit 865ac96

Please sign in to comment.