Skip to content

Commit

Permalink
fix: correctly unmarshal JSON 'null' value for maps and slices
Browse files Browse the repository at this point in the history
  • Loading branch information
padamstx committed Jun 2, 2020
1 parent a07791f commit 0117461
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 32 deletions.
80 changes: 54 additions & 26 deletions v4/core/unmarshal_v2.go
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,11 @@ func unmarshalModelSlice(rawInput interface{}, propertyName string, result inter

var rawMsg json.RawMessage
rawMsg, foundIt = rawMap[propertyName]
if foundIt && rawMsg != nil {

// If we didn't find the property containing the JSON input, then bail out now.
if !foundIt || isJsonNull(rawMsg) {
return
} else {
err = json.Unmarshal(rawMsg, &rawSlice)
if err != nil {
causedBy := fmt.Errorf(errorUnmarshalModel, propInsert(propertyName),
Expand Down Expand Up @@ -347,6 +351,15 @@ func unmarshalModelSlice(rawInput interface{}, propertyName string, result inter
return
}

// isJsonNull returns true iff 'rawMsg' is exlicitly nil or contains a JSON "null" value.
func isJsonNull(rawMsg json.RawMessage) bool {
var nullLiteral = []byte("null")
if rawMsg == nil || string(rawMsg) == string(nullLiteral) {
return true
}
return false
}

//
// unmarshalModelMap unmarshals 'rawInput' into a map[string]<model>.
//
Expand Down Expand Up @@ -376,6 +389,10 @@ func unmarshalModelMap(rawInput interface{}, propertyName string, result interfa
return
}

if !foundInput {
return
}

// Get a "reflective" view of the result map and initialize it.
rResultMap := reflect.ValueOf(result).Elem()
rResultMap.Set(reflect.MakeMap(reflect.TypeOf(result).Elem()))
Expand All @@ -391,7 +408,7 @@ func unmarshalModelMap(rawInput interface{}, propertyName string, result interfa
reflect.TypeOf(result).Elem().String(), err.Error())
return
}

for k, v := range rawMap {
// Unmarshal the map entry's value (a json.RawMessage) into a map[string]RawMessage.
// The resulting map should contain an instance of the model.
Expand Down Expand Up @@ -451,36 +468,44 @@ func unmarshalModelSliceMap(rawInput interface{}, propertyName string, result in
return
}

if !foundInput {
return
}

// Get a "reflective" view of the result map and initialize it.
rResultMap := reflect.ValueOf(result).Elem()
rResultMap.Set(reflect.MakeMap(reflect.TypeOf(result).Elem()))

if foundInput && rawMap != nil {
for k, v := range rawMap {
// Each value in 'rawMap' should contain an instance of []<model>.
// We'll first unmarshal each value into a []jsonRawMessage, then unmarshal that
// into a []<model> using unmarshalModelSlice.
var rawSlice []json.RawMessage
err = json.Unmarshal(v, &rawSlice)
if err != nil {
err = fmt.Errorf(errorUnmarshalModel, propInsert(propertyName),
reflect.TypeOf(result).Elem().String(), err.Error())
return
}

// Construct a slice of the correct type.
rSliceValue := reflect.New(reflect.TypeOf(result).Elem().Elem())

// Unmarshal rawSlice into a model slice.
err = unmarshalModelSlice(rawSlice, "", rSliceValue.Interface(), unmarshaller)
if err != nil {
err = fmt.Errorf(errorUnmarshalModel, propInsert(propertyName),
reflect.TypeOf(result).Elem().String(), err.Error())
return

// Make sure our slice raw message isn't an explicit JSON null value.
if !isJsonNull(v) {
// Each value in 'rawMap' should contain an instance of []<model>.
// We'll first unmarshal each value into a []jsonRawMessage, then unmarshal that
// into a []<model> using unmarshalModelSlice.
var rawSlice []json.RawMessage
err = json.Unmarshal(v, &rawSlice)
if err != nil {
err = fmt.Errorf(errorUnmarshalModel, propInsert(propertyName),
reflect.TypeOf(result).Elem().String(), err.Error())
return
}

// Construct a slice of the correct type.
rSliceValue := reflect.New(reflect.TypeOf(result).Elem().Elem())

// Unmarshal rawSlice into a model slice.
err = unmarshalModelSlice(rawSlice, "", rSliceValue.Interface(), unmarshaller)
if err != nil {
err = fmt.Errorf(errorUnmarshalModel, propInsert(propertyName),
reflect.TypeOf(result).Elem().String(), err.Error())
return
}

// Now add the unmarshalled model slice to the result map (reflectively, of course :) ).
rResultMap.SetMapIndex(reflect.ValueOf(k), rSliceValue.Elem())
}

// Now add the unmarshalled model slice to the result map (reflectively, of course :) ).
rResultMap.SetMapIndex(reflect.ValueOf(k), rSliceValue.Elem())
}
}
return
Expand Down Expand Up @@ -511,7 +536,10 @@ func getUnmarshalInputSource(rawInput interface{}, propertyName string) (foundIn
if propertyName != "" {
var rawMsg json.RawMessage
rawMsg, foundInput = rawMap[propertyName]
if foundInput && rawMsg != nil {
if !foundInput || isJsonNull(rawMsg) {
foundInput = false
return
} else {
rawMap = make(map[string]json.RawMessage)
err = json.Unmarshal(rawMsg, &rawMap)
if err != nil {
Expand Down
18 changes: 12 additions & 6 deletions v4/core/unmarshal_v2_models_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -186,18 +186,21 @@ func TestUnmarshalModelSliceNil(t *testing.T) {
mySlice = nil
err = UnmarshalModel(rawMap, "missing_slice", &mySlice, UnmarshalMyModel)
assert.Nil(t, err)
assert.Nil(t, mySlice)
assert.Equal(t, 0, len(mySlice))

// Unmarshal an explicit null value.
mySlice = nil
err = UnmarshalModel(rawMap, "null_slice", &mySlice, UnmarshalMyModel)
assert.Nil(t, err)
assert.Nil(t, mySlice)
assert.Equal(t, 0, len(mySlice))

// Unmarshal an explicit null value.
// Unmarshal an empty slice.
mySlice = nil
err = UnmarshalModel(rawMap, "empty_slice", &mySlice, UnmarshalMyModel)
assert.Nil(t, err)
assert.NotNil(t, mySlice)
assert.Equal(t, 0, len(mySlice))
}

Expand Down Expand Up @@ -231,13 +234,14 @@ func TestUnmarshalModelMapNil(t *testing.T) {
myMap = nil
err = UnmarshalModel(rawMap, "missing_map", &myMap, UnmarshalMyModel)
assert.Nil(t, err)
assert.Nil(t, myMap)
assert.Equal(t, 0, len(myMap))

// Unmarshal an explicit null value.
myMap = nil
err = UnmarshalModel(rawMap, "null_map", &myMap, UnmarshalMyModel)
assert.Nil(t, err)
assert.NotNil(t, myMap)
assert.Nil(t, myMap)
assert.Equal(t, 0, len(myMap))

// Unmarshal an empty map.
Expand Down Expand Up @@ -292,12 +296,14 @@ func TestUnmarshalModelSliceMapNil(t *testing.T) {
mySliceMap = nil
err = UnmarshalModel(rawMap, "missing_prop", &mySliceMap, UnmarshalMyModel)
assert.Nil(t, err)
assert.Nil(t, mySliceMap)
assert.Equal(t, 0, len(mySliceMap))

// Unmarshal an explicit null value.
mySliceMap = nil
err = UnmarshalModel(rawMap, "null_prop", &mySliceMap, UnmarshalMyModel)
assert.Nil(t, err)
assert.Nil(t, mySliceMap)
assert.Equal(t, 0, len(mySliceMap))

// Unmarshal a map with an empty slice.
Expand All @@ -308,13 +314,13 @@ func TestUnmarshalModelSliceMapNil(t *testing.T) {
assert.NotNil(t, mySliceMap["empty_slice"])
assert.Equal(t, 0, len(mySliceMap["empty_slice"]))

// Unmarshal a map with an explicit null slice.
// Unmarshal a map with an explicit null slice. Result should be an empty map.
mySliceMap = nil
err = UnmarshalModel(rawMap, "null_slice_map", &mySliceMap, UnmarshalMyModel)
assert.Nil(t, err)
assert.Equal(t, 1, len(mySliceMap))
assert.NotNil(t, mySliceMap["null_slice"])
assert.Equal(t, 0, len(mySliceMap["null_slice"]))
assert.NotNil(t, mySliceMap)
assert.Equal(t, 0, len(mySliceMap))
assert.Nil(t, mySliceMap["null_slice"])
}

func TestUnmarshalModelStruct(t *testing.T) {
Expand Down

0 comments on commit 0117461

Please sign in to comment.