tafia / quick-protobuf

A rust implementation of protobuf parser
MIT License
446 stars 82 forks source link

fix: writer does not factor in length prefix #226

Closed snproj closed 1 year ago

snproj commented 1 year ago

Problems

This PR aims to fix #222.

There are two problems, one in serialize_into_vec() and one in serialize_into_slice(); both are caused by the same function: sizeof_len().

sizeof_len(x) is meant to be called on some length prefix x, and will return the length of the prefix itself, plus whatever length it encodes. This is meant for convenience.

However, the functions serialize_into_vec() and serialize_into_slice() make some mistakes regarding this.

Background: What is a length prefix?
> In protobuf, some fields are called length-delimited records, such as strings or repeated packed fields. They use a varint to encode the length of the data, before the data itself. This varint is called a [length prefix](https://developers.google.com/protocol-buffers/docs/encoding#:~:text=Length%20prefixes%20are%20another%20major%20concept%20in%20the%20wire%20format.%20The%20LEN%20wire%20type%20has%20a%20dynamic%20length%2C%20specified%20by%20a%20varint%20immediately%20after%20the%20tag%2C%20which%20is%20followed%20by%20the%20payload%20as%20usual.). When we want to get the length of the whole field, we have to not only get the length encoded by the prefix (the length of the data), but also the length of the prefix itself.

Problem 1: serialize_into_vec()

Forgets that sizeof_len() already takes into account the length encoded by the prefix (length of data), and adds len to it again.

Problem 2: serialize_into_slice()

Forgets that we need to count the length of the prefix itself, in addition to the length that it encodes (length of data).

Solutions

Fixing serialize_into_vec()

Remove the extra len.

@@ -310,7 +310,7 @@ impl<W: WriterBackend> Writer<W> {
 #[cfg(feature = "std")]
 pub fn serialize_into_vec<M: MessageWrite>(message: &M) -> Result<Vec<u8>> {
     let len = message.get_size();
-    let mut v = Vec::with_capacity(len + crate::sizeofs::sizeof_len(len));
+    let mut v = Vec::with_capacity(crate::sizeofs::sizeof_len(len));
     {
         let mut writer = Writer::new(&mut v);
         writer.write_message(message)?;

Fixing serialize_into_slice()

Check out.len() against sizeof_len(len) instead of len.

@@ -321,7 +321,7 @@ pub fn serialize_into_vec<M: MessageWrite>(message: &M) -> Result<Vec<u8>> {
 /// Serialize a `MessageWrite` into a byte slice
 pub fn serialize_into_slice<M: MessageWrite>(message: &M, out: &mut [u8]) -> Result<()> {
     let len = message.get_size();
-    if out.len() < len {
+    if out.len() < crate::sizeofs::sizeof_len(len) {
         return Err(Error::OutputBufferTooSmall);
     }
     {