Repository: fxamacker/cbor (go)

Issue:
feature: Add decoding option to be more strict than encoding/json: fail on CBOR map if destination struct field is not found
Currently any unknown fields are skipped during decoding, would it be possible to have an option to fail instead?

Pull request description:
Add option to disallow unknown field during decoding
New decoding option ExtraReturnErrors specifies extra conditions that should be treated as errors.  It's a bit set of ExtraDecErrorCond values.

ExtraDecErrorCond values:

* ExtraDecErrorNone: (default) no extra error condition.

* ExtraDecErrorUnknownField: error condition when destination Go struct doesn't have a field matching CBOR map key.  It only affects decoding CBOR maps to Go struct values.

Closes #178



Patch:
diff --git a/decode.go b/decode.go
index f418d07..21fb65c 100644
--- a/decode.go
+++ b/decode.go
@@ -153,6 +153,15 @@ func (e *DupMapKeyError) Error() string {
 	return fmt.Sprintf("cbor: found duplicate map key \"%v\" at map element index %d", e.Key, e.Index)
 }
 
+// UnknownFieldError describes detected unknown field in CBOR map when decoding to Go struct.
+type UnknownFieldError struct {
+	Index int
+}
+
+func (e *UnknownFieldError) Error() string {
+	return fmt.Sprintf("cbor: found unknown field at map element index %d", e.Index)
+}
+
 // DupMapKeyMode specifies how to enforce duplicate map key.
 type DupMapKeyMode int
 
@@ -232,6 +241,20 @@ func (idm IntDecMode) valid() bool {
 	return idm < maxIntDec
 }
 
+// ExtraDecErrorCond specifies extra conditions that should be treated as errors.
+type ExtraDecErrorCond int
+
+const (
+	// ExtraDecErrorNone indicates no extra error condition.
+	ExtraDecErrorNone ExtraDecErrorCond = 0
+
+	// ExtraDecErrorUnknownField indicates error condition when destination
+	// Go struct doesn't have a field matching a CBOR map key.
+	ExtraDecErrorUnknownField = 1 << 0
+
+	maxExtraDecError int = 1 << 1
+)
+
 // DecOptions specifies decoding options.
 type DecOptions struct {
 	// DupMapKey specifies whether to enforce duplicate map key.
@@ -262,6 +285,10 @@ type DecOptions struct {
 	// IntDec specifies which Go integer type (int64 or uint64) to use
 	// when decoding CBOR int (major type 0 and 1) to Go interface{}.
 	IntDec IntDecMode
+
+	// ExtraReturnErrors specifies extra conditions that should be treated as errors.
+	// ExtraReturnErrors is a bit set of ExtraDecErrorCond values.
+	ExtraReturnErrors int
 }
 
 // DecMode returns DecMode with immutable options and no tags (safe for concurrency).
@@ -358,15 +385,19 @@ func (opts DecOptions) decMode() (*decMode, error) {
 	} else if opts.MaxMapPairs < minMaxMapPairs || opts.MaxMapPairs > maxMaxMapPairs {
 		return nil, errors.New("cbor: invalid MaxMapPairs " + strconv.Itoa(opts.MaxMapPairs) + " (range is [" + strconv.Itoa(minMaxMapPairs) + ", " + strconv.Itoa(maxMaxMapPairs) + "])")
 	}
+	if opts.ExtraReturnErrors >= maxExtraDecError {
+		return nil, errors.New("cbor: invalid ExtraReturnErrors " + strconv.Itoa(opts.ExtraReturnErrors))
+	}
 	dm := decMode{
-		dupMapKey:        opts.DupMapKey,
-		timeTag:          opts.TimeTag,
-		maxNestedLevels:  opts.MaxNestedLevels,
-		maxArrayElements: opts.MaxArrayElements,
-		maxMapPairs:      opts.MaxMapPairs,
-		indefLength:      opts.IndefLength,
-		tagsMd:           opts.TagsMd,
-		intDec:           opts.IntDec,
+		dupMapKey:         opts.DupMapKey,
+		timeTag:           opts.TimeTag,
+		maxNestedLevels:   opts.MaxNestedLevels,
+		maxArrayElements:  opts.MaxArrayElements,
+		maxMapPairs:       opts.MaxMapPairs,
+		indefLength:       opts.IndefLength,
+		tagsMd:            opts.TagsMd,
+		intDec:            opts.IntDec,
+		extraReturnErrors: opts.ExtraReturnErrors,
 	}
 	return &dm, nil
 }
@@ -379,15 +410,16 @@ type DecMode interface {
 }
 
 type decMode struct {
-	tags             tagProvider
-	dupMapKey        DupMapKeyMode
-	timeTag          DecTagMode
-	maxNestedLevels  int
-	maxArrayElements int
-	maxMapPairs      int
-	indefLength      IndefLengthMode
-	tagsMd           TagsMode
-	intDec           IntDecMode
+	tags              tagProvider
+	dupMapKey         DupMapKeyMode
+	timeTag           DecTagMode
+	maxNestedLevels   int
+	maxArrayElements  int
+	maxMapPairs       int
+	indefLength       IndefLengthMode
+	tagsMd            TagsMode
+	intDec            IntDecMode
+	extraReturnErrors int
 }
 
 var defaultDecMode, _ = DecOptions{}.decMode()
