tafia / quick-protobuf

A rust implementation of protobuf parser
MIT License
452 stars 87 forks source link

Serialization without prefix length #202

Open JalonWong opened 2 years ago

JalonWong commented 2 years ago

Why do serialize_into_slice() and deserialize_from_slice() add an an extra length before the data? This seems to be incompatible with the official library.

Can I add some new functions like serialize_into_slice_without_len() and deserialize_from_slice_without_len() ?

AlaaZorkane commented 2 years ago

Yes! Not only serialize...() functions but also the Writer

AlaaZorkane commented 2 years ago

Hey @JalonWong just letting you know that I opened a PR here: https://github.com/tafia/quick-protobuf/pull/208 - Please take a look and give your feedback if possible, thanks! 🙏

JalonWong commented 2 years ago

Hey @JalonWong just letting you know that I opened a PR here: #208 - Please take a look and give your feedback if possible, thanks! 🙏

The PR is pretty good. Also, That will be more handy If the serialize_into_slice*() can return the encoded length.

AlaaZorkane commented 2 years ago

The PR is pretty good. Also, That will be more handy If the serialize_into_slice*() can return the encoded length.

Thanks! I added it with the appropriate tests in the PR, if there is anything else let me know on the PR thread 🙏

nerdrew commented 2 years ago

FYI: You can serialize / deserialize protos without length prefixes like this:

let mut buf = Vec::new();
let mut writer = Writer::new(&mut buf);
MyMessage::default().write_message(&mut writer)?;

let mut reader = BytesReader::from_bytes(&buf);
let proto = MyMessage::from_reader(&mut reader, &buf)?;
snproj commented 1 year ago

Hi! Since this seems to be a common stumbling block for new users, would you guys prefer:

  1. Modify serialize_into_slice() and deserialize_from_slice() to no longer include the length prefix
  2. Modify serialize_into_slice() and deserialize_from_slice() to no longer include the length prefix, AND add two new methods that keep the old behavior (still a breaking change)
  3. Keep serialize_into_slice() and deserialize_from_slice() the same, but add documentation
  4. Keep serialize_into_slice() and deserialize_from_slice() the same, but add documentation AND two new methods that follow expectations better (basically #208)

I personally prefer 1), since we will most likely have breaking changes already from #235 and #240.

Linking #214 and #208 as well

ScarboroughCoral commented 1 year ago

I have the same issue when I want to use protocol buffers between rust and typescript, protobufjs decoded error because the prefix length. Now I can middleware the writer remove the prefix length but I think it's better there is a function without prefix length 😄