ugorji / go

idiomatic codec and rpc lib for msgpack, cbor, json, etc. msgpack.org[Go]
MIT License
1.87k stars 295 forks source link

Not decoding slice #70

Closed fgrzadkowski closed 9 years ago

fgrzadkowski commented 9 years ago

I'm decoding Pod object: https://github.com/GoogleCloudPlatform/kubernetes/blob/master/pkg/api/types.go#L860

using this code:

    data, err := ioutil.ReadFile("pod_example.json")
    if err != nil {
        panic("unexpected error while reading file")
    }
    var pod api.Pod
    if err := api.Scheme.DecodeInto(data, &pod); err != nil {
        panic("unexpected error decoding pod")
    }

    func() {
        data := make([]byte, 0)
        h := new(codec.JsonHandle)
        codec.NewEncoderBytes(&data, h).Encode(pod)
        glog.Errorf("%s", string(data))
        var podCopy api.Pod
        codec.NewDecoderBytes(data, h).Decode(&podCopy)

        if !api.Semantic.DeepDerivative(pod, &podCopy) {
            t.Fatalf("Incorrect conversion: expected %+v, got %+v", pod, &podCopy)
        }
    }()

where pod_example.json is https://github.com/GoogleCloudPlatform/kubernetes/blob/master/pkg/conversion/pod_example.json

For some reason ContainerStatuses is not decoded. I verified that it is correctly encoded, but decoder ignores it for some reason.

Am I doing something wrong?

ugorji commented 9 years ago

Please provide a small reproducer. It will take me longer to set this up, which will delay my looking into it.

fgrzadkowski commented 9 years ago

I was not able to reproduce exactly the same error, but I found a similar one, that looks very similar:

import (
    "reflect"
    "testing"

    "github.com/google/gofuzz"
    "github.com/ugorji/go/codec"
)

// DeepCopy makes a deep copy of source or returns an error.
func DeepCopy(source interface{}) (interface{}, error) {
    v := reflect.New(reflect.TypeOf(source))

    data := make([]byte, 0)
    h := new(codec.JsonHandle)
    if err := codec.NewEncoderBytes(&data, h).Encode(source); err != nil {
        return nil, err
    }
    if err := codec.NewDecoderBytes(data, h).Decode(v.Interface()); err != nil {
        return nil, err
    }

    return v.Elem().Interface(), nil
}

type Test struct {
    X []int
    Y []byte
}

func TestCodecsBug(t *testing.T) {
    f := fuzz.New().NilChance(.5).NumElements(0, 100)

    obj := Test{}
    obj2 := reflect.New(reflect.TypeOf(obj)).Interface()
    f.Fuzz(obj2)
    obj3, err := DeepCopy(obj2)
    if err != nil {
        t.Errorf("Error: couldn't copy %#v", obj2)
    }
    if len(obj2.(*Test).X) != len(obj3.(*Test).X) || len(obj2.(*Test).Y) != len(obj3.(*Test).Y) {
        t.Errorf("expected %+v\ngot %+v", obj2, obj3)
    }
    f.Fuzz(obj2)
}

I run this in a loop:

$ for i in {1..100}; do echo $i; go test ./pkg/conversion/ || break; done
1
ok      github.com/GoogleCloudPlatform/kubernetes/pkg/conversion    0.648s
2
ok      github.com/GoogleCloudPlatform/kubernetes/pkg/conversion    0.614s
3
ok      github.com/GoogleCloudPlatform/kubernetes/pkg/conversion    0.588s
4
--- FAIL: TestCodecsBug (0.00s)
    deep_copy_test.go:73: expected &{X:[] Y:[204 127 229 83 217 111 1 214 209 20 17 23 228 49 142 255 40 199 69 191 33 72 81 62 3 86 126 150 7 217 1 204 113 42 9 204 219 147 193 98 20 183 188 154 7 170 148 255 6 182 241 88 240 17 16 124 33 160 106 8 69 155 31 100 4 185 89 233 4 192 201 173 214 146 16 224 183 119 50 59 154 206 8 158 187 168 79 147 180 93 33 189 103 123 160 146 8]}
        got &{X:[] Y:[204 127 229 83 217 111 1 214 209 20 17 23 228 49 142 255 40 199 69 191 33 72 81 62 3 86 126 150 7 217 1 204 113 42 9 204 219 147 193 98 20 183 188 154 7 170 148 255 6 182 241 88 240 17 16 124 33 160 106 8 69 155 31 100 4 185 89 233 4 192 201 173 214 146 16 224 183 119 50 59 154 206 8 158 187 168 79 147 180 93 33 189 103 123 160 146 8 0 0]}
FAIL
FAIL    github.com/GoogleCloudPlatform/kubernetes/pkg/conversion    0.616s

As you can see there are two additional 0 in the second object. What's interesting is that this is flaky and breaks only sometimes. Any ideas what could the reason?

fgrzadkowski commented 9 years ago

UPDATE: Actually it's enough to have []byte in the struct.

fgrzadkowski commented 9 years ago

UPDATE: I've noticed that each time there is a problem, the base64 string representation of bytes ends with "=", e.g.:

Source: {"Y":"LpiymWyBMg8C9Q69thLkoxGQ9ZY5V7VMckqVDDeRsT/Ai7m9o9oEBBJYbQ8mEFE4SkNfpFmv9SnWxKykSRJE5suIWXaRrLf/m/tK7E7MWma4sQ0="} Destination: {"Y":"LpiymWyBMg8C9Q69thLkoxGQ9ZY5V7VMckqVDDeRsT/Ai7m9o9oEBBJYbQ8mEFE4SkNfpFmv9SnWxKykSRJE5suIWXaRrLf/m/tK7E7MWma4sQ0A"}

fgrzadkowski commented 9 years ago

The bug is here: https://github.com/ugorji/go/blob/master/codec/json.go#L736

It should be:

realLen, _ := base64.StdEncoding.Decode(bsOut, bs0)
bsOut = bsOut[:realLen]

Otherwise the slice may contain some zero bytes, as DecodeLen returns maximum lenght, not the real lenght.