ugorji / go

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

Latest commit causes decode to break #17

Closed armon closed 11 years ago

armon commented 11 years ago

Latest version is causing issues for us, as we now get the following:

/opt/gopath/src/github.com/ugorji/go/codec/helper.go:504 (0x51c024)
    com/ugorji/go/codec.panicToErr: debug.PrintStack()
/opt/go/src/pkg/runtime/panic.c:229 (0x413f61)
    panic: reflect·call(d->fn, (byte*)d->args, d->siz);
/opt/go/src/pkg/runtime/iface.c:564 (0x40ae58)
    ifacehash1: runtime·panic(err);
/opt/go/src/pkg/runtime/iface.c:584 (0x40af59)
    efacehash: return ifacehash1(a.data, a.type, h);
/opt/go/src/pkg/runtime/alg.c:387 (0x4032eb)
    nilinterhash: *h = runtime·efacehash(*(Eface*)a, *h ^ M0) * M1;
/opt/go/src/pkg/runtime/hashmap.c:597 (0x40834d)
    hash_insert: t->key->alg->hash(&hash, t->key->size, key);
/opt/go/src/pkg/runtime/hashmap.c:1272 (0x4099d7)
    mapassign: hash_insert(t, h, ak, av);
/opt/go/src/pkg/runtime/hashmap.c:1301 (0x409a5d)
    mapassign1: runtime·mapassign(t, h, ak, av);
/opt/gopath/src/github.com/ugorji/go/codec/decode.go:904 (0x51563f)
    com/ugorji/go/codec.(*Decoder).decMapIntfIntf: m[mk] = mv
/opt/gopath/src/github.com/ugorji/go/codec/decode.go:682 (0x5141ff)
    com/ugorji/go/codec.(*Decoder).decode: d.decMapIntfIntf(v)
/opt/gopath/src/github.com/ugorji/go/codec/decode.go:341 (0x511b2b)
    com/ugorji/go/codec.(*decFnInfo).kInterface: f.d.decode(v)
/opt/gopath/src/github.com/ugorji/go/codec/decode.go:817 (0x51496a)
    com/ugorji/go/codec.(*Decoder).decodeValue: fn.f(fn.i, rv)
/opt/gopath/src/github.com/ugorji/go/codec/decode.go:687 (0x51402f)
    com/ugorji/go/codec.(*Decoder).decode: d.decodeValue(reflect.ValueOf(iv).Elem())
/opt/gopath/src/github.com/ugorji/go/codec/decode.go:626 (0x5135e7)
    com/ugorji/go/codec.(*Decoder).Decode: d.decode(v)
/opt/gopath/src/github.com/ugorji/go/codec/rpc.go:79 (0x51fbc1)
    com/ugorji/go/codec.(*rpcCodec).read: return c.dec.Decode(obj)
/opt/gopath/src/github.com/ugorji/go/codec/rpc.go:87 (0x51fc69)
    com/ugorji/go/codec.(*rpcCodec).ReadResponseBody: return c.read(body)
/opt/go/src/pkg/net/rpc/client.go:138 (0x4ac74e)
    (*Client).input: err = client.codec.ReadResponseBody(call.Reply)
/opt/go/src/pkg/runtime/proc.c:1223 (0x417ae0)
    goexit: runtime·goexit(void)
2013/10/17 00:52:38 rpc: client protocol error: runtime error: hash of unhashable type []uint8

Still trying to get a minimal test case, but I can reproduce this in our project test suite.

ugorji commented 11 years ago

Thanks.

No need to send a minimal test case. I see the error. I'd upload a fix pronto.

armon commented 11 years ago

Thanks!

armon commented 11 years ago

Sorry, quick question, will the conversion of []byte to string potentially cause problems if the data is not UTF8 format?

armon commented 11 years ago

The latest commit does fix our issue though. Thanks for the quick turn around!

ugorji commented 11 years ago

Please re-test and confirm fix.

No - it will not cause an issue.

This is in line with the old logic before (see kMap method). We special-case and fast-tracked some common map types, and I forgot to replicate that check in there.

The reason for doing this, is that the original msgpack encodes a string as raw []byte (there was no separate string or binary type). When decoding, we need to make a determination what to do when we see raw bytes. We use the following to make that determination:

A string in Go is just an immutable string of bytes - it doesn't specify an encoding. There are libs to treat it as utf8, utf16, runes (utf-32), etc.

ugorji commented 11 years ago

And thanks for reporting this quickly. Much appreciated. I pride myself on not overlooking things, and am happy you caught this one for me quickly before others ran across it, and wasted time checking if the problem was on their end.

armon commented 11 years ago

Thanks for the response and the excellent library! Cheers.