diff --git a/orderedmap.go b/orderedmap.go index 61587d9..2e27fbb 100644 --- a/orderedmap.go +++ b/orderedmap.go @@ -3,6 +3,7 @@ package orderedmap import ( "bytes" "encoding/json" + "fmt" "sort" ) @@ -29,9 +30,10 @@ func (a ByPair) Swap(i, j int) { a.Pairs[i], a.Pairs[j] = a.Pairs[j], a.Pai func (a ByPair) Less(i, j int) bool { return a.LessFunc(a.Pairs[i], a.Pairs[j]) } type OrderedMap struct { - keys []string - values map[string]interface{} - escapeHTML bool + keys []string + values map[string]interface{} + escapeHTML bool + jsonLastValueWins bool // Permits duplicates on JSON unmarshal. Otherwise errors on duplicates on unmarshal. } func New() *OrderedMap { @@ -46,6 +48,10 @@ func (o *OrderedMap) SetEscapeHTML(on bool) { o.escapeHTML = on } +func (o *OrderedMap) SetJSONLastValueWins(on bool) { + o.jsonLastValueWins = on +} + func (o *OrderedMap) Get(key string) (interface{}, bool) { val, exists := o.values[key] return val, exists @@ -104,10 +110,17 @@ func (o *OrderedMap) Sort(lessFunc func(a *Pair, b *Pair) bool) { } func (o *OrderedMap) UnmarshalJSON(b []byte) error { + var err error + if !o.jsonLastValueWins { + err = CheckDuplicate(json.NewDecoder(bytes.NewReader(b))) + if err != nil { + return err + } + } if o.values == nil { o.values = map[string]interface{}{} } - err := json.Unmarshal(b, &o.values) + err = json.Unmarshal(b, &o.values) if err != nil { return err } @@ -264,3 +277,84 @@ func (o OrderedMap) MarshalJSON() ([]byte, error) { buf.WriteByte('}') return buf.Bytes(), nil } + +// ErrJSONDuplicate allows applications to check for JSON duplicate error. +type ErrJSONDuplicate error + +// CheckDuplicate checks for JSON duplicates on ingest (unmarshal). Note that +// Go maps and structs and Javascript objects (ES6) already require unique +// JSON names. +// +// Duplicate JSON fields are a security issue that wasn't addressed by the +// original spec that results in surprising behavior and is a source of bugs. +// See the article, "[An Exploration of JSON Interoperability +// Vulnerabilities](https://bishopfox.com/blog/json-interoperability-vulnerabilities)" +// and control-f "duplicate". +// +// Until Go releases the planned revision to the JSON package (See +// https://github.com/go-json-experiment/json), or adds support for erroring on +// duplicates to the current package, this function is needed. +// +// After JSON was widely adopted, Douglas Crockford (JSON's inventor), tried to +// fix this by updating JSON to define "must error on duplicates" as the correct +// behavior, but it was decided it was too late +// (https://esdiscuss.org/topic/json-duplicate-keys). +// +// Although Douglas Crockford couldn't change the JSON spec to force +// implementations to error on duplicate, his Java JSON implementation errors on +// duplicates. Others implementations behaviors are `last-value-wins`, support +// duplicate keys, or other non-standard behavior. The [JSON +// RFC](https://datatracker.ietf.org/doc/html/rfc8259#section-4) states that +// implementations should not allow duplicate keys. It then notes the varying behavior +// of existing implementations. +// +// Disallowing duplicates conforms to the small I-JSON RFC. The author of +// I-JSON, Tim Bray, is also the author of current JSON specification (RFC +// 8259). See also https://github.com/json5/json5-spec/issues/38. +func CheckDuplicate(d *json.Decoder) error { + t, err := d.Token() + if err != nil { + return err + } + + delim, ok := t.(json.Delim) // Is it a delimiter? + if !ok { + return nil // scaler type, nothing to do + } + + switch delim { + case '{': + keys := make(map[string]bool) + for d.More() { + t, err := d.Token() // Get field key. + if err != nil { + return err + } + + key := t.(string) + if keys[key] { // Check for duplicates. + return ErrJSONDuplicate(fmt.Errorf("Coze: JSON duplicate field %q", key)) + } + keys[key] = true + err = CheckDuplicate(d) // Recursive, Check value in case value is object. + if err != nil { + return err + } + } + if _, err := d.Token(); err != nil { // consume trailing } + return err + } + + case '[': + for d.More() { + if err := CheckDuplicate(d); err != nil { + return err + } + } + // consume trailing ] + if _, err := d.Token(); err != nil { + return err + } + } + return nil +} diff --git a/orderedmap_test.go b/orderedmap_test.go index 5b89ef6..9a49893 100644 --- a/orderedmap_test.go +++ b/orderedmap_test.go @@ -69,10 +69,10 @@ func TestOrderedMap(t *testing.T) { // Values method values := o.Values() expectedValues := map[string]interface{}{ - "number": 4, - "string": "x", + "number": 4, + "string": "x", "strings": []string{"t", "u"}, - "mixed": []interface{}{ 1, "1" }, + "mixed": []interface{}{1, "1"}, } if !reflect.DeepEqual(values, expectedValues) { t.Error("Values method returned unexpected map") @@ -350,6 +350,7 @@ func TestUnmarshalJSONDuplicateKeys(t *testing.T) { "b": [[1]] }` o := New() + o.SetJSONLastValueWins(true) err := json.Unmarshal([]byte(s), &o) if err != nil { t.Error("JSON Unmarshal error with special chars", err) @@ -404,6 +405,23 @@ func TestUnmarshalJSONDuplicateKeys(t *testing.T) { } } +// TestOrderedMapUnmarshalJSONDuplicate tests that orderedMap errors on +// duplicate JSON fields. +func TestOrderedMapUnmarshalJSONDuplicate(t *testing.T) { + s := `{ + "a": [{}, []], + "b": {"x":[1]}, + "c": "x", + "d": {"x":1}, + "b": [{"x":[]}] + }` + o := New() + err := json.Unmarshal([]byte(s), &o) + if err == nil { + t.Errorf("orderedMap unmarshal did not error on duplicate key") + } +} + func TestUnmarshalJSONSpecialChars(t *testing.T) { s := `{ " \u0041\n\r\t\\\\\\\\\\\\ " : { "\\\\\\" : "\\\\\"\\" }, "\\": " \\\\ test ", "\n": "\r" }` o := New()