seq: Move has_value_field into MapAccess, because it is not used outside it

This fixes a data race with nested maps in sequence fields.
Also, top-level sequences always have `has_value_field == false`,
so no need to check it

Co-authored-by: Daniel Alley <[email protected]>

Author: Mingun

Changelog.md

@@ -15,6 +15,9 @@
 - [#9]: Deserialization erroneously was successful in some cases where error is expected.
   This broke deserialization of untagged enums which rely on error if variant cannot be parsed
 - [#387]: Allow to have an ordinary elements together with a `$value` field
+- [#387]: Internal deserializer state can be broken when deserializing a map with
+  a sequence field (such as `Vec<T>`), where elements of this sequence contains
+  another sequence. This error affects only users with the `serialize` feature enabled
 
 ### Misc Changes
 

src/de/map.rs

@@ -173,6 +173,14 @@ where
     source: ValueSource,
     /// List of field names of the struct. It is empty for maps
     fields: &'static [&'static str],
+    /// If `true`, then the deserialized struct has a field with a special name:
+    /// [`INNER_VALUE`]. That field should be deserialized from the text content
+    /// of an XML node:
+    ///
+    /// ```xml
+    /// <tag>value for INNER_VALUE field<tag>
+    /// ```
+    has_value_field: bool,
     /// list of fields yet to unflatten (defined as starting with $unflatten=)
     unflatten_fields: Vec<&'static [u8]>,
 }
@@ -193,6 +201,7 @@ where
             iter: IterState::new(0, false),
             source: ValueSource::Unknown,
             fields,
+            has_value_field: fields.contains(&INNER_VALUE),
             unflatten_fields: fields
                 .iter()
                 .filter(|f| f.starts_with(UNFLATTEN_PREFIX))
@@ -217,7 +226,6 @@ where
         // FIXME: There error positions counted from end of tag name - need global position
         let slice = self.start.attributes_raw();
         let decoder = self.de.reader.decoder();
-        let has_value_field = self.de.has_value_field;
 
         if let Some(a) = self.iter.next(slice).transpose()? {
             // try getting map from attributes (key= "value")
@@ -255,7 +263,7 @@ where
                 // }
                 // TODO: This should be handled by #[serde(flatten)]
                 // See https://github.com/serde-rs/serde/issues/1905
-                DeEvent::Start(e) if has_value_field && not_in(self.fields, e, decoder)? => {
+                DeEvent::Start(e) if self.has_value_field && not_in(self.fields, e, decoder)? => {
                     self.source = ValueSource::Content;
                     seed.deserialize(INNER_VALUE.into_deserializer()).map(Some)
                 }

src/de/mod.rs

@@ -255,14 +255,6 @@ where
 {
     reader: R,
     peek: Option<DeEvent<'de>>,
-    /// Special sing that deserialized struct have a field with the special
-    /// name (see constant `INNER_VALUE`). That field should be deserialized
-    /// from the text content of the XML node:
-    ///
-    /// ```xml
-    /// <tag>value for INNER_VALUE field<tag>
-    /// ```
-    has_value_field: bool,
 }
 
 /// Deserialize an instance of type `T` from a string of XML text.
@@ -354,7 +346,6 @@ where
         Deserializer {
             reader,
             peek: None,
-            has_value_field: false,
         }
     }
 
@@ -544,10 +535,8 @@ where
         // Try to go to the next `<tag ...>...</tag>` or `<tag .../>`
         if let Some(e) = self.next_start()? {
             let name = e.name().to_vec();
-            self.has_value_field = fields.contains(&INNER_VALUE);
             let map = map::MapAccess::new(self, e, fields)?;
             let value = visitor.visit_map(map)?;
-            self.has_value_field = false;
             self.read_to_end(&name)?;
             Ok(value)
         } else {

src/de/seq.rs

@@ -92,15 +92,11 @@ where
 {
     /// Creates a new accessor to a top-level sequence of XML elements.
     pub fn new(de: &'a mut Deserializer<'de, R>) -> Result<Self, DeError> {
-        let filter = if de.has_value_field {
-            TagFilter::Exclude(&[])
+        let filter = if let DeEvent::Start(e) = de.peek()? {
+            // Clone is cheap if event borrows from the input
+            TagFilter::Include(e.clone())
         } else {
-            if let DeEvent::Start(e) = de.peek()? {
-                // Clone is cheap if event borrows from the input
-                TagFilter::Include(e.clone())
-            } else {
-                TagFilter::Exclude(&[])
-            }
+            TagFilter::Exclude(&[])
         };
         Ok(Self { de, filter })
     }