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

Latest commit causes decode to break #17

Closed
armon opened this issue Oct 17, 2013 · 7 comments
Closed

Latest commit causes decode to break #17

armon opened this issue Oct 17, 2013 · 7 comments
Labels

Comments

@armon
Copy link

armon commented Oct 17, 2013

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
Copy link
Owner

ugorji commented Oct 17, 2013

Thanks.

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

@armon
Copy link
Author

armon commented Oct 17, 2013

Thanks!

ugorji added a commit that referenced this issue Oct 17, 2013
This is inline with kMap logic. We needed to replicate it in the fast-path
decMapIntfIntf.

Fixes issue #17 .
@ugorji ugorji closed this as completed Oct 17, 2013
@armon
Copy link
Author

armon commented Oct 17, 2013

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

@armon
Copy link
Author

armon commented Oct 17, 2013

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

@ugorji
Copy link
Owner

ugorji commented Oct 17, 2013

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:

  • If we see raw bytes as key in a map, then it must have been a string (because Go doesn't support slices as map keys). So we just convert it to a string (else you get the error you received that hashmap cannot have a slice as a key value because slices do not support equality).
  • In any other situation that we see raw bytes in the stream, we use the value of the Option RawToString
    to determine what to do (either keep as []byte or convert to string).

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
Copy link
Owner

ugorji commented Oct 17, 2013

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
Copy link
Author

armon commented Oct 17, 2013

Thanks for the response and the excellent library! Cheers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants