From 61654e9f5da57d1b44a8b4595b4d10275079a726 Mon Sep 17 00:00:00 2001 From: Ben Luddy Date: Wed, 15 May 2024 13:49:42 -0400 Subject: [PATCH] Reject math/big.Int on encode and bignum tags on decode for CBOR. Although CBOR can roundtrip arbitrary precision integers, they don't roundtrip properly through Unstructured because Unstructured is limited to the dynamic numeric types int64 and float64. Rather than serializing them (and potentially losing information), reject them explicitly with an error. --- .../cbor/internal/modes/appendixa_test.go | 2 -- .../serializer/cbor/internal/modes/decode.go | 4 +++ .../cbor/internal/modes/decode_test.go | 28 ++++++++----------- .../serializer/cbor/internal/modes/encode.go | 9 +++--- .../cbor/internal/modes/encode_test.go | 13 ++++++++- 5 files changed, 32 insertions(+), 24 deletions(-) diff --git a/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/internal/modes/appendixa_test.go b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/internal/modes/appendixa_test.go index dec784b2526..7442e584079 100644 --- a/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/internal/modes/appendixa_test.go +++ b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/internal/modes/appendixa_test.go @@ -121,7 +121,6 @@ func TestAppendixA(t *testing.T) { { example: hex("c249010000000000000000"), reject: "decoding tagged positive bigint value to interface{} can't reproduce this value without losing distinction between float and integer", - fixme: "decoding bigint to interface{} must not produce math/big.Int", }, { example: hex("3bffffffffffffffff"), @@ -130,7 +129,6 @@ func TestAppendixA(t *testing.T) { { example: hex("c349010000000000000000"), reject: "-18446744073709551617 overflows int64 and falling back to float64 (as with JSON) loses distinction between float and integer", - fixme: "decoding negative bigint to interface{} must not produce math/big.Int", }, { example: hex("20"), diff --git a/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/internal/modes/decode.go b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/internal/modes/decode.go index df8a33a0745..14c56970569 100644 --- a/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/internal/modes/decode.go +++ b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/internal/modes/decode.go @@ -96,6 +96,10 @@ var Decode cbor.DecMode = func() cbor.DecMode { // representation (RFC 8259 Section 6). NaN: cbor.NaNDecodeForbidden, Inf: cbor.InfDecodeForbidden, + + // Reject the arbitrary-precision integer tags because they can't be faithfully + // roundtripped through the allowable Unstructured types. + BignumTag: cbor.BignumTagForbidden, }.DecMode() if err != nil { panic(err) diff --git a/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/internal/modes/decode_test.go b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/internal/modes/decode_test.go index 3472ec436b6..e10468a79d7 100644 --- a/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/internal/modes/decode_test.go +++ b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/internal/modes/decode_test.go @@ -739,29 +739,25 @@ func TestDecode(t *testing.T) { group(t, "unsigned bignum", []test{ { - name: "rejected", - in: hex("c249010000000000000000"), // 2(18446744073709551616) - fixme: "decoding cbor data tagged with 2 produces big.Int instead of rejecting", - assertOnError: func(t *testing.T, e error) { - // TODO: Once this can pass, make the assertion stronger. - if e == nil { - t.Error("expected non-nil error") + name: "rejected", + in: hex("c249010000000000000000"), // 2(18446744073709551616) + assertOnError: assertOnConcreteError(func(t *testing.T, e *cbor.UnacceptableDataItemError) { + if diff := cmp.Diff(&cbor.UnacceptableDataItemError{CBORType: "tag", Message: "bignum"}, e); diff != "" { + t.Errorf("unexpected error diff:\n%s", diff) } - }, + }), }, }) group(t, "negative bignum", []test{ { - name: "rejected", - in: hex("c349010000000000000000"), // 3(-18446744073709551617) - fixme: "decoding cbor data tagged with 3 produces big.Int instead of rejecting", - assertOnError: func(t *testing.T, e error) { - // TODO: Once this can pass, make the assertion stronger. - if e == nil { - t.Error("expected non-nil error") + name: "rejected", + in: hex("c349010000000000000000"), // 3(-18446744073709551617) + assertOnError: assertOnConcreteError(func(t *testing.T, e *cbor.UnacceptableDataItemError) { + if diff := cmp.Diff(&cbor.UnacceptableDataItemError{CBORType: "tag", Message: "bignum"}, e); diff != "" { + t.Errorf("unexpected error diff:\n%s", diff) } - }, + }), }, }) diff --git a/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/internal/modes/encode.go b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/internal/modes/encode.go index 16a05c5e57d..ac839a53521 100644 --- a/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/internal/modes/encode.go +++ b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/internal/modes/encode.go @@ -37,11 +37,10 @@ var Encode cbor.EncMode = func() cbor.EncMode { NaNConvert: cbor.NaNConvertReject, InfConvert: cbor.InfConvertReject, - // Prefer encoding math/big.Int to one of the 64-bit integer types if it fits. When - // later decoded into Unstructured, the set of allowable concrete numeric types is - // limited to int64 and float64, so the distinction between big integer and integer - // can't be preserved. - BigIntConvert: cbor.BigIntConvertShortest, + // Error on attempt to encode math/big.Int values, which can't be faithfully + // roundtripped through Unstructured in general (the dynamic numeric types allowed + // in Unstructured are limited to float64 and int64). + BigIntConvert: cbor.BigIntConvertReject, // MarshalJSON for time.Time writes RFC3339 with nanos. Time: cbor.TimeRFC3339Nano, diff --git a/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/internal/modes/encode_test.go b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/internal/modes/encode_test.go index 1d9b8e47880..e9d8906e502 100644 --- a/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/internal/modes/encode_test.go +++ b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/cbor/internal/modes/encode_test.go @@ -18,6 +18,8 @@ package modes_test import ( "fmt" + "math/big" + "reflect" "testing" "github.com/fxamacker/cbor/v2" @@ -53,6 +55,15 @@ func TestEncode(t *testing.T) { want: []byte{0xa1, 0x41, 0x41, 0x02}, // {"A": 2} assertOnError: assertNilError, }, + { + name: "math/big.Int values are rejected", + in: big.NewInt(1), + assertOnError: assertOnConcreteError(func(t *testing.T, got *cbor.UnsupportedTypeError) { + if want := (&cbor.UnsupportedTypeError{Type: reflect.TypeFor[big.Int]()}); *want != *got { + t.Errorf("unexpected error, got %#v (%q), want %#v (%q)", got, got.Error(), want, want.Error()) + } + }), + }, } { encModes := tc.modes if len(encModes) == 0 { @@ -68,7 +79,7 @@ func TestEncode(t *testing.T) { t.Run(fmt.Sprintf("mode=%s/%s", modeName, tc.name), func(t *testing.T) { out, err := encMode.Marshal(tc.in) tc.assertOnError(t, err) - if diff := cmp.Diff(tc.want, out); diff != "" { + if diff := cmp.Diff(tc.want, out, cmp.Comparer(func(a, b reflect.Type) bool { return a == b })); diff != "" { t.Errorf("unexpected output:\n%s", diff) } })