Properly use xsi:nil to deserialize null values via serde

This commit fixes an issue that causes `quick-xml` trying to deserialize
empty tags via the serde interface even if these tags were explicitly
marked as `xsi:nil="true"`

For example the following XML failed to deserialize before this commit:

```xml
<bar>
	<foo xsi:nil="true"/>
</bar>
```

into the following rust type:

```rust
#[derive(Deserialize)]
struct Bar {
    foo: Option<Foo>,
}

#[derive(Deserialize)]
struct Foo {
    baz: String,
}
```

Before this commit this failed to deserialize with an error message that
complained that the `baz` field was missing. After this commit this uses
the `xsi:nil` attribute to deserialize this into `foo: None` instead.
The standard (https://www.w3.org/TR/xmlschema-1/#xsi_nil) seems to
support this behaviour.

Fix #497

Author: weiznich

Changelog.md

@@ -17,11 +17,13 @@
 
 - [#850]: Add `Attribute::as_bool()` method to get an attribute value as a boolean.
 - [#850]: Add `Attributes::has_nil()` method to check if attributes has `xsi:nil` attribute set to `true`.
+- [#497]: Handle `xsi:nil` attribute in serde Deserializer to better process optional fields.
 
 ### Bug Fixes
 
 ### Misc Changes
 
+[#497]: https://github.com/tafia/quick-xml/issues/497
 [#850]: https://github.com/tafia/quick-xml/pull/850
 
 

src/de/map.rs

@@ -215,6 +215,21 @@ where
             has_value_field: fields.contains(&VALUE_KEY),
         })
     }
+
+    /// Determines if subtree started with the specified event shoould be skipped.
+    ///
+    /// Used to map elements with `xsi:nil` attribute set to true to `None` in optional contexts.
+    ///
+    /// We need to handle two attributes:
+    /// - on parent element: <map xsi:nil="true"><foo/></map>
+    /// - on this element:   <map><foo xsi:nil="true"/></map>
+    ///
+    /// We check parent element too because `xsi:nil` affects only nested elements of the
+    /// tag where it is defined. We can map structure with fields mapped to attributes to
+    /// the `<map>` element and set to `None` all its optional elements.
+    fn should_skip_subtree(&self, start: &BytesStart) -> bool {
+        self.de.reader.reader.has_nil_attr(&self.start) || self.de.reader.reader.has_nil_attr(start)
+    }
 }
 
 impl<'de, 'd, R, E> MapAccess<'de> for ElementMapAccess<'de, 'd, R, E>
@@ -540,8 +555,14 @@ where
     where
         V: Visitor<'de>,
     {
-        match self.map.de.peek()? {
+        // We cannot use result of `peek()` directly because of borrow checker
+        let _ = self.map.de.peek()?;
+        match self.map.de.last_peeked() {
             DeEvent::Text(t) if t.is_empty() => visitor.visit_none(),
+            DeEvent::Start(start) if self.map.should_skip_subtree(start) => {
+                self.map.de.skip_next_tree()?;
+                visitor.visit_none()
+            }
             _ => visitor.visit_some(self),
         }
     }

src/de/mod.rs

@@ -19,6 +19,7 @@
 //!   - [Optional attributes and elements](#optional-attributes-and-elements)
 //!   - [Choices (`xs:choice` XML Schema type)](#choices-xschoice-xml-schema-type)
 //!   - [Sequences (`xs:all` and `xs:sequence` XML Schema types)](#sequences-xsall-and-xssequence-xml-schema-types)
+//! - [Mapping of `xsi:nil`](#mapping-of-xsinil)
 //! - [Generate Rust types from XML](#generate-rust-types-from-xml)
 //! - [Composition Rules](#composition-rules)
 //! - [Enum Representations](#enum-representations)
@@ -413,6 +414,13 @@
 //! <any-tag optional=""/>   <!-- Some("") -->
 //! <any-tag/>               <!-- None -->
 //! ```
+//! <div style="background:rgba(120,145,255,0.45);padding:0.75em;">
+//!
+//! NOTE: The behaviour is not symmetric by default. `None` will be serialized as
+//! `optional=""`. This behaviour is consistent across serde crates. You should add
+//! `#[serde(skip_serializing_if = "Option::is_none")]` attribute to the field to
+//! skip `None`s.
+//! </div>
 //! </td>
 //! </tr>
 //! <!-- 7 ===================================================================================== -->
@@ -454,9 +462,15 @@
 //! When the XML element is present, type `T` will be deserialized from an
 //! element (which is a string or a multi-mapping -- i.e. mapping which can have
 //! duplicated keys).
-//! <div style="background:rgba(80, 240, 100, 0.20);padding:0.75em;">
+//! <div style="background:rgba(120,145,255,0.45);padding:0.75em;">
 //!
-//! Currently some edge cases exists described in the issue [#497].
+//! NOTE: The behaviour is not symmetric by default. `None` will be serialized as
+//! `<optional/>`. This behaviour is consistent across serde crates. You should add
+//! `#[serde(skip_serializing_if = "Option::is_none")]` attribute to the field to
+//! skip `None`s.
+//!
+//! NOTE: Deserializer will automatically handle a [`xsi:nil`] attribute and set field to `None`.
+//! For more info see [Mapping of `xsi:nil`](#mapping-of-xsinil).
 //! </div>
 //! </td>
 //! </tr>
@@ -1312,6 +1326,65 @@
 //! </table>
 //!
 //!
+//! Mapping of `xsi:nil`
+//! ====================
+//!
+//! quick-xml supports handling of [`xsi:nil`] special attribute. When field of optional
+//! type is mapped to the XML element which have `xsi:nil="true"` set, or if that attribute
+//! is placed on parent XML element, the deserializer will call [`Visitor::visit_none`]
+//! and skip XML element corresponding to a field.
+//!
+//! Examples:
+//!
+//! ```
+//! # use pretty_assertions::assert_eq;
+//! # use serde::Deserialize;
+//! #[derive(Deserialize, Debug, PartialEq)]
+//! struct TypeWithOptionalField {
+//!   element: Option<String>,
+//! }
+//!
+//! assert_eq!(
+//!   TypeWithOptionalField {
+//!     element: None,
+//!   },
+//!   quick_xml::de::from_str("
+//!     <any-tag xmlns:xsi='http://www.w3.org/2001/XMLSchema-instance'>
+//!       <element xsi:nil='true'>Content is skiped because of xsi:nil='true'</element>
+//!     </any-tag>
+//!   ").unwrap(),
+//! );
+//! ```
+//!
+//! You can capture attributes from the optional type, because ` xsi:nil="true"` elements can have
+//! attributes:
+//! ```
+//! # use pretty_assertions::assert_eq;
+//! # use serde::Deserialize;
+//! #[derive(Deserialize, Debug, PartialEq)]
+//! struct TypeWithOptionalField {
+//!   #[serde(rename = "@attribute")]
+//!   attribute: usize,
+//!
+//!   element: Option<String>,
+//!   non_optional: String,
+//! }
+//!
+//! assert_eq!(
+//!   TypeWithOptionalField {
+//!     attribute: 42,
+//!     element: None,
+//!     non_optional: "Note, that non-optional fields will be deserialized as usual".to_string(),
+//!   },
+//!   quick_xml::de::from_str("
+//!     <any-tag attribute='42' xsi:nil='true' xmlns:xsi='http://www.w3.org/2001/XMLSchema-instance'>
+//!       <element>Content is skiped because of xsi:nil='true'</element>
+//!       <non_optional>Note, that non-optional fields will be deserialized as usual</non_optional>
+//!     </any-tag>
+//!   ").unwrap(),
+//! );
+//! ```
+//!
 //! Generate Rust types from XML
 //! ============================
 //!
@@ -1820,7 +1893,7 @@
 //! [`overlapped-lists`]: ../index.html#overlapped-lists
 //! [specification]: https://www.w3.org/TR/xmlschema11-1/#Simple_Type_Definition
 //! [`deserialize_with`]: https://serde.rs/field-attrs.html#deserialize_with
-//! [#497]: https://github.com/tafia/quick-xml/issues/497
+//! [`xsi:nil`]: https://www.w3.org/TR/xmlschema-1/#xsi_nil
 //! [`Serializer::serialize_unit_variant`]: serde::Serializer::serialize_unit_variant
 //! [`Deserializer::deserialize_enum`]: serde::Deserializer::deserialize_enum
 //! [`SeError::Unsupported`]: crate::errors::serialize::SeError::Unsupported
@@ -2534,6 +2607,22 @@ where
         }
     }
 
+    #[inline]
+    fn last_peeked(&self) -> &DeEvent<'de> {
+        #[cfg(feature = "overlapped-lists")]
+        {
+            self.read
+                .front()
+                .expect("`Deserializer::peek()` should be called")
+        }
+        #[cfg(not(feature = "overlapped-lists"))]
+        {
+            self.peek
+                .as_ref()
+                .expect("`Deserializer::peek()` should be called")
+        }
+    }
+
     fn next(&mut self) -> Result<DeEvent<'de>, DeError> {
         // Replay skipped or peeked events
         #[cfg(feature = "overlapped-lists")]
