Skip to content

Commit

Permalink
Error on conversion to unstructured for invalid json.Marshalers.
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@benluddy
benluddy committed Jul 5, 2024
1 parent 92c1d38 commit f07895d
Show file tree
Hide file tree
Showing 2 changed files with 191 additions and 22 deletions.
53 changes: 34 additions & 19 deletions value/reflectcache.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ package value
import (
"bytes"
"encoding/json"
"errors"
"fmt"
"io"
"reflect"
"sort"
"sync"
Expand Down Expand Up @@ -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.
Expand Down
160 changes: 157 additions & 3 deletions value/reflectcache_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ limitations under the License.
package value

import (
"encoding/json"
"fmt"
"reflect"
"testing"
"time"
Expand Down Expand Up @@ -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},
Expand All @@ -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 {
Expand All @@ -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)
Expand Down Expand Up @@ -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)
}
})
}
}

0 comments on commit f07895d

Please sign in to comment.