@@ -395,14 +427,15 @@ var defaultDecMode, _ = DecOptions{}.decMode()
 // DecOptions returns user specified options used to create this DecMode.
 func (dm *decMode) DecOptions() DecOptions {
 	return DecOptions{
-		DupMapKey:        dm.dupMapKey,
-		TimeTag:          dm.timeTag,
-		MaxNestedLevels:  dm.maxNestedLevels,
-		MaxArrayElements: dm.maxArrayElements,
-		MaxMapPairs:      dm.maxMapPairs,
-		IndefLength:      dm.indefLength,
-		TagsMd:           dm.tagsMd,
-		IntDec:           dm.intDec,
+		DupMapKey:         dm.dupMapKey,
+		TimeTag:           dm.timeTag,
+		MaxNestedLevels:   dm.maxNestedLevels,
+		MaxArrayElements:  dm.maxArrayElements,
+		MaxMapPairs:       dm.maxMapPairs,
+		IndefLength:       dm.indefLength,
+		TagsMd:            dm.tagsMd,
+		IntDec:            dm.intDec,
+		ExtraReturnErrors: dm.extraReturnErrors,
 	}
 }
 
@@ -1192,16 +1225,25 @@ func (d *decodeState) parseMapToStruct(v reflect.Value, tInfo *typeInfo) error {
 		}
 	}
 
-	foundFldIdx := make([]bool, len(structType.fields))
+	var err, lastErr error
+
+	// Get CBOR map size
 	_, ai, val := d.getHead()
 	hasSize := (ai != 31)
 	count := int(val)
-	var err, lastErr error
+
+	// Keeps track of matched struct fields
+	foundFldIdx := make([]bool, len(structType.fields))
+
+	// Keeps track of CBOR map keys to detect duplicate map key
 	keyCount := 0
-	var mapKeys map[interface{}]struct{} // Store map keys, used for detecting duplicate map key.
+	var mapKeys map[interface{}]struct{}
 	if d.dm.dupMapKey == DupMapKeyEnforcedAPF {
 		mapKeys = make(map[interface{}]struct{}, len(structType.fields))
 	}
