Skip to content

Commit 5764500

Browse files
authored
Merge pull request kubernetes#125420 from benluddy/cbor-bignum-bigint
KEP-4222: Reject math/big.Int on encode and bignum tags on decode for CBOR.
2 parents 902088a + 61654e9 commit 5764500

File tree

5 files changed

+32
-24
lines changed

5 files changed

+32
-24
lines changed

staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/internal/modes/appendixa_test.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,6 @@ func TestAppendixA(t *testing.T) {
121121
{
122122
example: hex("c249010000000000000000"),
123123
reject: "decoding tagged positive bigint value to interface{} can't reproduce this value without losing distinction between float and integer",
124-
fixme: "decoding bigint to interface{} must not produce math/big.Int",
125124
},
126125
{
127126
example: hex("3bffffffffffffffff"),
@@ -130,7 +129,6 @@ func TestAppendixA(t *testing.T) {
130129
{
131130
example: hex("c349010000000000000000"),
132131
reject: "-18446744073709551617 overflows int64 and falling back to float64 (as with JSON) loses distinction between float and integer",
133-
fixme: "decoding negative bigint to interface{} must not produce math/big.Int",
134132
},
135133
{
136134
example: hex("20"),

staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/internal/modes/decode.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,10 @@ var Decode cbor.DecMode = func() cbor.DecMode {
9696
// representation (RFC 8259 Section 6).
9797
NaN: cbor.NaNDecodeForbidden,
9898
Inf: cbor.InfDecodeForbidden,
99+
100+
// Reject the arbitrary-precision integer tags because they can't be faithfully
101+
// roundtripped through the allowable Unstructured types.
102+
BignumTag: cbor.BignumTagForbidden,
99103
}.DecMode()
100104
if err != nil {
101105
panic(err)

staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/internal/modes/decode_test.go

Lines changed: 12 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -739,29 +739,25 @@ func TestDecode(t *testing.T) {
739739

740740
group(t, "unsigned bignum", []test{
741741
{
742-
name: "rejected",
743-
in: hex("c249010000000000000000"), // 2(18446744073709551616)
744-
fixme: "decoding cbor data tagged with 2 produces big.Int instead of rejecting",
745-
assertOnError: func(t *testing.T, e error) {
746-
// TODO: Once this can pass, make the assertion stronger.
747-
if e == nil {
748-
t.Error("expected non-nil error")
742+
name: "rejected",
743+
in: hex("c249010000000000000000"), // 2(18446744073709551616)
744+
assertOnError: assertOnConcreteError(func(t *testing.T, e *cbor.UnacceptableDataItemError) {
745+
if diff := cmp.Diff(&cbor.UnacceptableDataItemError{CBORType: "tag", Message: "bignum"}, e); diff != "" {
746+
t.Errorf("unexpected error diff:\n%s", diff)
749747
}
750-
},
748+
}),
751749
},
752750
})
753751

754752
group(t, "negative bignum", []test{
755753
{
756-
name: "rejected",
757-
in: hex("c349010000000000000000"), // 3(-18446744073709551617)
758-
fixme: "decoding cbor data tagged with 3 produces big.Int instead of rejecting",
759-
assertOnError: func(t *testing.T, e error) {
760-
// TODO: Once this can pass, make the assertion stronger.
761-
if e == nil {
762-
t.Error("expected non-nil error")
754+
name: "rejected",
755+
in: hex("c349010000000000000000"), // 3(-18446744073709551617)
756+
assertOnError: assertOnConcreteError(func(t *testing.T, e *cbor.UnacceptableDataItemError) {
757+
if diff := cmp.Diff(&cbor.UnacceptableDataItemError{CBORType: "tag", Message: "bignum"}, e); diff != "" {
758+
t.Errorf("unexpected error diff:\n%s", diff)
763759
}
764-
},
760+
}),
765761
},
766762
})
767763

staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/internal/modes/encode.go

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -37,11 +37,10 @@ var Encode cbor.EncMode = func() cbor.EncMode {
3737
NaNConvert: cbor.NaNConvertReject,
3838
InfConvert: cbor.InfConvertReject,
3939

40-
// Prefer encoding math/big.Int to one of the 64-bit integer types if it fits. When
41-
// later decoded into Unstructured, the set of allowable concrete numeric types is
42-
// limited to int64 and float64, so the distinction between big integer and integer
43-
// can't be preserved.
44-
BigIntConvert: cbor.BigIntConvertShortest,
40+
// Error on attempt to encode math/big.Int values, which can't be faithfully
41+
// roundtripped through Unstructured in general (the dynamic numeric types allowed
42+
// in Unstructured are limited to float64 and int64).
43+
BigIntConvert: cbor.BigIntConvertReject,
4544

4645
// MarshalJSON for time.Time writes RFC3339 with nanos.
4746
Time: cbor.TimeRFC3339Nano,

staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/internal/modes/encode_test.go

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ package modes_test
1818

1919
import (
2020
"fmt"
21+
"math/big"
22+
"reflect"
2123
"testing"
2224

2325
"github.com/fxamacker/cbor/v2"
@@ -53,6 +55,15 @@ func TestEncode(t *testing.T) {
5355
want: []byte{0xa1, 0x41, 0x41, 0x02}, // {"A": 2}
5456
assertOnError: assertNilError,
5557
},
58+
{
59+
name: "math/big.Int values are rejected",
60+
in: big.NewInt(1),
61+
assertOnError: assertOnConcreteError(func(t *testing.T, got *cbor.UnsupportedTypeError) {
62+
if want := (&cbor.UnsupportedTypeError{Type: reflect.TypeFor[big.Int]()}); *want != *got {
63+
t.Errorf("unexpected error, got %#v (%q), want %#v (%q)", got, got.Error(), want, want.Error())
64+
}
65+
}),
66+
},
5667
} {
5768
encModes := tc.modes
5869
if len(encModes) == 0 {
@@ -68,7 +79,7 @@ func TestEncode(t *testing.T) {
6879
t.Run(fmt.Sprintf("mode=%s/%s", modeName, tc.name), func(t *testing.T) {
6980
out, err := encMode.Marshal(tc.in)
7081
tc.assertOnError(t, err)
71-
if diff := cmp.Diff(tc.want, out); diff != "" {
82+
if diff := cmp.Diff(tc.want, out, cmp.Comparer(func(a, b reflect.Type) bool { return a == b })); diff != "" {
7283
t.Errorf("unexpected output:\n%s", diff)
7384
}
7485
})

0 commit comments

Comments
 (0)