search

shamblett / cbor

A CBOR implementation for Dart
MIT License
36 stars 15 forks source link

Uint8Buffer does not encode correctly #17

Closed jeremyherbert closed 3 years ago

jeremyherbert commented 3 years ago

I believe this is similar to https://github.com/shamblett/cbor/issues/13

Using a Uint8Buffer in a map value does not encode correctly; the output is not identical to the same data in Uint8List (which is correct).

Trying to decode the data in python using cbor2 results in a corrupted packet. It still decodes without an error, but the data is not correct.

jeremyherbert commented 3 years ago

After reviewing the code, this may not be a bug, but intended behaviour?

shamblett commented 3 years ago

OK, this doesn't ring any bells immediately but if I recall I think this is intended behaviour, I'll check further.

shamblett commented 3 years ago

OK, looks as though Uint8Buffer encoding should be supported in both an array and a map. When you say it doesn't encode correctly how do you mean? The buffer is output as it is supplied, i.e. as a raw buffer of bytes, no major type is encoded with it as the encoder doesn't know what the buffer data is, you are expected to encode this yourself before you encode the buffer, is this not what you expect?

jeremyherbert commented 3 years ago

I would have expected it to encode the same as a Uint8List, I personally can't really think of a use case where someone would want to directly pass through pre-encoded data.

shamblett commented 3 years ago

This sounds like a case for removing it, if the user wants to encode as a list then they can simply use a list, you could even leave the type as a Uint8Buffer and use the toList() method to pass it as a list. I think I'll mark it as deprecated and see if anyone shouts.

jeremyherbert commented 3 years ago

Sounds good. The reason I created this issue is because I was working with someone who spent a few hours trying to work out why their CBOR was being rejected by a server. So I think removing it is a good solution also. Feel free to close this if you think this is resolved.

shamblett commented 3 years ago

Ok, done, I'll push this out in the next release and keep an eye on it, thanks for the feedback.