Merge pull request kubernetes#125421 from benluddy/cbor-simple-values

KEP-4222: Reject CBOR simple values other than true, false, and null.

Author: k8s-ci-robot

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

@@ -249,17 +249,14 @@ func TestAppendixA(t *testing.T) {
 		{
 			example: hex("f7"),
 			reject:  "only simple values false, true, and null have a clear analog",
-			fixme:   "the undefined simple value should not successfully decode as nil",
 		},
 		{
 			example: hex("f0"),
 			reject:  "only simple values false, true, and null have a clear analog",
-			fixme:   "simple values other than false, true, and null should be rejected",
 		},
 		{
 			example: hex("f8ff"),
 			reject:  "only simple values false, true, and null have a clear analog",
-			fixme:   "simple values other than false, true, and null should be rejected",
 		},
 		{
 			example: hex("c074323031332d30332d32315432303a30343a30305a"),

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

@@ -22,6 +22,28 @@ import (
 	"github.com/fxamacker/cbor/v2"
 )
 
+var simpleValues *cbor.SimpleValueRegistry = func() *cbor.SimpleValueRegistry {
+	var opts []func(*cbor.SimpleValueRegistry) error
+	for sv := 0; sv <= 255; sv++ {
+		// Reject simple values 0-19, 23, and 32-255. The simple values 24-31 are reserved
+		// and considered ill-formed by the CBOR specification. We only accept false (20),
+		// true (21), and null (22).
+		switch sv {
+		case 20: // false
+		case 21: // true
+		case 22: // null
+		case 24, 25, 26, 27, 28, 29, 30, 31: // reserved
+		default:
+			opts = append(opts, cbor.WithRejectedSimpleValue(cbor.SimpleValue(sv)))
+		}
+	}
+	simpleValues, err := cbor.NewSimpleValueRegistryFromDefaults(opts...)
+	if err != nil {
+		panic(err)
+	}
+	return simpleValues
+}()
+
 var Decode cbor.DecMode = func() cbor.DecMode {
 	decode, err := cbor.DecOptions{
 		// Maps with duplicate keys are well-formed but invalid according to the CBOR spec
@@ -100,6 +122,9 @@ var Decode cbor.DecMode = func() cbor.DecMode {
 		// Reject the arbitrary-precision integer tags because they can't be faithfully
 		// roundtripped through the allowable Unstructured types.
 		BignumTag: cbor.BignumTagForbidden,
+
+		// Reject anything other than the simple values true, false, and null.
+		SimpleValues: simpleValues,
 	}.DecMode()
 	if err != nil {
 		panic(err)

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

@@ -585,13 +585,11 @@ func TestDecode(t *testing.T) {
 		{
 			name: "simple value 23",
 			in:   hex("f7"), // undefined
-			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")
+			assertOnError: assertOnConcreteError(func(t *testing.T, e *cbor.UnacceptableDataItemError) {
+				if diff := cmp.Diff(&cbor.UnacceptableDataItemError{CBORType: "primitives", Message: "simple value 23 is not recognized"}, e); diff != "" {
+					t.Errorf("unexpected error diff:\n%s", diff)
 				}
-			},
-			fixme: "cbor simple value 23 (\"undefined\") should not be accepted",
+			}),
 		},
 	}, func() (generated []test) {
 		// Generate test cases for all simple values (0 to 255) because the number of possible simple values is fixed and small.
@@ -619,13 +617,11 @@ func TestDecode(t *testing.T) {
 				})
 			default:
 				// reject all unrecognized simple values
-				each.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")
+				each.assertOnError = assertOnConcreteError(func(t *testing.T, e *cbor.UnacceptableDataItemError) {
+					if diff := cmp.Diff(&cbor.UnacceptableDataItemError{CBORType: "primitives", Message: fmt.Sprintf("simple value %d is not recognized", i)}, e); diff != "" {
+						t.Errorf("unexpected error diff:\n%s", diff)
 					}
-				}
-				each.fixme = "unrecognized simple values should be rejected"
+				})
 			}
 			generated = append(generated, each)
 		}