From 2a31354e266e829d6d63a776b933947b4118436e Mon Sep 17 00:00:00 2001 From: Ben Luddy Date: Thu, 9 May 2024 15:19:51 -0400 Subject: [PATCH] Reject NaN or infinite floating-point values in the CBOR serializer. --- .../cbor/internal/modes/appendixa_test.go | 47 ++----- .../serializer/cbor/internal/modes/decode.go | 5 + .../cbor/internal/modes/decode_test.go | 117 +++++++++--------- .../serializer/cbor/internal/modes/encode.go | 10 +- 4 files changed, 75 insertions(+), 104 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 c80f48e3dd7..5075a426b6d 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 @@ -56,8 +56,6 @@ func TestAppendixA(t *testing.T) { const ( reasonArrayFixedLength = "indefinite-length arrays are re-encoded with fixed length" reasonByteString = "strings are encoded as the byte string major type" - reasonFloatPacked = "floats are packed into the smallest value-preserving width" - reasonNaN = "all NaN values are represented with a single encoding" reasonMapFixedLength = "indefinite-length maps are re-encoded with fixed length" reasonMapSorted = "map entries are sorted" reasonStringFixedLength = "indefinite-length strings are re-encoded with fixed length" @@ -202,68 +200,41 @@ func TestAppendixA(t *testing.T) { example: hex("fbc010666666666666"), decoded: -4.1, }, - // TODO: Should Inf/-Inf/NaN be supported? Current Protobuf will encode this, but - // JSON will produce an error. This is less than ideal -- we can't transcode - // everything to JSON. { example: hex("f97c00"), - decoded: math.Inf(1), + reject: "floating-point NaN and infinities are not accepted", }, { example: hex("f97e00"), - decoded: math.Float64frombits(0x7ff8000000000000), + reject: "floating-point NaN and infinities are not accepted", }, { example: hex("f9fc00"), - decoded: math.Inf(-1), + reject: "floating-point NaN and infinities are not accepted", }, { example: hex("fa7f800000"), - decoded: math.Inf(1), - encoded: hex("f97c00"), - reasons: []string{ - reasonFloatPacked, - }, + reject: "floating-point NaN and infinities are not accepted", }, { example: hex("fa7fc00000"), - decoded: math.NaN(), - encoded: hex("f97e00"), - reasons: []string{ - reasonNaN, - }, + reject: "floating-point NaN and infinities are not accepted", }, { example: hex("faff800000"), - decoded: math.Inf(-1), - encoded: hex("f9fc00"), - reasons: []string{ - reasonFloatPacked, - }, + reject: "floating-point NaN and infinities are not accepted", }, { example: hex("fb7ff0000000000000"), - decoded: math.Inf(1), - encoded: hex("f97c00"), - reasons: []string{ - reasonFloatPacked, - }, + reject: "floating-point NaN and infinities are not accepted", }, { example: hex("fb7ff8000000000000"), - decoded: math.NaN(), - encoded: hex("f97e00"), - reasons: []string{ - reasonNaN, - }, + reject: "floating-point NaN and infinities are not accepted", }, { example: hex("fbfff0000000000000"), - decoded: math.Inf(-1), - encoded: hex("f9fc00"), - reasons: []string{ - reasonFloatPacked, - }, + reject: "floating-point NaN and infinities are not accepted", }, { example: hex("f4"), 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 f1290989754..f2c7795039d 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 @@ -88,6 +88,11 @@ var Decode cbor.DecMode = func() cbor.DecMode { // For parity with JSON, strings can be decoded into time.Time if they are RFC 3339 // timestamps. ByteStringToTime: cbor.ByteStringToTimeAllowed, + + // Reject NaN and infinite floating-point values since they don't have a JSON + // representation (RFC 8259 Section 6). + NaN: cbor.NaNDecodeForbidden, + Inf: cbor.InfDecodeForbidden, }.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 57ddef9fed3..ecca0b7787e 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 @@ -435,92 +435,83 @@ func TestDecode(t *testing.T) { { name: "half precision infinity", in: hex("f97c00"), - assertOnError: func(t *testing.T, e error) { - if e == nil { - t.Fatal("expected non-nil error") + assertOnError: assertOnConcreteError(func(t *testing.T, e *cbor.UnacceptableDataItemError) { + if diff := cmp.Diff(&cbor.UnacceptableDataItemError{CBORType: "primitives", Message: "floating-point infinity"}, e); diff != "" { + t.Errorf("unexpected error diff:\n%s", diff) } - }, - fixme: "NaN and positive/negative infinities should be rejected", + }), }, { name: "single precision infinity", in: hex("fa7f800000"), - assertOnError: func(t *testing.T, e error) { - if e == nil { - t.Fatal("expected non-nil error") + assertOnError: assertOnConcreteError(func(t *testing.T, e *cbor.UnacceptableDataItemError) { + if diff := cmp.Diff(&cbor.UnacceptableDataItemError{CBORType: "primitives", Message: "floating-point infinity"}, e); diff != "" { + t.Errorf("unexpected error diff:\n%s", diff) } - }, - fixme: "NaN and positive/negative infinities should be rejected", + }), }, { name: "double precision infinity", in: hex("fb7ff0000000000000"), - assertOnError: func(t *testing.T, e error) { - if e == nil { - t.Fatal("expected non-nil error") + assertOnError: assertOnConcreteError(func(t *testing.T, e *cbor.UnacceptableDataItemError) { + if diff := cmp.Diff(&cbor.UnacceptableDataItemError{CBORType: "primitives", Message: "floating-point infinity"}, e); diff != "" { + t.Errorf("unexpected error diff:\n%s", diff) } - }, - fixme: "NaN and positive/negative infinities should be rejected", + }), }, { name: "half precision negative infinity", in: hex("f9fc00"), - assertOnError: func(t *testing.T, e error) { - if e == nil { - t.Fatal("expected non-nil error") + assertOnError: assertOnConcreteError(func(t *testing.T, e *cbor.UnacceptableDataItemError) { + if diff := cmp.Diff(&cbor.UnacceptableDataItemError{CBORType: "primitives", Message: "floating-point infinity"}, e); diff != "" { + t.Errorf("unexpected error diff:\n%s", diff) } - }, - fixme: "NaN and positive/negative infinities should be rejected", + }), }, { name: "single precision negative infinity", in: hex("faff800000"), - assertOnError: func(t *testing.T, e error) { - if e == nil { - t.Fatal("expected non-nil error") + assertOnError: assertOnConcreteError(func(t *testing.T, e *cbor.UnacceptableDataItemError) { + if diff := cmp.Diff(&cbor.UnacceptableDataItemError{CBORType: "primitives", Message: "floating-point infinity"}, e); diff != "" { + t.Errorf("unexpected error diff:\n%s", diff) } - }, - fixme: "NaN and positive/negative infinities should be rejected", + }), }, { name: "double precision negative infinity", in: hex("fbfff0000000000000"), - assertOnError: func(t *testing.T, e error) { - if e == nil { - t.Fatal("expected non-nil error") + assertOnError: assertOnConcreteError(func(t *testing.T, e *cbor.UnacceptableDataItemError) { + if diff := cmp.Diff(&cbor.UnacceptableDataItemError{CBORType: "primitives", Message: "floating-point infinity"}, e); diff != "" { + t.Errorf("unexpected error diff:\n%s", diff) } - }, - fixme: "NaN and positive/negative infinities should be rejected", + }), }, { name: "half precision NaN", in: hex("f97e00"), - assertOnError: func(t *testing.T, e error) { - if e == nil { - t.Fatal("expected non-nil error") + assertOnError: assertOnConcreteError(func(t *testing.T, e *cbor.UnacceptableDataItemError) { + if diff := cmp.Diff(&cbor.UnacceptableDataItemError{CBORType: "primitives", Message: "floating-point NaN"}, e); diff != "" { + t.Errorf("unexpected error diff:\n%s", diff) } - }, - fixme: "NaN and positive/negative infinities should be rejected", + }), }, { name: "single precision NaN", in: hex("fa7fc00000"), - assertOnError: func(t *testing.T, e error) { - if e == nil { - t.Fatal("expected non-nil error") + assertOnError: assertOnConcreteError(func(t *testing.T, e *cbor.UnacceptableDataItemError) { + if diff := cmp.Diff(&cbor.UnacceptableDataItemError{CBORType: "primitives", Message: "floating-point NaN"}, e); diff != "" { + t.Errorf("unexpected error diff:\n%s", diff) } - }, - fixme: "NaN and positive/negative infinities should be rejected", + }), }, { name: "double precision NaN", in: hex("fb7ff8000000000000"), - assertOnError: func(t *testing.T, e error) { - if e == nil { - t.Fatal("expected non-nil error") + assertOnError: assertOnConcreteError(func(t *testing.T, e *cbor.UnacceptableDataItemError) { + if diff := cmp.Diff(&cbor.UnacceptableDataItemError{CBORType: "primitives", Message: "floating-point NaN"}, e); diff != "" { + t.Errorf("unexpected error diff:\n%s", diff) } - }, - fixme: "NaN and positive/negative infinities should be rejected", + }), }, { name: "smallest nonzero float64", @@ -728,25 +719,31 @@ func TestDecode(t *testing.T) { assertOnError: assertNilError, }, { - name: "tag 1 with a positive infinity", - in: hex("c1f97c00"), // 1(Infinity) - want: "0001-01-01T00:00:00Z", - fixme: "decoding cbor data tagged with 1 produces time.Time instead of RFC3339 timestamp string", - assertOnError: assertNilError, + name: "tag 1 with a positive infinity", + in: hex("c1f97c00"), // 1(Infinity) + assertOnError: assertOnConcreteError(func(t *testing.T, e *cbor.UnacceptableDataItemError) { + if diff := cmp.Diff(&cbor.UnacceptableDataItemError{CBORType: "primitives", Message: "floating-point infinity"}, e); diff != "" { + t.Errorf("unexpected error diff:\n%s", diff) + } + }), }, { - name: "tag 1 with a negative infinity", - in: hex("c1f9fc00"), // 1(-Infinity) - want: "0001-01-01T00:00:00Z", - fixme: "decoding cbor data tagged with 1 produces time.Time instead of RFC3339 timestamp string", - assertOnError: assertNilError, + name: "tag 1 with a negative infinity", + in: hex("c1f9fc00"), // 1(-Infinity) + assertOnError: assertOnConcreteError(func(t *testing.T, e *cbor.UnacceptableDataItemError) { + if diff := cmp.Diff(&cbor.UnacceptableDataItemError{CBORType: "primitives", Message: "floating-point infinity"}, e); diff != "" { + t.Errorf("unexpected error diff:\n%s", diff) + } + }), }, { - name: "tag 1 with NaN", - in: hex("c1f9fc00"), // 1(NaN) - want: "0001-01-01T00:00:00Z", - fixme: "decoding cbor data tagged with 1 produces time.Time instead of RFC3339 timestamp string", - assertOnError: assertNilError, + name: "tag 1 with NaN", + in: hex("c1f97e00"), // 1(NaN) + assertOnError: assertOnConcreteError(func(t *testing.T, e *cbor.UnacceptableDataItemError) { + if diff := cmp.Diff(&cbor.UnacceptableDataItemError{CBORType: "primitives", Message: "floating-point NaN"}, 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 95a9b46b716..16a05c5e57d 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 @@ -32,12 +32,10 @@ var Encode cbor.EncMode = func() cbor.EncMode { // encoding. Satisfies one of the "Core Deterministic Encoding Requirements". ShortestFloat: cbor.ShortestFloat16, - // ShortestFloat doesn't apply to NaN or Inf values. Inf values are losslessly - // encoded to float16. RFC 8949 recommends choosing a single representation of NaN - // in applications that do not smuggle additional information inside NaN values, we - // use 0x7e00. - NaNConvert: cbor.NaNConvert7e00, - InfConvert: cbor.InfConvertFloat16, + // Error on attempt to encode NaN and infinite values. This is what the JSON + // serializer does. + 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