+
+	errOnUnknownField := (d.dm.extraReturnErrors & ExtraDecErrorUnknownField) > 0
+
 	for j := 0; (hasSize && j < count) || (!hasSize && !d.foundBreak()); j++ {
 		var f *field
 		var k interface{} // Used by duplicate map key detection
@@ -1326,6 +1368,17 @@ func (d *decodeState) parseMapToStruct(v reflect.Value, tInfo *typeInfo) error {
 		}
 
 		if f == nil {
+			if errOnUnknownField {
+				err = &UnknownFieldError{j}
+				d.skip() // Skip value
+				j++
+				// skip the rest of the map
+				for ; (hasSize && j < count) || (!hasSize && !d.foundBreak()); j++ {
+					d.skip()
+					d.skip()
+				}
+				return err
+			}
 			d.skip() // Skip value
 			continue
 		}


Test patch:
diff --git a/decode_test.go b/decode_test.go
index bb83da9..a4b6603 100644
--- a/decode_test.go
+++ b/decode_test.go
@@ -2938,12 +2938,15 @@ func TestUnmarshalToNotNilInterface(t *testing.T) {
 
 func TestDecOptions(t *testing.T) {
 	opts1 := DecOptions{
-		TimeTag:          DecTagRequired,
-		DupMapKey:        DupMapKeyEnforcedAPF,
-		IndefLength:      IndefLengthForbidden,
-		MaxNestedLevels:  100,
-		MaxMapPairs:      101,
-		MaxArrayElements: 102,
+		TimeTag:           DecTagRequired,
+		DupMapKey:         DupMapKeyEnforcedAPF,
+		IndefLength:       IndefLengthForbidden,
+		MaxNestedLevels:   100,
+		MaxMapPairs:       101,
+		MaxArrayElements:  102,
+		TagsMd:            TagsForbidden,
+		IntDec:            IntDecConvertSigned,
+		ExtraReturnErrors: ExtraDecErrorUnknownField,
 	}
 	dm, err := opts1.DecMode()
 	if err != nil {
@@ -4352,3 +4355,119 @@ func TestIntDec(t *testing.T) {
 		})
 	}
 }
+
+func TestDecModeInvalidExtraError(t *testing.T) {
+	wantErrorMsg := "cbor: invalid ExtraReturnErrors 3"
+	_, err := DecOptions{ExtraReturnErrors: 3}.DecMode()
+	if err == nil {
+		t.Errorf("DecMode() didn't return an error")
+	} else if err.Error() != wantErrorMsg {
+		t.Errorf("DecMode() returned error %q, want %q", err.Error(), wantErrorMsg)
+	}
+}
+
+func TestExtraErrorCondUnknowField(t *testing.T) {
+	type s struct {
+		A string
+		B string
+		C string
+	}
+
+	dm, _ := DecOptions{}.DecMode()
+	dmUnknownFieldError, _ := DecOptions{ExtraReturnErrors: ExtraDecErrorUnknownField}.DecMode()
+
+	testCases := []struct {
+		name         string
+		cborData     []byte
+		dm           DecMode
+		wantObj      interface{}
+		wantErrorMsg string
+	}{
+		{
+			name:     "field by field match",
+			cborData: hexDecode("a3614161616142616261436163"), // map[string]string{"A": "a", "B": "b", "C": "c"}
+			dm:       dm,
+			wantObj:  s{A: "a", B: "b", C: "c"},
+		},
+		{
+			name:     "field by field match with ExtraDecErrorUnknownField",
+			cborData: hexDecode("a3614161616142616261436163"), // map[string]string{"A": "a", "B": "b", "C": "c"}
+			dm:       dmUnknownFieldError,
+			wantObj:  s{A: "a", B: "b", C: "c"},
+		},
+		{
+			name:     "CBOR map less field",
+			cborData: hexDecode("a26141616161426162"), // map[string]string{"A": "a", "B": "b"}
+			dm:       dm,
+			wantObj:  s{A: "a", B: "b", C: ""},
+		},
+		{
+			name:     "CBOR map less field with ExtraDecErrorUnknownField",
+			cborData: hexDecode("a26141616161426162"), // map[string]string{"A": "a", "B": "b"}
+			dm:       dmUnknownFieldError,
+			wantObj:  s{A: "a", B: "b", C: ""},
+		},
+		{
+			name:     "CBOR map unknown field",
+			cborData: hexDecode("a461416161614261626143616361446164"), // map[string]string{"A": "a", "B": "b", "C": "c", "D": "d"}
+			dm:       dm,
+			wantObj:  s{A: "a", B: "b", C: "c"},
+		},
+		{
+			name:         "CBOR map unknown field with ExtraDecErrorUnknownField",
+			cborData:     hexDecode("a461416161614261626143616361446164"), // map[string]string{"A": "a", "B": "b", "C": "c", "D": "d"}
+			dm:           dmUnknownFieldError,
+			wantErrorMsg: "cbor: found unknown field at map element index 3",
+		},
+	}
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			var v s
+			err := tc.dm.Unmarshal(tc.cborData, &v)
+			if err == nil {
+				if tc.wantErrorMsg != "" {
+					t.Errorf("Unmarshal(0x%x) didn't return an error, want %q", tc.cborData, tc.wantErrorMsg)
+				} else if !reflect.DeepEqual(v, tc.wantObj) {
+					t.Errorf("Unmarshal(0x%x) return %v (%T), want %v (%T)", tc.cborData, v, v, tc.wantObj, tc.wantObj)
+				}
+			} else {
+				if tc.wantErrorMsg == "" {
+					t.Errorf("Unmarshal(0x%x) returned error %q", tc.cborData, err)
+				} else if !strings.Contains(err.Error(), tc.wantErrorMsg) {
+					t.Errorf("Unmarshal(0x%x) returned error %q, want %q", tc.cborData, err.Error(), tc.wantErrorMsg)
+				}
+			}
+		})
+	}
+}
+
+func TestStreamExtraErrorCondUnknowField(t *testing.T) {
+	type s struct {
+		A string
+		B string
+		C string
+	}
+
+	cborData := hexDecode("a461416161614461646142616261436163a3614161616142616261436163") // map[string]string{"A": "a", "D": "d", "B": "b", "C": "c"}, map[string]string{"A": "a", "B": "b", "C": "c"}
+	wantErrorMsg := "cbor: found unknown field at map element index 1"
+	wantObj := s{A: "a", B: "b", C: "c"}
+
+	dmUnknownFieldError, _ := DecOptions{ExtraReturnErrors: ExtraDecErrorUnknownField}.DecMode()
+	dec := dmUnknownFieldError.NewDecoder(bytes.NewReader(cborData))
+
+	var v1 s
+	err := dec.Decode(&v1)
+	if err == nil {
+		t.Errorf("Decode() didn't return an error, want %q", wantErrorMsg)
+	} else if !strings.Contains(err.Error(), wantErrorMsg) {
+		t.Errorf("Decode() returned error %q, want %q", err.Error(), wantErrorMsg)
+	}
+
+	var v2 s
+	err = dec.Decode(&v2)
+	if err != nil {
+		t.Errorf("Decode() returned an error %v", err)
+	} else if !reflect.DeepEqual(v2, wantObj) {
+		t.Errorf("Decode() return %v (%T), want %v (%T)", v2, v2, wantObj, wantObj)
+	}
+}