Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

encoding/json: Unmarshal inserts map keys when the value is not appropriate for the map type #42508

Open
leventov opened this issue Nov 11, 2020 · 2 comments
Labels
NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@leventov
Copy link

package main

import (
	"fmt"
	"encoding/json"
)

func main() {
	var x map[string]int
	err := json.Unmarshal([]byte(`{"a":1,"b":"c"}`), &x)
	fmt.Printf("x = %#v, err = %s", x, err)
}

Output:

x = map[string]int{"a":1, "b":0}, err = json: cannot unmarshal string into Go value of type int

I think this violates the principle of least astonishment, "b" key shouldn't appear in the output x map.

The relevant part of the documentation for json.Unmarshal() is:

To unmarshal a JSON object into a map, Unmarshal first establishes a map to use. If the map is nil, Unmarshal allocates a new map. Otherwise Unmarshal reuses the existing map, keeping existing entries. Unmarshal then stores key-value pairs from the JSON object into the map. The map's key type must either be any string type, an integer, implement json.Unmarshaler, or implement encoding.TextUnmarshaler.

If a JSON value is not appropriate for a given target type, or if a JSON number overflows the target type, Unmarshal skips that field and completes the unmarshaling as best it can. If no more serious errors are encountered, Unmarshal returns an UnmarshalTypeError describing the earliest such error. In any case, it's not guaranteed that all the remaining fields following the problematic one will be unmarshaled into the target object.

It doesn't seem to me that it describes the above behaviour.

@mvdan mvdan changed the title encoding/json: Unmarshal() leaves keys mapping to zeros in map[string]int when the value is not an integer encoding/json: Unmarshal inserts map keys when the value is not appropriate for the map type Nov 11, 2020
@mvdan
Copy link
Member

mvdan commented Nov 11, 2020

I think this is a valid bug report, and the behavior also surprises me a little. I think it might be simply because the "init" operations (creating a zero value, inserting the key to the map) are out of order.

@mvdan mvdan added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Nov 11, 2020
@AlexanderYastrebov
Copy link
Contributor

I think it might be simply because the "init" operations (creating a zero value, inserting the key to the map) are out of order.

The fix does not seem to be trivial as the value is always initialized to the zero value:

if v.Kind() == reflect.Map {
elemType := t.Elem()
if !mapElem.IsValid() {
mapElem = reflect.New(elemType).Elem()
} else {
mapElem.Set(reflect.Zero(elemType))
}
subv = mapElem

so at the assignment point
if kv.IsValid() {
v.SetMapIndex(kv, subv)
}

it is not possible to tell whether the value was changed (it could also be set to zero) by
if err := d.value(subv); err != nil {
return err
}

#39149 could be related

@seankhliao seankhliao added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Aug 27, 2022
@seankhliao seankhliao added this to the Unplanned milestone Aug 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

4 participants