v2.1.4

* Fixed a bug with UTF-8 special characters in JSON keys
* Made fuzz tests easier to understand/debug in case of failures

Signed-off-by: Jean Rouge <[email protected]>

Author: wk8

.circleci/circle_build.sh

@@ -2,9 +2,12 @@
 
 set -ex
 
+# might as well run a little longer
+export FUZZ_TIME=25s
+
 # there are too many golangci plugins that don't work for 1.19 just yet, so just skip linting for it
 if [[ "$GO_VER" == 1.18.* ]]; then
   make
 else
-  make test fuzz
+  make test_with_fuzz
 fi

CHANGELOG.md

@@ -2,6 +2,10 @@
 
 [comment]: # (Changes since last release go here)
 
+# 2.1.4 - Dec 12th 2022
+
+* Fixed a bug with UTF-8 special characters in JSON keys
+
 # 2.1.3 - Dec 11th 2022
 
 * Added support for JSON marshalling/unmarshalling of wrapper of primitive types

Makefile

@@ -1,20 +1,27 @@
 .DEFAULT_GOAL := all
 
 .PHONY: all
-all: test lint fuzz
+all: test_with_fuzz lint
 
 # the TEST_FLAGS env var can be set to eg run only specific tests
+TEST_COMMAND = go test -v -count=1 -race -cover $(TEST_FLAGS)
+
 .PHONY: test
 test:
-	go test -v -count=1 -race -cover $(TEST_FLAGS)
+	$(TEST_COMMAND)
 
 .PHONY: bench
 bench:
 	go test -bench=.
 
+FUZZ_TIME ?= 10s
+
+.PHONY: test_with_fuzz
+test_with_fuzz:
+	$(TEST_COMMAND) -fuzz=. -fuzztime=$(FUZZ_TIME)
+
 .PHONY: fuzz
-fuzz:
-	go test -fuzz=. -fuzztime=10s ./...
+fuzz: test_with_fuzz
 
 .PHONY: lint
 lint:

fuzz_test.go

@@ -6,11 +6,11 @@ require (
 	github.com/bahlo/generic-list-go v0.2.0
 	github.com/buger/jsonparser v1.1.1
 	github.com/mailru/easyjson v0.7.7
-	github.com/stretchr/testify v1.6.1
+	github.com/stretchr/testify v1.8.1
 )
 
 require (
-	github.com/davecgh/go-spew v1.1.0 // indirect
+	github.com/davecgh/go-spew v1.1.1 // indirect
 	github.com/pmezard/go-difflib v1.0.0 // indirect
-	gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c // indirect
+	gopkg.in/yaml.v3 v3.0.1 // indirect
 )

go.mod

@@ -2,17 +2,23 @@ github.com/bahlo/generic-list-go v0.2.0 h1:5sz/EEAK+ls5wF+NeqDpk5+iNdMDXrh3z3nPn
 github.com/bahlo/generic-list-go v0.2.0/go.mod h1:2KvAjgMlE5NNynlg/5iLrrCCZ2+5xWbdbCW3pNTGyYg=
 github.com/buger/jsonparser v1.1.1 h1:2PnMjfWD7wBILjqQbt530v576A/cAbQvEW9gGIpYMUs=
 github.com/buger/jsonparser v1.1.1/go.mod h1:6RYKKt7H4d4+iWqouImQ9R2FZql3VbhNgx27UK13J/0=
-github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8=
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
+github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
+github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/josharian/intern v1.0.0/go.mod h1:5DoeVV0s6jJacbCEi61lwdGj/aVlrQvzHFFd8Hwg//Y=
 github.com/mailru/easyjson v0.7.7 h1:UGYAvKxe3sBsEDzO8ZeWOSlIQfWFlxbzLZe7hwFURr0=
 github.com/mailru/easyjson v0.7.7/go.mod h1:xzfreul335JAWq5oZzymOObrkdz5UnU4kGfJJLY9Nlc=
 github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
 github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
 github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
