dustin / go-coap

Implementation of CoAP in go.
MIT License
342 stars 103 forks source link

Doesn't parse token #15

Closed orian closed 9 years ago

orian commented 9 years ago

The library doesn't parse/skip token when parsing a message. Fixed in https://github.com/teslaworksumn/go-coap

dustin commented 9 years ago

Do you have a failing test case I can try?

orian commented 9 years ago

80 2 122 35 177 69 15 11 115 112 97 114 107 47 99 99 51 48 48 48 45 112 97 116 99 104 45 118 101 114 115 105 111 110 255 49 46 50 56

This are the bytes (received from Spark Core). ;-)

I think any valid message with token will do. It's not visible as long as one Marshal and Unmarshal using go-coap. But I deal with multilanguage system and this is how I've spot a problem.

Please compare: http://tools.ietf.org/html/draft-ietf-core-coap-10#section-3 and http://tools.ietf.org/html/rfc7252#section-3

The UnmarshalBinary reads tokenLen but never really reads the token: https://github.com/dustin/go-coap/blob/master/message.go#L452

Somewhere after the line 455 you should add: m.Token = make([]byte, tokenLen) copy(m.Token, data[4:4+tokenLen]) b := data[4+tokenLen:]

I've also checked and the option parsing implements an old version of RFC.

BTW thanks for a library :+1:

orian commented 9 years ago

I've integrated other's changes into my branch of your code. I've also fixed a memory leak. Unfortunately, the test are broken for now.

If you care, my repo is here, please consider pulling it: https://github.com/orian/go-coap

dustin commented 9 years ago

The difference in parsing that data between what I've already got and your example seems negligible:

Before:

coap.Message{Type:0x1, Code:0x2, MessageID:0x7a23, Token:[]uint8(nil), Payload:[]uint8{0x31, 0x2e, 0x32, 0x38}, opts:coap.options{coap.option{ID:0xb, Value:"E"}, coap.option{ID:0xb, Value:"spark/cc3000-patch-version"}}}

After:

coap.Message{Type:0x1, Code:0x2, MessageID:0x7a23, Token:[]uint8{}, Payload:[]uint8{0x31, 0x2e, 0x32, 0x38}, opts:coap.options{coap.option{ID:0xb, Value:"E"}, coap.option{ID:0xb, Value:"spark/cc3000-patch-version"}}}

Do you have an example that uses tokens?

I'm open to bringing in your changes, but they're not quite done in the style I'm used to. i.e. merges, fixes, etc… How do you feel about an am+cleanup import of the changes? Authors keep their names, but merges and fixups would be removed.