@@ -2764,6 +2853,14 @@ where
         }
         self.reader.read_to_end(name)
     }
+
+    fn skip_next_tree(&mut self) -> Result<(), DeError> {
+        let DeEvent::Start(start) = self.next()? else {
+            unreachable!("Only call this if the next event is a start event")
+        };
+        let name = start.name();
+        self.read_to_end(name)
+    }
 }
 
 impl<'de> Deserializer<'de, SliceReader<'de>> {
@@ -2945,9 +3042,16 @@ where
     where
         V: Visitor<'de>,
     {
-        match self.peek()? {
+        // We cannot use result of `peek()` directly because of borrow checker
+        let _ = self.peek()?;
+        match self.last_peeked() {
             DeEvent::Text(t) if t.is_empty() => visitor.visit_none(),
             DeEvent::Eof => visitor.visit_none(),
+            // if the `xsi:nil` attribute is set to true we got a none value
+            DeEvent::Start(start) if self.reader.reader.has_nil_attr(&start) => {
+                self.skip_next_tree()?;
+                visitor.visit_none()
+            }
             _ => visitor.visit_some(self),
         }
     }
@@ -3071,6 +3175,12 @@ pub trait XmlRead<'i> {
 
     /// A copy of the reader's decoder used to decode strings.
     fn decoder(&self) -> Decoder;
+
+    /// Checks if the `start` tag has a [`xsi:nil`] attribute. This method ignores
+    /// any errors in attributes.
+    ///
+    /// [`xsi:nil`]: https://www.w3.org/TR/xmlschema-1/#xsi_nil
+    fn has_nil_attr(&self, start: &BytesStart) -> bool;
 }
 
 /// XML input source that reads from a std::io input stream.
@@ -3140,6 +3250,10 @@ impl<'i, R: BufRead> XmlRead<'i> for IoReader<R> {
     fn decoder(&self) -> Decoder {
         self.reader.decoder()
     }
+
+    fn has_nil_attr(&self, start: &BytesStart) -> bool {
+        start.attributes().has_nil(&self.reader)
+    }
 }
 
 /// XML input source that reads from a slice of bytes and can borrow from it.
@@ -3205,6 +3319,10 @@ impl<'de> XmlRead<'de> for SliceReader<'de> {
     fn decoder(&self) -> Decoder {
         self.reader.decoder()
     }
+
+    fn has_nil_attr(&self, start: &BytesStart) -> bool {
+        start.attributes().has_nil(&self.reader)
+    }
 }
 
 #[cfg(test)]