fix incorrect usage of Buf::bytes in simplejson

Reviewed By: Imxset21

Differential Revision: D26609805

fbshipit-source-id: f9007752868aa24d08a4ca86ac5072f3c9056755

src/dep_tests/simplejson.rs

@@ -16,11 +16,13 @@
 
 use anyhow::Result;
 use fbthrift::simplejson_protocol::{deserialize, serialize};
+use fbthrift::{simplejson_protocol::SimpleJsonProtocolDeserializer, Deserialize};
 use fbthrift_test_if::{
     Basic, Containers, En, MainStruct, MainStructNoBinary, Small, SubStruct, Un, UnOne,
 };
 use std::collections::BTreeMap;
 use std::default::Default;
+use std::io::Cursor;
 
 #[test]
 fn test_large_roundtrip() -> Result<()> {
@@ -365,3 +367,29 @@ fn test_serde_compat() -> Result<()> {
     assert_eq!(r, deserialize(&serde_s).unwrap());
     Ok(())
 }
+
+#[test]
+fn test_multiple_deser() -> Result<()> {
+    // Tests that we don't too eagerly advance the buffer
+    let b1 = Basic {
+        b: true,
+        b2: false,
+        ..Default::default()
+    };
+    let b2 = Basic {
+        b: true,
+        b2: true,
+        ..Default::default()
+    };
+    // serialize it and assert that it serializes correctly
+    let s1 = String::from_utf8(serialize(&b1).to_vec()).unwrap();
+    let s2 = String::from_utf8(serialize(&b2).to_vec()).unwrap();
+    let to_check = format!("{} {}", s1, s2);
+
+    let mut deserializer = SimpleJsonProtocolDeserializer::new(Cursor::new(to_check.as_bytes()));
+    // Assert that deserialize builts the exact same struct
+    assert_eq!(b1, Basic::read(&mut deserializer)?);
+    assert_eq!(b2, Basic::read(&mut deserializer)?);
+
+    Ok(())
+}

src/simplejson_protocol.rs

@@ -365,16 +365,24 @@ impl<B: Buf> SimpleJsonProtocolDeserializer<B> {
     }
 
     // TODO(azw): codify this (number of bytes left) in the Deserializer API
-    pub fn peek_bytes(&self, len: usize) -> Option<&[u8]> {
-        if self.buffer.bytes().len() >= len {
-            Some(&self.buffer.bytes()[..len])
+    // TODO(azw): decrease the number of calls to `remaining`
+    /// Returns a byte from the underly buffer if there is enough remaining
+    // `bytes` (named `chunks` in bytes1.0) does not represent a contiguous slice
+    // of the remaining bytes. Implicitly, if the remaining is > 0, then all we
+    // can do is read 1 byte (and not more) from the chunk
+    pub fn peek(&self) -> Option<u8> {
+        if self.buffer.remaining() > 0 {
+            // panic free as it follows:
+            // https://docs.rs/bytes/1.0.1/src/bytes/buf/buf_impl.rs.html#284-289
+            Some(self.buffer.bytes()[0])
         } else {
             None
         }
     }
 
-    fn peek(&self) -> Option<u8> {
-        self.peek_bytes(1).map(|s| s[0])
+    /// Like peek but panics if there is none remaining
+    fn peek_can_panic(&self) -> u8 {
+        self.buffer.bytes()[0]
     }
 
     fn strip_whitespace(&mut self) {
@@ -389,15 +397,19 @@ impl<B: Buf> SimpleJsonProtocolDeserializer<B> {
     // strip whitespace before and after
     fn eat(&mut self, val: &[u8]) -> Result<()> {
         self.strip_whitespace();
-        match self.peek_bytes(val.len()) {
-            Some(slice) => {
-                if slice != val {
-                    bail!("Expected {:?}, got {:?}", val, slice)
-                }
+
+        if self.buffer.remaining() < val.len() {
+            bail!("Expected {:?}, not enough remaining", val)
+        }
+
+        for to_check in val {
+            let b = self.peek_can_panic();
+            if b != *to_check {
+                // TODO(azw): better error messages
+                bail!("Expected {} got {}", to_check, b)
             }
-            None => bail!("Expected {:?}", val),
+            self.advance(1);
         }
-        self.advance(val.len());
         self.strip_whitespace();
         Ok(())
     }
@@ -558,8 +570,8 @@ impl<B: Buf> SimpleJsonProtocolDeserializer<B> {
 
     fn check_null(&mut self) -> bool {
         self.strip_whitespace();
-        match self.peek_bytes(4) {
-            Some(slice) if slice == b"null" => {
+        match self.peek() {
+            Some(b'n') => {
                 return true;
             }
             _ => false,