RUST-454 Implement error case BSON corpus tests (#181)

Author: patrickfreed

Cargo.toml

@@ -28,7 +28,7 @@ byteorder = "1"
 chrono = "0.4"
 libc = "0.2"
 rand = "0.7"
-serde = "1.0"
+serde = { version = "1.0", features = ["derive"] }
 serde_json = { version = "1.0", features = ["preserve_order"] }
 time = "0.1"
 linked-hash-map = "0.5"
@@ -39,7 +39,6 @@ base64 = "0.12.1"
 
 [dev-dependencies]
 assert_matches = "1.2"
-serde_derive = "1.0"
 serde_bytes = "0.11"
 pretty_assertions = "0.6.1"
 

src/bson.rs

@@ -203,6 +203,12 @@ impl From<Binary> for Bson {
     }
 }
 
+impl From<Timestamp> for Bson {
+    fn from(ts: Timestamp) -> Bson {
+        Bson::Timestamp(ts)
+    }
+}
+
 impl<T> From<&T> for Bson
 where
     T: Clone + Into<Bson>,
@@ -294,32 +300,7 @@ impl From<DbPointer> for Bson {
     }
 }
 
-impl From<Value> for Bson {
-    fn from(a: Value) -> Bson {
-        match a {
-            Value::Number(x) => x
-                .as_i64()
-                .map(|i| {
-                    if i >= i32::MIN as i64 && i <= i32::MAX as i64 {
-                        Bson::Int32(i as i32)
-                    } else {
-                        Bson::Int64(i)
-                    }
-                })
-                .or_else(|| x.as_u64().map(Bson::from))
-                .or_else(|| x.as_f64().map(Bson::from))
-                .unwrap_or_else(|| panic!("Invalid number value: {}", x)),
-            Value::String(x) => x.into(),
-            Value::Bool(x) => x.into(),
-            Value::Array(x) => Bson::Array(x.into_iter().map(Bson::from).collect()),
-            Value::Object(x) => Bson::from_extended_document(
-                x.into_iter().map(|(k, v)| (k, Bson::from(v))).collect(),
-            ),
-            Value::Null => Bson::Null,
-        }
-    }
-}
-
+/// This will create the [relaxed Extended JSON v2](https://docs.mongodb.com/manual/reference/mongodb-extended-json/) representation of the provided [`Bson`](../enum.Bson.html).
 impl From<Bson> for Value {
     fn from(bson: Bson) -> Self {
         bson.into_relaxed_extjson()
@@ -754,31 +735,7 @@ impl Bson {
 
             ["$date"] => {
                 if let Ok(date) = doc.get_i64("$date") {
-                    let mut num_secs = date / 1000;
-                    let mut num_millis = date % 1000;
-
-                    // The chrono API only lets us create a DateTime with an i64 number of seconds
-                    // and a u32 number of nanoseconds. In the case of a negative timestamp, this
-                    // means that we need to turn the negative fractional part into a positive and
-                    // shift the number of seconds down. For example:
-                    //
-                    //     date       = -4300 ms
-                    //     num_secs   = date / 1000 = -4300 / 1000 = -4
-                    //     num_millis = date % 1000 = -4300 % 1000 = -300
-                    //
-                    // Since num_millis is less than 0:
-                    //     num_secs   = num_secs -1 = -4 - 1 = -5
-                    //     num_millis = num_nanos + 1000 = -300 + 1000 = 700
-                    //
-                    // Instead of -4 seconds and -300 milliseconds, we now have -5 seconds and +700
-                    // milliseconds, which expresses the same timestamp, but in a way we can create
-                    // a DateTime with.
-                    if num_millis < 0 {
-                        num_secs -= 1;
-                        num_millis += 1000;
-                    };
-
-                    return Bson::DateTime(Utc.timestamp(num_secs, num_millis as u32 * 1_000_000));
+                    return Bson::DateTime(DateTime::from_i64(date).into());
                 }
 
                 if let Ok(date) = doc.get_str("$date") {
@@ -1017,7 +974,7 @@ impl Timestamp {
 ///
 /// Just a helper for convenience
 ///
-/// ```rust,ignore
+/// ```rust
 /// use serde::{Serialize, Deserialize};
 /// use bson::DateTime;
 ///
@@ -1029,6 +986,37 @@ impl Timestamp {
 #[derive(Debug, Eq, PartialEq, Ord, PartialOrd, Hash, Copy, Clone)]
 pub struct DateTime(pub chrono::DateTime<Utc>);
 
+impl DateTime {
+    pub(crate) fn from_i64(date: i64) -> Self {
+        let mut num_secs = date / 1000;
+        let mut num_millis = date % 1000;
+
+        // The chrono API only lets us create a DateTime with an i64 number of seconds
+        // and a u32 number of nanoseconds. In the case of a negative timestamp, this
+        // means that we need to turn the negative fractional part into a positive and
+        // shift the number of seconds down. For example:
+        //
+        //     date       = -4300 ms
+        //     num_secs   = date / 1000 = -4300 / 1000 = -4
+        //     num_millis = date % 1000 = -4300 % 1000 = -300
+        //
+        // Since num_millis is less than 0:
+        //     num_secs   = num_secs -1 = -4 - 1 = -5
+        //     num_millis = num_nanos + 1000 = -300 + 1000 = 700
+        //
+        // Instead of -4 seconds and -300 milliseconds, we now have -5 seconds and +700
+        // milliseconds, which expresses the same timestamp, but in a way we can create
+        // a DateTime with.
+        if num_millis < 0 {
+            num_secs -= 1;
+            num_millis += 1000;
+        };
+
+        Utc.timestamp(num_secs, num_millis as u32 * 1_000_000)
+            .into()
+    }
+}
+
 impl Deref for DateTime {
     type Target = chrono::DateTime<Utc>;
 

src/de/error.rs

@@ -1,8 +1,8 @@
 use std::{error, fmt, fmt::Display, io, string};
 
+use serde::de::{self, Unexpected};
+
 use crate::Bson;
-use de::Unexpected;
-use serde::de;
 
 /// Possible errors that can arise during decoding.
 #[derive(Debug)]

src/de/mod.rs

@@ -44,11 +44,41 @@ use crate::{
     Decimal128,
 };
 
-use ::serde::de::{self, Error as _};
+use ::serde::{
+    de::{Error as _, Unexpected},
+    Deserialize,
+};
 
 const MAX_BSON_SIZE: i32 = 16 * 1024 * 1024;
+pub(crate) const MIN_BSON_DOCUMENT_SIZE: i32 = 4 + 1; // 4 bytes for length, one byte for null terminator
+const MIN_BSON_STRING_SIZE: i32 = 4 + 1; // 4 bytes for length, one byte for null terminator
+const MIN_CODE_WITH_SCOPE_SIZE: i32 = 4 + MIN_BSON_STRING_SIZE + MIN_BSON_DOCUMENT_SIZE;
+
+/// Run the provided closure, ensuring that over the course of its execution, exactly `length` bytes
+/// were read from the reader.
+pub(crate) fn ensure_read_exactly<F, R>(
+    reader: &mut R,
+    length: usize,
+    error_message: &str,
+    func: F,
+) -> Result<()>
+where
+    F: FnOnce(&mut std::io::Cursor<Vec<u8>>) -> Result<()>,
+    R: Read + ?Sized,
+{
+    let mut buf = vec![0u8; length];
+    reader.read_exact(&mut buf)?;
+    let mut cursor = std::io::Cursor::new(buf);
+
+    func(&mut cursor)?;
+
+    if cursor.position() != length as u64 {
+        return Err(Error::invalid_length(length, &error_message));
+    }
+    Ok(())
+}
 
-fn read_string<R: Read + ?Sized>(reader: &mut R, utf8_lossy: bool) -> crate::de::Result<String> {
+fn read_string<R: Read + ?Sized>(reader: &mut R, utf8_lossy: bool) -> Result<String> {
     let len = reader.read_i32::<LittleEndian>()?;
 
     // UTF-8 String must have at least 1 byte (the last 0x00).
@@ -68,12 +98,19 @@ fn read_string<R: Read + ?Sized>(reader: &mut R, utf8_lossy: bool) -> crate::de:
         reader.take(len as u64 - 1).read_to_string(&mut s)?;
         s
     };
-    reader.read_u8()?; // The last 0x00
+
+    // read the null terminator
+    if reader.read_u8()? != 0 {
+        return Err(Error::invalid_length(
+            len as usize,
+            &"contents of string longer than provided length",
+        ));
+    }
 
     Ok(s)
 }
 
-fn read_cstring<R: Read + ?Sized>(reader: &mut R) -> crate::de::Result<String> {
+fn read_cstring<R: Read + ?Sized>(reader: &mut R) -> Result<String> {
     let mut v = Vec::new();
 
     loop {
@@ -88,28 +125,28 @@ fn read_cstring<R: Read + ?Sized>(reader: &mut R) -> crate::de::Result<String> {
 }
 
 #[inline]
-pub(crate) fn read_i32<R: Read + ?Sized>(reader: &mut R) -> crate::de::Result<i32> {
+pub(crate) fn read_i32<R: Read + ?Sized>(reader: &mut R) -> Result<i32> {
     reader.read_i32::<LittleEndian>().map_err(From::from)
 }
 
 #[inline]
-fn read_i64<R: Read + ?Sized>(reader: &mut R) -> crate::de::Result<i64> {
+fn read_i64<R: Read + ?Sized>(reader: &mut R) -> Result<i64> {
     reader.read_i64::<LittleEndian>().map_err(From::from)
 }
 
 /// Placeholder decoder for `Decimal128`. Reads 128 bits and just stores them, does no validation or
 /// parsing.
 #[cfg(not(feature = "decimal128"))]
 #[inline]
-fn read_f128<R: Read + ?Sized>(reader: &mut R) -> crate::de::Result<Decimal128> {
+fn read_f128<R: Read + ?Sized>(reader: &mut R) -> Result<Decimal128> {
     let mut buf = [0u8; 128 / 8];
     reader.read_exact(&mut buf)?;
     Ok(Decimal128 { bytes: buf })
 }
 
 #[cfg(feature = "decimal128")]
 #[inline]
-fn read_f128<R: Read + ?Sized>(reader: &mut R) -> crate::de::Result<Decimal128> {
+fn read_f128<R: Read + ?Sized>(reader: &mut R) -> Result<Decimal128> {
     use std::mem;
 
     let mut local_buf: [u8; 16] = unsafe { mem::MaybeUninit::uninit().assume_init() };
@@ -118,24 +155,27 @@ fn read_f128<R: Read + ?Sized>(reader: &mut R) -> crate::de::Result<Decimal128>
     Ok(val)
 }
 
-fn deserialize_array<R: Read + ?Sized>(
-    reader: &mut R,
-    utf8_lossy: bool,
-) -> crate::de::Result<Array> {
+fn deserialize_array<R: Read + ?Sized>(reader: &mut R, utf8_lossy: bool) -> Result<Array> {
     let mut arr = Array::new();
-
-    // disregard the length: using Read::take causes infinite type recursion
-    read_i32(reader)?;
-
-    loop {
-        let tag = reader.read_u8()?;
-        if tag == 0 {
-            break;
-        }
-
-        let (_, val) = deserialize_bson_kvp(reader, tag, utf8_lossy)?;
-        arr.push(val)
-    }
+    let length = read_i32(reader)?;
+
+    ensure_read_exactly(
+        reader,
+        (length as usize) - 4,
+        "array length longer than contents",
+        |cursor| {
+            loop {
+                let tag = cursor.read_u8()?;
+                if tag == 0 {
+                    break;
+                }
+
+                let (_, val) = deserialize_bson_kvp(cursor, tag, utf8_lossy)?;
+                arr.push(val)
+            }
+            Ok(())
+        },
+    )?;
 
     Ok(arr)
 }
@@ -144,7 +184,7 @@ pub(crate) fn deserialize_bson_kvp<R: Read + ?Sized>(
     reader: &mut R,
     tag: u8,
     utf8_lossy: bool,
-) -> crate::de::Result<(String, Bson)> {
+) -> Result<(String, Bson)> {
     use spec::ElementType;
     let key = read_cstring(reader)?;
 
@@ -165,7 +205,15 @@ pub(crate) fn deserialize_bson_kvp<R: Read + ?Sized>(
 
             // Skip length data in old binary.
             if let BinarySubtype::BinaryOld = subtype {
-                read_i32(reader)?;
+                let data_len = read_i32(reader)?;
+
+                if data_len + 4 != len {
+                    return Err(Error::invalid_length(
+                        data_len as usize,
+                        &"0x02 length did not match top level binary length",
+                    ));
+                }
+
                 len -= 4;
             }
 
@@ -181,7 +229,17 @@ pub(crate) fn deserialize_bson_kvp<R: Read + ?Sized>(
             }
             Bson::ObjectId(oid::ObjectId::with_bytes(objid))
         }
-        Some(ElementType::Boolean) => Bson::Boolean(reader.read_u8()? != 0),
+        Some(ElementType::Boolean) => {
+            let val = reader.read_u8()?;
+            if val > 1 {
+                return Err(Error::invalid_value(
+                    Unexpected::Unsigned(val as u64),
+                    &"boolean must be stored as 0 or 1",
+                ));
+            }
+
+            Bson::Boolean(val != 0)
+        }
         Some(ElementType::Null) => Bson::Null,
         Some(ElementType::RegularExpression) => {
             let pattern = read_cstring(reader)?;
@@ -198,12 +256,29 @@ pub(crate) fn deserialize_bson_kvp<R: Read + ?Sized>(
             read_string(reader, utf8_lossy).map(Bson::JavaScriptCode)?
         }
         Some(ElementType::JavaScriptCodeWithScope) => {
-            // disregard the length:
-            //     using Read::take causes infinite type recursion
-            read_i32(reader)?;
+            let length = read_i32(reader)?;
+            if length < MIN_CODE_WITH_SCOPE_SIZE {
+                return Err(Error::invalid_length(
+                    length as usize,
+                    &format!(
+                        "code with scope length must be at least {}",
+                        MIN_CODE_WITH_SCOPE_SIZE
+                    )
+                    .as_str(),
+                ));
+            } else if length > MAX_BSON_SIZE {
+                return Err(Error::invalid_length(
+                    length as usize,
+                    &"code with scope length too large",
+                ));
+            }
+
+            let mut buf = vec![0u8; (length - 4) as usize];
+            reader.read_exact(&mut buf)?;
 
-            let code = read_string(reader, utf8_lossy)?;
-            let scope = Document::from_reader(reader)?;
+            let mut slice = buf.as_slice();
+            let code = read_string(&mut slice, utf8_lossy)?;
+            let scope = Document::from_reader(&mut slice)?;
             Bson::JavaScriptCodeWithScope(JavaScriptCodeWithScope { code, scope })
         }
         Some(ElementType::Int32) => read_i32(reader).map(Bson::Int32)?,
@@ -256,10 +331,10 @@ pub(crate) fn deserialize_bson_kvp<R: Read + ?Sized>(
 }
 
 /// Decode a BSON `Value` into a `T` Deserializable.
-pub fn from_bson<'de, T>(bson: Bson) -> crate::de::Result<T>
+pub fn from_bson<'de, T>(bson: Bson) -> Result<T>
 where
-    T: de::Deserialize<'de>,
+    T: Deserialize<'de>,
 {
     let de = Deserializer::new(bson);
-    de::Deserialize::deserialize(de)
+    Deserialize::deserialize(de)
 }