-github.com/stretchr/testify v1.6.1 h1:hDPOHmpOpP40lSULcqw7IrRb/u7w6RpDC9399XyoNd0=
-github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
+github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw=
+github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo=
+github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
+github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU=
+github.com/stretchr/testify v1.8.1 h1:w7B6lhMri9wdJUVmEZPGGhZzrYTPvgJArz7wNPgYKsk=
+github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
-gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c h1:dUUwHk2QECo/6vqA44rthZ8ie2QXMNeKRTHCNY2nXvo=
 gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
+gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
+gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=

go.sum

@@ -6,6 +6,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"reflect"
+	"unicode/utf8"
 
 	"github.com/buger/jsonparser"
 	"github.com/mailru/easyjson/jwriter"
@@ -116,30 +117,33 @@ func (om *OrderedMap[K, V]) UnmarshalJSON(data []byte) error {
 			var key K
 			var value V
 
-			switch tkp := any(&key).(type) {
+			switch typedKey := any(&key).(type) {
 			case *string:
-				*tkp = string(keyData)
-			case encoding.TextUnmarshaler:
-				if err := tkp.UnmarshalText(keyData); err != nil {
+				s, err := decodeUTF8(keyData)
+				if err != nil {
 					return err
 				}
-			case *encoding.TextUnmarshaler:
-				// This is to preserve compatibility with original implementation
-				// that handled none pointer receivers, but I (xiegeo) believes this is unused.
-				if err := (*tkp).UnmarshalText(keyData); err != nil {
+				*typedKey = s
+			case encoding.TextUnmarshaler:
+				if err := typedKey.UnmarshalText(keyData); err != nil {
 					return err
 				}
 			case *int, *int8, *int16, *int32, *int64, *uint, *uint8, *uint16, *uint32, *uint64:
-				if err := json.Unmarshal(keyData, tkp); err != nil {
+				if err := json.Unmarshal(keyData, typedKey); err != nil {
 					return err
 				}
 			default:
 				// this switch takes care of wrapper types around primitive types, such as
 				// type myType string
 				switch reflect.TypeOf(key).Kind() {
 				case reflect.String:
-					convertedkeyData := reflect.ValueOf(keyData).Convert(reflect.TypeOf(key))
-					reflect.ValueOf(&key).Elem().Set(convertedkeyData)
+					s, err := decodeUTF8(keyData)
+					if err != nil {
+						return err
+					}
+
+					convertedKeyData := reflect.ValueOf(s).Convert(reflect.TypeOf(key))
+					reflect.ValueOf(&key).Elem().Set(convertedKeyData)
 				case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64,
 					reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64:
 					if err := json.Unmarshal(keyData, &key); err != nil {
@@ -158,3 +162,21 @@ func (om *OrderedMap[K, V]) UnmarshalJSON(data []byte) error {
 			return nil
 		})
 }
+
+func decodeUTF8(input []byte) (string, error) {
+	remaining, offset := input, 0
+	runes := make([]rune, 0, len(remaining))
+
+	for len(remaining) > 0 {
+		r, size := utf8.DecodeRune(remaining)
+		if r == utf8.RuneError && size <= 1 {
+			return "", fmt.Errorf("not a valid UTF-8 string (at position %d): %s", offset, string(input))
+		}
+
+		runes = append(runes, r)
+		remaining = remaining[size:]
+		offset += size
+	}
+
+	return string(runes), nil
+}

json.go

@@ -0,0 +1,116 @@
+package orderedmap
+
+// Adapted from https://github.com/dvyukov/go-fuzz-corpus/blob/c42c1b2/json/json.go
+
+import (
+	"encoding/json"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"testing"
+)
+
+func FuzzRoundTrip(f *testing.F) {
+	f.Fuzz(func(t *testing.T, data []byte) {
+		for _, testCase := range []struct {
+			name        string
+			constructor func() any
+			// should be a function that asserts that 2 objects of the type returned by constructor are equal
+			equalityAssertion func(*testing.T, any, any) bool
+		}{
+			{
+				name:              "with a string -> string map",
+				constructor:       func() any { return &OrderedMap[string, string]{} },
+				equalityAssertion: assertOrderedMapsEqual[string, string],
+			},
+			{
+				name:              "with a string -> int map",
+				constructor:       func() any { return &OrderedMap[string, int]{} },
+				equalityAssertion: assertOrderedMapsEqual[string, int],
+			},
+			{
+				name:              "with a string -> any map",
+				constructor:       func() any { return &OrderedMap[string, any]{} },
+				equalityAssertion: assertOrderedMapsEqual[string, any],
+			},
+			{
+				name:              "with a struct with map fields",
+				constructor:       func() any { return new(testFuzzStruct) },
+				equalityAssertion: assertTestFuzzStructEqual,
+			},
+		} {
+			t.Run(testCase.name, func(t *testing.T) {
+				v1 := testCase.constructor()
+				if json.Unmarshal(data, v1) != nil {
+					return
+				}
+
+				jsonData, err := json.Marshal(v1)
+				require.NoError(t, err)
+
+				v2 := testCase.constructor()
+				require.NoError(t, json.Unmarshal(jsonData, v2))
+
+				if !assert.True(t, testCase.equalityAssertion(t, v1, v2), "failed with input data %q", string(data)) {
+					// look at that what the standard lib does with regular map, to help with debugging
+
+					var m1 map[string]any
+					require.NoError(t, json.Unmarshal(data, &m1))
+
+					mapJsonData, err := json.Marshal(m1)
+					require.NoError(t, err)
+
+					var m2 map[string]any
+					require.NoError(t, json.Unmarshal(mapJsonData, &m2))
+
+					t.Logf("initial data = %s", string(data))
+					t.Logf("unmarshalled map = %v", m1)
+					t.Logf("re-marshalled from map = %s", string(mapJsonData))
+					t.Logf("re-marshalled from test obj = %s", string(jsonData))
+					t.Logf("re-unmarshalled map = %s", m2)
+				}
+			})
+		}
+	})
+}
+
+// only works for fairly basic maps, that's why it's just in this file
+func assertOrderedMapsEqual[K comparable, V any](t *testing.T, v1, v2 any) bool {
+	om1, ok1 := v1.(*OrderedMap[K, V])
+	om2, ok2 := v2.(*OrderedMap[K, V])
+
+	if !assert.True(t, ok1, "v1 not an orderedmap") ||
+		!assert.True(t, ok2, "v2 not an orderedmap") {
+		return false
+	}
+
+	success := assert.Equal(t, om1.Len(), om2.Len(), "om1 and om2 have different lengths: %d vs %d", om1.Len(), om2.Len())
+
+	for i, pair1, pair2 := 0, om1.Oldest(), om2.Oldest(); pair1 != nil && pair2 != nil; i, pair1, pair2 = i+1, pair1.Next(), pair2.Next() {
+		success = assert.Equal(t, pair1.Key, pair2.Key, "different keys at position %d: %v vs %v", i, pair1.Key, pair2.Key) && success
+		success = assert.Equal(t, pair1.Value, pair2.Value, "different values at position %d: %v vs %v", i, pair1.Value, pair2.Value) && success
+	}
+
+	return success
+}
+
+type testFuzzStruct struct {
+	M1 *OrderedMap[int, any]
+	M2 *OrderedMap[int, string]
+	M3 *OrderedMap[string, string]
+}
+
+func assertTestFuzzStructEqual(t *testing.T, v1, v2 any) bool {
+	s1, ok := v1.(*testFuzzStruct)
+	s2, ok := v2.(*testFuzzStruct)
+
+	if !assert.True(t, ok, "v1 not an testFuzzStruct") ||
+		!assert.True(t, ok, "v2 not an testFuzzStruct") {
+		return false
+	}
+
+	success := assertOrderedMapsEqual[int, any](t, s1.M1, s2.M1)
+	success = assertOrderedMapsEqual[int, string](t, s1.M2, s2.M2) && success
+	success = assertOrderedMapsEqual[string, string](t, s1.M3, s2.M3) && success
+
+	return success
+}