Merge pull request #889 from dimforge/dvector_deserialize

Fix potential unsoundness after deserializing a DVector with a bad number or elements.

src/base/vec_storage.rs

@@ -13,6 +13,12 @@ use crate::base::storage::{
 };
 use crate::base::{Scalar, Vector};
 
+#[cfg(feature = "serde-serialize-no-std")]
+use serde::{
+    de::{Deserialize, Deserializer, Error},
+    ser::{Serialize, Serializer},
+};
+
 #[cfg(feature = "abomonation-serialize")]
 use abomonation::Abomonation;
 
@@ -24,13 +30,54 @@ use abomonation::Abomonation;
 /// A Vec-based matrix data storage. It may be dynamically-sized.
 #[repr(C)]
 #[derive(Eq, Debug, Clone, PartialEq)]
-#[cfg_attr(feature = "serde-serialize", derive(Serialize, Deserialize))]
 pub struct VecStorage<T, R: Dim, C: Dim> {
     data: Vec<T>,
     nrows: R,
     ncols: C,
 }
 
+#[cfg(feature = "serde-serialize")]
+impl<T, R: Dim, C: Dim> Serialize for VecStorage<T, R, C>
+where
+    T: Serialize,
+    R: Serialize,
+    C: Serialize,
+{
+    fn serialize<Ser>(&self, serializer: Ser) -> Result<Ser::Ok, Ser::Error>
+    where
+        Ser: Serializer,
+    {
+        (&self.data, &self.nrows, &self.ncols).serialize(serializer)
+    }
+}
+
+#[cfg(feature = "serde-serialize")]
+impl<'a, T, R: Dim, C: Dim> Deserialize<'a> for VecStorage<T, R, C>
+where
+    T: Deserialize<'a>,
+    R: Deserialize<'a>,
+    C: Deserialize<'a>,
+{
+    fn deserialize<Des>(deserializer: Des) -> Result<Self, Des::Error>
+    where
+        Des: Deserializer<'a>,
+    {
+        let (data, nrows, ncols): (Vec<T>, R, C) = Deserialize::deserialize(deserializer)?;
+
+        // SAFETY: make sure the data we deserialize have the
+        //         correct number of elements.
+        if nrows.value() * ncols.value() != data.len() {
+            return Err(Des::Error::custom(format!(
+                "Expected {} components, found {}",
+                nrows.value() * ncols.value(),
+                data.len()
+            )));
+        }
+
+        Ok(Self { data, nrows, ncols })
+    }
+}
+
 #[deprecated(note = "renamed to `VecStorage`")]
 /// Renamed to [VecStorage].
 pub type MatrixVec<T, R, C> = VecStorage<T, R, C>;

tests/core/serde.rs

@@ -1,8 +1,8 @@
 #![cfg(feature = "serde-serialize")]
 
 use na::{
-    DMatrix, Isometry2, Isometry3, IsometryMatrix2, IsometryMatrix3, Matrix3x4, Point2, Point3,
+    DMatrix, Isometry2, Isometry3, IsometryMatrix2, IsometryMatrix3, Matrix2x3, Matrix3x4, Point2,
-    Quaternion, Rotation2, Rotation3, Similarity2, Similarity3, SimilarityMatrix2,
+    Point3, Quaternion, Rotation2, Rotation3, Similarity2, Similarity3, SimilarityMatrix2,
     SimilarityMatrix3, Translation2, Translation3, Unit, Vector2,
 };
 use rand;
@@ -27,6 +27,32 @@ fn serde_dmatrix() {
     let serialized = serde_json::to_string(&v).unwrap();
     let deserialized: DMatrix<f32> = serde_json::from_str(&serialized).unwrap();
     assert_eq!(v, deserialized);
+
+    let m = DMatrix::from_column_slice(2, 3, &[1.0, 2.0, 3.0, 4.0, 5.0, 6.0]);
+    let mat_str = "[[1.0, 2.0, 3.0, 4.0, 5.0, 6.0],2,3]";
+    let deserialized: DMatrix<f32> = serde_json::from_str(&mat_str).unwrap();
+    assert_eq!(m, deserialized);
+
+    let m = Matrix2x3::from_column_slice(&[1.0, 2.0, 3.0, 4.0, 5.0, 6.0]);
+    let mat_str = "[1.0, 2.0, 3.0, 4.0, 5.0, 6.0]";
+    let deserialized: Matrix2x3<f32> = serde_json::from_str(&mat_str).unwrap();
+    assert_eq!(m, deserialized);
+}
+
+#[test]
+#[should_panic]
+fn serde_dmatrix_invalid_len() {
+    // This must fail: we attempt to deserialize a 2x3 with only 5 elements.
+    let mat_str = "[[1.0, 2.0, 3.0, 4.0, 5.0],2,3]";
+    let _: DMatrix<f32> = serde_json::from_str(&mat_str).unwrap();
+}
+
+#[test]
+#[should_panic]
+fn serde_smatrix_invalid_len() {
+    // This must fail: we attempt to deserialize a 2x3 with only 5 elements.
+    let mat_str = "[1.0, 2.0, 3.0, 4.0, 5.0]";
+    let _: Matrix2x3<f32> = serde_json::from_str(&mat_str).unwrap();
 }
 
 test_serde!(