From f07895d137f456f3a6fee7a5a0ae6e1ba60ec95c Mon Sep 17 00:00:00 2001 From: Ben Luddy Date: Wed, 5 Jun 2024 13:18:06 -0400 Subject: [PATCH] Error on conversion to unstructured for invalid json.Marshalers. If a type's implementation of json.Marshaler returns bytes representing a valid JSON object or array followed by anything other than trailing whitespace, return an error rather than ignoring the trailing data. The documentation for the Marshaler interface indicates that implementations shouldn't do this, but it is safer to check (as json.Marshal does) than to rely on it. --- value/reflectcache.go | 53 +++++++----- value/reflectcache_test.go | 160 ++++++++++++++++++++++++++++++++++++- 2 files changed, 191 insertions(+), 22 deletions(-) diff --git a/value/reflectcache.go b/value/reflectcache.go index 55123c38..88693b87 100644 --- a/value/reflectcache.go +++ b/value/reflectcache.go @@ -19,7 +19,9 @@ package value import ( "bytes" "encoding/json" + "errors" "fmt" + "io" "reflect" "sort" "sync" @@ -379,34 +381,47 @@ const maxDepth = 10000 // unmarshal unmarshals the given data // If v is a *map[string]interface{}, numbers are converted to int64 or float64 func unmarshal(data []byte, v interface{}) error { + // Build a decoder from the given data + decoder := json.NewDecoder(bytes.NewBuffer(data)) + // Preserve numbers, rather than casting to float64 automatically + decoder.UseNumber() + // Run the decode + if err := decoder.Decode(v); err != nil { + return err + } + next := decoder.InputOffset() + if _, err := decoder.Token(); !errors.Is(err, io.EOF) { + tail := bytes.TrimLeft(data[next:], " \t\r\n") + return fmt.Errorf("unexpected trailing data at offset %d", len(data)-len(tail)) + } + + // If the decode succeeds, post-process the object to convert json.Number objects to int64 or float64 switch v := v.(type) { case *map[string]interface{}: - // Build a decoder from the given data - decoder := json.NewDecoder(bytes.NewBuffer(data)) - // Preserve numbers, rather than casting to float64 automatically - decoder.UseNumber() - // Run the decode - if err := decoder.Decode(v); err != nil { - return err - } - // If the decode succeeds, post-process the map to convert json.Number objects to int64 or float64 return convertMapNumbers(*v, 0) case *[]interface{}: - // Build a decoder from the given data - decoder := json.NewDecoder(bytes.NewBuffer(data)) - // Preserve numbers, rather than casting to float64 automatically - decoder.UseNumber() - // Run the decode - if err := decoder.Decode(v); err != nil { - return err - } - // If the decode succeeds, post-process the map to convert json.Number objects to int64 or float64 return convertSliceNumbers(*v, 0) + case *interface{}: + return convertInterfaceNumbers(v, 0) + default: - return json.Unmarshal(data, v) + return nil + } +} + +func convertInterfaceNumbers(v *interface{}, depth int) error { + var err error + switch v2 := (*v).(type) { + case json.Number: + *v, err = convertNumber(v2) + case map[string]interface{}: + err = convertMapNumbers(v2, depth+1) + case []interface{}: + err = convertSliceNumbers(v2, depth+1) } + return err } // convertMapNumbers traverses the map, converting any json.Number values to int64 or float64. diff --git a/value/reflectcache_test.go b/value/reflectcache_test.go index c86eeb7a..33eae60c 100644 --- a/value/reflectcache_test.go +++ b/value/reflectcache_test.go @@ -17,6 +17,8 @@ limitations under the License. package value import ( + "encoding/json" + "fmt" "reflect" "testing" "time" @@ -57,8 +59,9 @@ func (t Time) ToUnstructured() interface{} { func TestToUnstructured(t *testing.T) { testcases := []struct { - Data string - Expected interface{} + Data string + Expected interface{} + ExpectedErrorMessage string }{ {Data: `null`, Expected: nil}, {Data: `true`, Expected: true}, @@ -69,6 +72,12 @@ func TestToUnstructured(t *testing.T) { {Data: `{"a":1}`, Expected: map[string]interface{}{"a": int64(1)}}, {Data: `0`, Expected: int64(0)}, {Data: `0.0`, Expected: float64(0)}, + {Data: "{} \t\r\n", Expected: map[string]interface{}{}}, + {Data: "{} \t\r\n}", ExpectedErrorMessage: "error decoding object from json: unexpected trailing data at offset 6"}, + {Data: "{} \t\r\n{}", ExpectedErrorMessage: "error decoding object from json: unexpected trailing data at offset 6"}, + {Data: "[] \t\r\n", Expected: []interface{}{}}, + {Data: "[] \t\r\n]", ExpectedErrorMessage: "error decoding array from json: unexpected trailing data at offset 6"}, + {Data: "[] \t\r\n[]", ExpectedErrorMessage: "error decoding array from json: unexpected trailing data at offset 6"}, } for _, tc := range testcases { @@ -84,7 +93,13 @@ func TestToUnstructured(t *testing.T) { rv := reflect.ValueOf(custom) result, err := TypeReflectEntryOf(rv.Type()).ToUnstructured(rv) if err != nil { - t.Fatal(err) + if tc.ExpectedErrorMessage == "" { + t.Fatal(err) + } else if got := err.Error(); got != tc.ExpectedErrorMessage { + t.Fatalf("expected error message %q but got %q", tc.ExpectedErrorMessage, got) + } + } else if tc.ExpectedErrorMessage != "" { + t.Fatalf("expected error message %q but got nil error", tc.ExpectedErrorMessage) } if !reflect.DeepEqual(result, tc.Expected) { t.Errorf("expected %#v but got %#v", tc.Expected, result) @@ -199,3 +214,142 @@ func TestTypeReflectEntryOf(t *testing.T) { }) } } + +func TestUnmarshal(t *testing.T) { + for _, tc := range []struct { + JSON string + IntoType reflect.Type + Want interface{} + WantError bool + }{ + { + JSON: "{}}", + IntoType: reflect.TypeOf([0]interface{}{}).Elem(), + Want: map[string]interface{}{}, + WantError: true, + }, + { + JSON: `1.0`, + IntoType: reflect.TypeOf(json.Number("")), + Want: json.Number("1.0"), + }, + { + JSON: `1`, + IntoType: reflect.TypeOf(json.Number("")), + Want: json.Number("1"), + }, + { + JSON: `1.0`, + IntoType: reflect.TypeOf(float64(0)), + Want: float64(1), + }, + { + JSON: `1`, + IntoType: reflect.TypeOf(float64(0)), + Want: float64(1), + }, + { + JSON: `1.0`, + IntoType: reflect.TypeOf(int64(0)), + Want: int64(0), + WantError: true, + }, + { + JSON: `1`, + IntoType: reflect.TypeOf(int64(0)), + Want: int64(1), + }, + { + JSON: `1.0`, + IntoType: reflect.TypeOf([0]interface{}{}).Elem(), + Want: float64(1), + }, + { + JSON: `1`, + IntoType: reflect.TypeOf([0]interface{}{}).Elem(), + Want: int64(1), + }, + { + JSON: `[1.0,[1.0],{"":1.0}]`, + IntoType: reflect.TypeOf([0]interface{}{}).Elem(), + Want: []interface{}{ + float64(1), + []interface{}{float64(1)}, + map[string]interface{}{"": float64(1)}, + }, + }, + { + JSON: `[1.0,[1.0],{"":1.0}]`, + IntoType: reflect.TypeOf([]interface{}{}), + Want: []interface{}{ + float64(1), + []interface{}{float64(1)}, + map[string]interface{}{"": float64(1)}, + }, + }, + { + JSON: `[1,[1],{"":1}]`, + IntoType: reflect.TypeOf([0]interface{}{}).Elem(), + Want: []interface{}{ + int64(1), + []interface{}{int64(1)}, + map[string]interface{}{"": int64(1)}, + }, + }, + { + JSON: `[1,[1],{"":1}]`, + IntoType: reflect.TypeOf([]interface{}{}), + Want: []interface{}{ + int64(1), + []interface{}{int64(1)}, + map[string]interface{}{"": int64(1)}, + }, + }, + { + JSON: `{"x":1.0,"y":[1.0],"z":{"":1.0}}`, + IntoType: reflect.TypeOf([0]interface{}{}).Elem(), + Want: map[string]interface{}{ + "x": float64(1), + "y": []interface{}{float64(1)}, + "z": map[string]interface{}{"": float64(1)}, + }, + }, + { + JSON: `{"x":1.0,"y":[1.0],"z":{"":1.0}}`, + IntoType: reflect.TypeOf(map[string]interface{}{}), + Want: map[string]interface{}{ + "x": float64(1), + "y": []interface{}{float64(1)}, + "z": map[string]interface{}{"": float64(1)}, + }, + }, + { + JSON: `{"x":1,"y":[1],"z":{"":1}}`, + IntoType: reflect.TypeOf([0]interface{}{}).Elem(), + Want: map[string]interface{}{ + "x": int64(1), + "y": []interface{}{int64(1)}, + "z": map[string]interface{}{"": int64(1)}, + }, + }, + { + JSON: `{"x":1,"y":[1],"z":{"":1}}`, + IntoType: reflect.TypeOf(map[string]interface{}{}), + Want: map[string]interface{}{ + "x": int64(1), + "y": []interface{}{int64(1)}, + "z": map[string]interface{}{"": int64(1)}, + }, + }, + } { + t.Run(fmt.Sprintf("%s into %v", tc.JSON, reflect.PointerTo(tc.IntoType)), func(t *testing.T) { + into := reflect.New(tc.IntoType) + if err := unmarshal([]byte(tc.JSON), into.Interface()); tc.WantError != (err != nil) { + t.Fatalf("unexpected error: %v", err) + } + if got := into.Elem().Interface(); !reflect.DeepEqual(tc.Want, got) { + t.Errorf("want %#v (%T), got %#v (%T)", tc.Want, tc.Want, got, got) + } + }) + } +}