Skip to content

Commit

Permalink
patch: add DisallowDuplicateFields decoder option
Browse files Browse the repository at this point in the history 
This adds a DisallowDuplicateFields decoder option (defaulting to false).

Duplicate JSON object keys encountered produce strict errors.
- When decoding to a struct, duplicate keys are keys that map to the same struct field.
- When decoding to a map, duplicate keys are identical strings (case-sensitive).

Duplicate detection is implemented as inlined closures to minimize heap allocs.
  • Loading branch information
@liggitt
liggitt committed Oct 20, 2021
1 parent d055cdf commit bef9430
Show file tree
Hide file tree
Showing 3 changed files with 262 additions and 0 deletions.
61 changes: 61 additions & 0 deletions internal/golang/encoding/json/decode.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,8 @@ type decodeState struct {
caseSensitive bool

preserveInts bool

disallowDuplicateFields bool
}

// readIndex returns the position of the last byte read.
Expand Down Expand Up @@ -649,6 +651,7 @@ func (d *decodeState) object(v reflect.Value) error {
}

var fields structFields
var checkDuplicateField func(fieldNameIndex int, fieldName string)

// Check type of target:
// struct or
Expand All @@ -672,8 +675,51 @@ func (d *decodeState) object(v reflect.Value) error {
if v.IsNil() {
v.Set(reflect.MakeMap(t))
}

if d.disallowDuplicateFields {
var seenKeys map[string]struct{}
checkDuplicateField = func(fieldNameIndex int, fieldName string) {
if seenKeys == nil {
seenKeys = map[string]struct{}{}
}
if _, seen := seenKeys[fieldName]; seen {
d.saveStrictError(fmt.Errorf("duplicate field %q", fieldName))
} else {
seenKeys[fieldName] = struct{}{}
}
}
}

case reflect.Struct:
fields = cachedTypeFields(t)

if d.disallowDuplicateFields {
if len(fields.list) <= 64 {
// bitset by field index for structs with <= 64 fields
var seenKeys uint64
checkDuplicateField = func(fieldNameIndex int, fieldName string) {
if seenKeys&(1<<fieldNameIndex) != 0 {
d.saveStrictError(fmt.Errorf("duplicate field %q", fieldName))
} else {
seenKeys = seenKeys | (1 << fieldNameIndex)
}
}
} else {
// list of seen field indices for structs with greater than 64 fields
var seenIndexes []bool
checkDuplicateField = func(fieldNameIndex int, fieldName string) {
if seenIndexes == nil {
seenIndexes = make([]bool, len(fields.list))
}
if seenIndexes[fieldNameIndex] {
d.saveStrictError(fmt.Errorf("duplicate field %q", fieldName))
} else {
seenIndexes[fieldNameIndex] = true
}
}
}
}

// ok
default:
d.saveError(&UnmarshalTypeError{Value: "object", Type: t, Offset: int64(d.off)})
Expand Down Expand Up @@ -719,18 +765,27 @@ func (d *decodeState) object(v reflect.Value) error {
mapElem.Set(reflect.Zero(elemType))
}
subv = mapElem
if checkDuplicateField != nil {
checkDuplicateField(0, string(key))
}
} else {
var f *field
if i, ok := fields.nameIndex[string(key)]; ok {
// Found an exact name match.
f = &fields.list[i]
if checkDuplicateField != nil {
checkDuplicateField(i, f.name)
}
} else if !d.caseSensitive {
// Fall back to the expensive case-insensitive
// linear search.
for i := range fields.list {
ff := &fields.list[i]
if ff.equalFold(ff.nameBytes, key) {
f = ff
if checkDuplicateField != nil {
checkDuplicateField(i, f.name)
}
break
}
}
Expand Down Expand Up @@ -1142,6 +1197,12 @@ func (d *decodeState) objectInterface() map[string]interface{} {
}
d.scanWhile(scanSkipSpace)

if d.disallowDuplicateFields {
if _, exists := m[key]; exists {
d.saveStrictError(fmt.Errorf("duplicate field %q", key))
}
}

// Read value.
m[key] = d.valueInterface()

Expand Down
8 changes: 8 additions & 0 deletions internal/golang/encoding/json/kubernetes_patch.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,14 @@ func (d *Decoder) PreserveInts() {
d.d.preserveInts = true
}

// DisallowDuplicateFields treats duplicate fields encountered while decoding as an error.
func DisallowDuplicateFields(d *decodeState) {
d.disallowDuplicateFields = true
}
func (d *Decoder) DisallowDuplicateFields() {
d.d.disallowDuplicateFields = true
}

// saveStrictError saves a strict decoding error,
// for reporting at the end of the unmarshal if no other errors occurred.
func (d *decodeState) saveStrictError(err error) {
Expand Down
193 changes: 193 additions & 0 deletions internal/golang/encoding/json/kubernetes_patch_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -567,3 +567,196 @@ func TestPreserveInts(t *testing.T) {
t.Fatalf("expected\n\t%#v, got\n\t%#v", e, a)
}
}

func TestDisallowDuplicateFields(t *testing.T) {
type MixedObj struct {
A int `json:"a"`
B int `json:"b"`
C int
D map[string]string
}
type SmallObj struct {
F01 int
F02 int
F03 int
F04 int
F05 int
F06 int
F07 int
F08 int
F09 int
F10 int
F11 int
F12 int
F13 int
F14 int
F15 int
F16 int
F17 int
F18 int
F19 int
F20 int
F21 int
F22 int
F23 int
F24 int
F25 int
F26 int
F27 int
F28 int
F29 int
F30 int
F31 int
F32 int
F33 int
F34 int
F35 int
F36 int
F37 int
F38 int
F39 int
F40 int
F41 int
F42 int
F43 int
F44 int
F45 int
F46 int
F47 int
F48 int
F49 int
F50 int
F51 int
F52 int
F53 int
F54 int
F55 int
F56 int
F57 int
F58 int
F59 int
F60 int
F61 int
F62 int
F63 int
F64 int
}

type BigObj struct {
F01 int
F02 int
F03 int
F04 int
F05 int
F06 int
F07 int
F08 int
F09 int
F10 int
F11 int
F12 int
F13 int
F14 int
F15 int
F16 int
F17 int
F18 int
F19 int
F20 int
F21 int
F22 int
F23 int
F24 int
F25 int
F26 int
F27 int
F28 int
F29 int
F30 int
F31 int
F32 int
F33 int
F34 int
F35 int
F36 int
F37 int
F38 int
F39 int
F40 int
F41 int
F42 int
F43 int
F44 int
F45 int
F46 int
F47 int
F48 int
F49 int
F50 int
F51 int
F52 int
F53 int
F54 int
F55 int
F56 int
F57 int
F58 int
F59 int
F60 int
F61 int
F62 int
F63 int
F64 int
F65 int
}

testcases := []struct {
name string
in string
to interface{}
expectErr string
}{
{
name: "duplicate typed",
in: `{"a":1,"a":2,"a":3,"b":4,"b":5,"b":6,"C":7,"C":8,"C":9}`,
to: &MixedObj{},
expectErr: `json: duplicate field "a", duplicate field "b", duplicate field "C"`,
},
{
name: "duplicate typed map field",
in: `{"d":{"a":"","b":"","c":"","a":"","b":"","c":""}}`,
to: &MixedObj{},
expectErr: `json: duplicate field "a", duplicate field "b", duplicate field "c"`,
},
{
name: "duplicate untyped map",
in: `{"a":"","b":"","a":"","b":"","c":{"c":"","c":""}}`,
to: &map[string]interface{}{},
expectErr: `json: duplicate field "a", duplicate field "b", duplicate field "c"`,
},
{
name: "small obj",
in: `{"f01":1,"f01":2,"f32":1,"f32":2,"f64":1,"f64":2}`,
to: &SmallObj{},
expectErr: `json: duplicate field "F01", duplicate field "F32", duplicate field "F64"`,
},
{
name: "big obj",
in: `{"f01":1,"f01":2,"f32":1,"f32":2,"f64":1,"f64":2,"f65":1,"f65":2}`,
to: &BigObj{},
expectErr: `json: duplicate field "F01", duplicate field "F32", duplicate field "F64", duplicate field "F65"`,
},
}

for _, tc := range testcases {
t.Run(tc.name, func(t *testing.T) {
err := Unmarshal([]byte(tc.in), &tc.to, DisallowDuplicateFields)
if (len(tc.expectErr) > 0) != (err != nil) {
t.Fatalf("expected err=%v, got %v", len(tc.expectErr) > 0, err)
}
if len(tc.expectErr) > 0 && !strings.Contains(err.Error(), tc.expectErr) {
t.Fatalf("expected err containing '%s', got %v", tc.expectErr, err)
}
})
}
}

0 comments on commit bef9430

Please sign in to comment.