Fix ron-rs#277 and ron-rs#405 with Value::Map IntoIter and extraneous item check for Value::Seq (ron-rs#406)

Author: juntyr

CHANGELOG.md

@@ -6,6 +6,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## Unreleased
 
+- Fix issues [#277](https://github.com/ron-rs/ron/issues/277) and [#405](https://github.com/ron-rs/ron/issues/405) with `Value::Map` `IntoIter` and extraneous item check for `Value::Seq` ([#406](https://github.com/ron-rs/ron/pull/406))
+
 ## [0.8.0] - 2022-08-17
 - Add `integer128` feature that guards `i128` and `u128` ([#304](https://github.com/ron-rs/ron/pull/304), [#351](https://github.com/ron-rs/ron/pull/351))
 - Fix issue [#265](https://github.com/ron-rs/ron/issues/265) with better missing comma error ([#353](https://github.com/ron-rs/ron/pull/353))

src/value.rs

@@ -83,6 +83,16 @@ impl FromIterator<(Value, Value)> for Map {
     }
 }
 
+impl IntoIterator for Map {
+    type Item = (Value, Value);
+
+    type IntoIter = <MapInner as IntoIterator>::IntoIter;
+
+    fn into_iter(self) -> Self::IntoIter {
+        self.0.into_iter()
+    }
+}
+
 /// Note: equality is only given if both values and order of values match
 impl Eq for Map {}
 
@@ -353,18 +363,45 @@ impl<'de> Deserializer<'de> for Value {
         match self {
             Value::Bool(b) => visitor.visit_bool(b),
             Value::Char(c) => visitor.visit_char(c),
-            Value::Map(m) => visitor.visit_map(MapAccessor {
-                keys: m.keys().cloned().rev().collect(),
-                values: m.values().cloned().rev().collect(),
-            }),
+            Value::Map(m) => {
+                let old_len = m.len();
+
+                let mut items: Vec<(Value, Value)> = m.into_iter().collect();
+                items.reverse();
+
+                let value = visitor.visit_map(MapAccessor {
+                    items: &mut items,
+                    value: None,
+                })?;
+
+                if items.is_empty() {
+                    Ok(value)
+                } else {
+                    Err(Error::ExpectedDifferentLength {
+                        expected: format!("a map of length {}", old_len - items.len()),
+                        found: old_len,
+                    })
+                }
+            }
             Value::Number(Number::Float(ref f)) => visitor.visit_f64(f.get()),
             Value::Number(Number::Integer(i)) => visitor.visit_i64(i),
             Value::Option(Some(o)) => visitor.visit_some(*o),
             Value::Option(None) => visitor.visit_none(),
             Value::String(s) => visitor.visit_string(s),
             Value::Seq(mut seq) => {
+                let old_len = seq.len();
+
                 seq.reverse();
-                visitor.visit_seq(Seq { seq })
+                let value = visitor.visit_seq(Seq { seq: &mut seq })?;
+
+                if seq.is_empty() {
+                    Ok(value)
+                } else {
+                    Err(Error::ExpectedDifferentLength {
+                        expected: format!("a sequence of length {}", old_len - seq.len()),
+                        found: old_len,
+                    })
+                }
             }
             Value::Unit => visitor.visit_unit(),
         }
@@ -433,41 +470,48 @@ impl<'de> Deserializer<'de> for Value {
     }
 }
 
-struct MapAccessor {
-    keys: Vec<Value>,
-    values: Vec<Value>,
+struct MapAccessor<'a> {
+    items: &'a mut Vec<(Value, Value)>,
+    value: Option<Value>,
 }
 
-impl<'de> MapAccess<'de> for MapAccessor {
+impl<'a, 'de> MapAccess<'de> for MapAccessor<'a> {
     type Error = Error;
 
     fn next_key_seed<K>(&mut self, seed: K) -> Result<Option<K::Value>>
     where
         K: DeserializeSeed<'de>,
     {
         // The `Vec` is reversed, so we can pop to get the originally first element
-        self.keys
-            .pop()
-            .map_or(Ok(None), |v| seed.deserialize(v).map(Some))
+        match self.items.pop() {
+            Some((key, value)) => {
+                self.value = Some(value);
+                seed.deserialize(key).map(Some)
+            }
+            None => Ok(None),
+        }
     }
 
     fn next_value_seed<V>(&mut self, seed: V) -> Result<V::Value>
     where
         V: DeserializeSeed<'de>,
     {
-        // The `Vec` is reversed, so we can pop to get the originally first element
-        self.values
-            .pop()
-            .map(|v| seed.deserialize(v))
-            .expect("Contract violation")
+        match self.value.take() {
+            Some(value) => seed.deserialize(value),
+            None => panic!("Contract violation: value before key"),
+        }
+    }
+
+    fn size_hint(&self) -> Option<usize> {
+        Some(self.items.len())
     }
 }
 
-struct Seq {
-    seq: Vec<Value>,
+struct Seq<'a> {
+    seq: &'a mut Vec<Value>,
 }
 
-impl<'de> SeqAccess<'de> for Seq {
+impl<'a, 'de> SeqAccess<'de> for Seq<'a> {
     type Error = Error;
 
     fn next_element_seed<T>(&mut self, seed: T) -> Result<Option<T::Value>>
@@ -479,6 +523,10 @@ impl<'de> SeqAccess<'de> for Seq {
             .pop()
             .map_or(Ok(None), |v| seed.deserialize(v).map(Some))
     }
+
+    fn size_hint(&self) -> Option<usize> {
+        Some(self.seq.len())
+    }
 }
 
 #[cfg(test)]

tests/value.rs

@@ -1,6 +1,9 @@
 use std::f64;
 
-use ron::value::{Map, Number, Value};
+use ron::{
+    error::Error,
+    value::{Map, Number, Value},
+};
 use serde::Serialize;
 
 #[test]
@@ -68,6 +71,34 @@ fn seq() {
         Value::Number(Number::new(2f64)),
     ];
     assert_eq!("[1, 2.0]".parse(), Ok(Value::Seq(seq)));
+
+    let err = Value::Seq(vec![Value::Number(Number::new(1))])
+        .into_rust::<[i32; 2]>()
+        .unwrap_err();
+
+    assert_eq!(
+        err,
+        Error::ExpectedDifferentLength {
+            expected: String::from("an array of length 2"),
+            found: 1,
+        }
+    );
+
+    let err = Value::Seq(vec![
+        Value::Number(Number::new(1)),
+        Value::Number(Number::new(2)),
+        Value::Number(Number::new(3)),
+    ])
+    .into_rust::<[i32; 2]>()
+    .unwrap_err();
+
+    assert_eq!(
+        err,
+        Error::ExpectedDifferentLength {
+            expected: String::from("a sequence of length 2"),
+            found: 3,
+        }
+    );
 }
 
 #[test]