encoding/json: clarify what happens when unmarshaling into a non-empty interface{}

[deuill]

What version of Go are you using (go version)?

go version go1.10.3 linux/amd64

What operating system and processor architecture are you using (go env)?

GOARCH="amd64"
GOBIN=""
GOCACHE="/home/deuill/.cache/go-build"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/deuill/.go"
GORACE=""
GOROOT="/usr/lib/go"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build634949289=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Link to Play: https://play.golang.org/p/37E1QHWofMy

Passing a pointer to a struct (anonymous or not) as type interface{} to json.Unmarshal will have the original type replaced with a map[string]interface{} (correctly decoded, however). It appears the difference between correct and incorrect result is changing:

var nopointer interface{} = Object{}

to:

var nopointer = Object{}

In the example above. A interface{} containing a pointer to a struct works as expected, e.g.:

var pointer interface{} = &Object{}

is the same as:

var pointer = &Object{}

What did you expect to see?

The following results, in order of desirability:

• The correct struct type (Object, for the example above) populated with data from the JSON object given.
• An error denoting the inability to unmarshal into the underlying type given.
• The behaviour above documented in the documentation for json.Unmarshal.

What did you see instead?

None of the above? The use case that led to this issue for me was giving constructors types, where these types would be used as "templates" of sorts for populating with data in subsequent requests. For example:

type Decoder interface {
    Decode(*http.Request) (interface{}, error)
}

type RequestHandler func(interface{}) (interface{}, error)

type Request struct {
    Value interface{}
}

func (r Request) Decode(r *http.request) (interface{}, error) {
    // Read body from request
    // ...

    if err := json.Unmarshal(body, &r.Value); err != nil {
        return nil, err
    }

    return r, nil
}

type User struct {
    Name string `json:"name"`
}

func NewHandler(decoder Decoder, handler RequestHandler) http.HandlerFunc {
    return func(w http.ResponseWriter, r *http.Request) {
        req, err := decoder.Decode(r)
        if err != nil {
            return
        }

        resp, err := handler(req)
        if err != nil {
            return
        }

        // Encode response into ResponseWriter etc.
        // ...
    }
}

It is implied, then, that NewHandler is called with the bare type:

NewHandler(Request{Value: User{}}, userHandler)

Which will give "fresh" values to the RequestHandler each time, as they emerge from Request.Decode. If given a pointer, i.e.:

NewHandler(Request{Value: &User{}}, userHandler)

Previous requests will populate the pointer to User, leaving garbage data for future requests.
Apologies for the perhaps obtuse example.

[deuill]

Noted that this doesn't seem to be a regression or recent change, as the same behavior can be found on all Go versions I could test (>= 1.2).

The documentation for json.Unmarshal is sort of ambiguous as to whether it considers struct values in interface{} types to be one or the other, and the results are non-orthogonal/inconsistent between Marshal and Unmarshal:

https://play.golang.org/p/cBQv0DOrPhG

Also worth noting that this isn't just an issue when essentially passing *interface{}, as nested structures are subject to the same rules:

https://play.golang.org/p/HmZXqM1gDYs

[andybons]

@dsnet @bradfitz

[bradfitz]

I suppose the Unmarshal docs could elaborate on what happens when a *interface{} is non-nil and pointing to a non-nil interface{} containing a pointer.

What did you expect to see?
The following results, in order of desirability:

• The correct struct type (Object, for the example above) populated with data from the JSON object given.

I don't even think it's possible to do the first thing you want there. Once you put a struct value in an interface, it's no longer addressable and nothing can mutate it.

[deuill]

Correct, it doesn't seem we're able to modify struct values once placed in an interface{}, even if we were originally given a *interface{} or a pointer to the parent struct for which our interface{} is a field.

However, it appears it's possible to create a new zero value for the underlying type with reflect.New, and then assign the correct type into the interface{} (as you would the map[string]interface{} fallback). The logic happens here, and we'd have to check for v.IsNil or v.Elem().Kind() != reflect.Invalid in addition to v.NumMethod() == 0.

I can try and work on a patch if the current functionality is considered flawed, knowing that this is quite possibly a breaking change that is hard to check for statically or with go vet (however pointless it might be, relying on setting a struct and handling a map[string]interface{} on the other side), and that the encoding/json package is considered frozen.

I confess to a certain amount of confusion on the rules behind set-ability in the reflect package, as I try to avoid using it unless necessary, and had to re-read The Laws of Reflection and the documentation for the package itself, which did little to dispel my confusion for this particular case.

If the behavior cannot be changed, documenting it may help, as the underlying rules are somewhat subtle.

[gopherbot]

Change https://golang.org/cl/135421 mentions this issue: encoding/json: resolve struct, map, array, or slice values in interface types