Repository: tafia/quick-xml (rust)

Issue:
quick-xml v0.27+ changed behaviour when writing empty values
I'm using quick-xml for my XML-RPC crate ([dxr](https://github.com/ironthree/dxr)) and I've been a happy user so far, thank you for maintaining the crate!

I'm trying to update my quick-xml dependency to a newer version (dxr is currently stuck at v0.25), but I'm hitting a roadblock. It looks like the new serde serializer that was implemented for v0.27 changed some behaviour when writing empty values.

For example, it now writes self-closing XML tags - previously, I got `<value><string></string></value>` for an empty string value, but with quick-xml v0.27+, I get `<value><string/></value>`. While this is technically valid XML, my library cannot write self-closing tags like this in order to maintain compatibility with other XML-RPC implementations (some older ones don't accept self-closing XML tags).

Is there a way to get back the old behaviour (i.e. deserialize an empty `$value` to `<foo></foo>` instead of `<foo/>`?

Pull request:
Add expand_empty_elements fn to Serializer
Setting this to true will serialize empty elements to `<element></element>` instead of `<element/>`.

Fixes #617

Patch:
diff --git a/Changelog.md b/Changelog.md
index 33e5e96..aff3757 100644
--- a/Changelog.md
+++ b/Changelog.md
@@ -15,14 +15,19 @@
 - [#609]: Added `Writer::write_serializable` to provide the capability to serialize
   arbitrary types using serde when using the lower-level `Writer` API.
 - [#615]: Added ability to set entity resolver when deserialize using borrowing reader.
+- [#617]: Added ability to enforce the expansion of empty elements.
 
 ### Bug Fixes
 
+- [#604]: Avoid crashing on wrong comments like `<!-->` when using `read_event_into*` functions.
+
 ### Misc Changes
 
 
+[#604]: https://github.com/tafia/quick-xml/issue/604
 [#609]: https://github.com/tafia/quick-xml/pull/609
 [#615]: https://github.com/tafia/quick-xml/pull/615
+[#617]: https://github.com/tafia/quick-xml/pull/617
 
 
 ## 0.29.0 -- 2023-06-13
diff --git a/src/reader/buffered_reader.rs b/src/reader/buffered_reader.rs
index ee58aed..c3cec06 100644
--- a/src/reader/buffered_reader.rs
+++ b/src/reader/buffered_reader.rs
@@ -117,7 +117,9 @@ macro_rules! impl_buffered_source {
                     // somewhere sane rather than at the EOF
                     Ok(n) if n.is_empty() => return Err(bang_type.to_err()),
                     Ok(available) => {
-                        if let Some((consumed, used)) = bang_type.parse(buf, available) {
+                        // We only parse from start because we don't want to consider
+                        // whatever is in the buffer before the bang element
+                        if let Some((consumed, used)) = bang_type.parse(&buf[start..], available) {
                             buf.extend_from_slice(consumed);
 
                             self $(.$reader)? .consume(used);
diff --git a/src/se/content.rs b/src/se/content.rs
index 24c29ac..bd5b456 100644
--- a/src/se/content.rs
+++ b/src/se/content.rs
@@ -56,6 +56,9 @@ pub struct ContentSerializer<'w, 'i, W: Write> {
     /// If `true`, then current indent will be written before writing the content,
     /// but only if content is not empty.
     pub write_indent: bool,
+    // If `true`, then empty elements will be serialized as `<element></element>`
+    // instead of `<element/>`.
+    pub expand_empty_elements: bool,
     //TODO: add settings to disallow consequent serialization of primitives
 }
 
@@ -85,6 +88,7 @@ impl<'w, 'i, W: Write> ContentSerializer<'w, 'i, W> {
             level: self.level,
             indent: self.indent.borrow(),
             write_indent: self.write_indent,
+            expand_empty_elements: false,
         }
     }
 
@@ -92,9 +96,17 @@ impl<'w, 'i, W: Write> ContentSerializer<'w, 'i, W> {
     #[inline]
     pub(super) fn write_empty(mut self, name: XmlName) -> Result<(), DeError> {
         self.write_indent()?;
-        self.writer.write_char('<')?;
-        self.writer.write_str(name.0)?;
-        self.writer.write_str("/>")?;
+        if self.expand_empty_elements {
+            self.writer.write_char('<')?;
+            self.writer.write_str(name.0)?;
+            self.writer.write_str("></")?;
+            self.writer.write_str(name.0)?;
+            self.writer.write_char('>')?;
+        } else {
+            self.writer.write_str("<")?;
+            self.writer.write_str(name.0)?;
+            self.writer.write_str("/>")?;
+        }
         Ok(())
     }
 
@@ -483,6 +495,7 @@ pub(super) mod tests {
                         level: QuoteLevel::Full,
                         indent: Indent::None,
                         write_indent: false,
+                        expand_empty_elements: false,
                     };
 
                     $data.serialize(ser).unwrap();
@@ -503,6 +516,7 @@ pub(super) mod tests {
                         level: QuoteLevel::Full,
                         indent: Indent::None,
                         write_indent: false,
+                        expand_empty_elements: false,
                     };
 
                     match $data.serialize(ser).unwrap_err() {
@@ -672,6 +686,7 @@ pub(super) mod tests {
                         level: QuoteLevel::Full,
                         indent: Indent::Owned(Indentation::new(b' ', 2)),
                         write_indent: false,
+                        expand_empty_elements: false,
                     };
 
                     $data.serialize(ser).unwrap();
@@ -692,6 +707,7 @@ pub(super) mod tests {
                         level: QuoteLevel::Full,
                         indent: Indent::Owned(Indentation::new(b' ', 2)),
                         write_indent: false,
+                        expand_empty_elements: false,
                     };
 
                     match $data.serialize(ser).unwrap_err() {
diff --git a/src/se/element.rs b/src/se/element.rs
index 19a9657..4f076df 100644
--- a/src/se/element.rs
+++ b/src/se/element.rs
@@ -397,6 +397,7 @@ impl<'w, 'k, W: Write> Struct<'w, 'k, W> {
             level: self.ser.ser.level,
             indent: self.ser.ser.indent.borrow(),
             write_indent: true,
+            expand_empty_elements: false,
         };
 
         if key == TEXT_KEY {
@@ -574,6 +575,7 @@ mod tests {
                             level: QuoteLevel::Full,
                             indent: Indent::None,
                             write_indent: false,
+                            expand_empty_elements: false,
                         },
                         key: XmlName("root"),
                     };
@@ -597,6 +599,7 @@ mod tests {
                             level: QuoteLevel::Full,
                             indent: Indent::None,
                             write_indent: false,
+                            expand_empty_elements: false,
                         },
                         key: XmlName("root"),
                     };
@@ -1533,6 +1536,7 @@ mod tests {
                             level: QuoteLevel::Full,
                             indent: Indent::Owned(Indentation::new(b' ', 2)),
                             write_indent: false,
+                            expand_empty_elements: false,
                         },
                         key: XmlName("root"),
                     };
@@ -1556,6 +1560,7 @@ mod tests {
                             level: QuoteLevel::Full,
                             indent: Indent::Owned(Indentation::new(b' ', 2)),
                             write_indent: false,
+                            expand_empty_elements: false,
                         },
                         key: XmlName("root"),
                     };
@@ -2533,4 +2538,76 @@ mod tests {
                     </root>");
         }
     }
+
+    mod expand_empty_elements {
+        use super::*;
+        use pretty_assertions::assert_eq;
+
+        /// Checks that given `$data` successfully serialized as `$expected`
+        macro_rules! serialize_as {
+            ($name:ident: $data:expr => $expected:expr) => {
+                #[test]
+                fn $name() {
+                    let mut buffer = String::new();
+                    let ser = ElementSerializer {
+                        ser: ContentSerializer {
+                            writer: &mut buffer,
+                            level: QuoteLevel::Full,
+                            indent: Indent::None,
+                            write_indent: false,
+                            expand_empty_elements: true,
+                        },
+                        key: XmlName("root"),
+                    };
+
+                    $data.serialize(ser).unwrap();
+                    assert_eq!(buffer, $expected);
+                }
+            };
+        }
+
+        /// Checks that attempt to serialize given `$data` results to a
+        /// serialization error `$kind` with `$reason`
+        macro_rules! err {
+            ($name:ident: $data:expr => $kind:ident($reason:literal)) => {
+                #[test]
+                fn $name() {
+                    let mut buffer = String::new();
+                    let ser = ElementSerializer {
+                        ser: ContentSerializer {
+                            writer: &mut buffer,
+                            level: QuoteLevel::Full,
+                            indent: Indent::None,
+                            write_indent: false,
+                            expand_empty_elements: false,
+                        },
+                        key: XmlName("root"),
+                    };
+
+                    match $data.serialize(ser).unwrap_err() {
+                        DeError::$kind(e) => assert_eq!(e, $reason),
+                        e => panic!(
+                            "Expected `{}({})`, found `{:?}`",
+                            stringify!($kind),
+                            $reason,
+                            e
+                        ),
+                    }
+                    // We can write something before fail
+                    // assert_eq!(buffer, "");
+                }
+            };
+        }
+
+        serialize_as!(option_some_empty: Some("") => "<root></root>");
+        serialize_as!(option_some_empty_str: Some("") => "<root></root>");
+
+        serialize_as!(unit: () => "<root></root>");
+        serialize_as!(unit_struct: Unit => "<root></root>");
+        serialize_as!(unit_struct_escaped: UnitEscaped => "<root></root>");
+
+        serialize_as!(enum_unit: Enum::Unit => "<Unit></Unit>");
+        err!(enum_unit_escaped: Enum::UnitEscaped
+            => Unsupported("character `<` is not allowed at the start of an XML name `<\"&'>`"));
+    }
 }
diff --git a/src/se/mod.rs b/src/se/mod.rs
index cce81fd..06b8004 100644
--- a/src/se/mod.rs
+++ b/src/se/mod.rs
@@ -458,6 +458,7 @@ impl<'w, 'r, W: Write> Serializer<'w, 'r, W> {
                 level: QuoteLevel::Full,
                 indent: Indent::None,
                 write_indent: false,
+                expand_empty_elements: false,
             },
             root_tag: None,
         }
@@ -522,11 +523,45 @@ impl<'w, 'r, W: Write> Serializer<'w, 'r, W> {
                 level: QuoteLevel::Full,
                 indent: Indent::None,
                 write_indent: false,
+                expand_empty_elements: false,
             },
             root_tag: root_tag.map(|tag| XmlName::try_from(tag)).transpose()?,
         })
     }
 
+    /// Enable or disable expansion of empty elements. Defaults to `false`.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// # use pretty_assertions::assert_eq;
+    /// # use serde::Serialize;
+    /// # use quick_xml::se::Serializer;
+    ///
+    /// #[derive(Debug, PartialEq, Serialize)]
+    /// struct Struct {
+    ///     question: Option<String>,
+    /// }
+    ///
+    /// let mut buffer = String::new();
+    /// let mut ser = Serializer::new(&mut buffer);
+    /// ser.expand_empty_elements(true);
+    ///
+    /// let data = Struct {
+    ///   question: None,
+    /// };
+    ///
+    /// data.serialize(ser).unwrap();
+    /// assert_eq!(
+    ///     buffer,
+    ///     "<Struct><question></question></Struct>"
+    /// );
+    /// ```
+    pub fn expand_empty_elements(&mut self, expand: bool) -> &mut Self {
+        self.ser.expand_empty_elements = expand;
+        self
+    }
+
     /// Configure indent for a serializer
     pub fn indent(&mut self, indent_char: char, indent_size: usize) -> &mut Self {
         self.ser.indent = Indent::Owned(Indentation::new(indent_char as u8, indent_size));


Test patch:
diff --git a/tests/issues.rs b/tests/issues.rs
index a19355b..90efc73 100644
--- a/tests/issues.rs
+++ b/tests/issues.rs
@@ -4,7 +4,7 @@
 
 use std::sync::mpsc;
 
-use quick_xml::events::{BytesStart, Event};
+use quick_xml::events::{BytesDecl, BytesStart, BytesText, Event};
 use quick_xml::name::QName;
 use quick_xml::reader::Reader;
 use quick_xml::Error;
@@ -98,10 +98,90 @@ mod issue514 {
                 assert_eq!(found, "other-tag");
             }
             x => panic!(
-                r#"Expected `Err(EndEventMismatch("some-tag", "other-tag")))`, but found {:?}"#,
+                r#"Expected `Err(EndEventMismatch("some-tag", "other-tag"))`, but found {:?}"#,
                 x
             ),
         }
         assert_eq!(reader.read_event().unwrap(), Event::Eof);
     }
 }
+
+/// Regression test for https://github.com/tafia/quick-xml/issues/604
+mod issue604 {
+    use super::*;
+    use pretty_assertions::assert_eq;
+
+    #[test]
+    fn short() {
+        let data = b"<?xml version=\"1.0\"?><!-->";
+        let mut reader = Reader::from_reader(data.as_slice());
+        let mut buf = Vec::new();
+        assert_eq!(
+            reader.read_event_into(&mut buf).unwrap(),
+            Event::Decl(BytesDecl::new("1.0", None, None))
+        );
+        match reader.read_event_into(&mut buf) {
+            Err(Error::UnexpectedEof(reason)) => assert_eq!(reason, "Comment"),
+            x => panic!(
+                r#"Expected `Err(UnexpectedEof("Comment"))`, but found {:?}"#,
+                x
+            ),
+        }
+        assert_eq!(reader.read_event_into(&mut buf).unwrap(), Event::Eof);
+    }
+
+    #[test]
+    fn long() {
+        let data = b"<?xml version=\"1.0\"?><!--->";
+        let mut reader = Reader::from_reader(data.as_slice());
+        let mut buf = Vec::new();
+        assert_eq!(
+            reader.read_event_into(&mut buf).unwrap(),
+            Event::Decl(BytesDecl::new("1.0", None, None))
+        );
+        match reader.read_event_into(&mut buf) {
+            Err(Error::UnexpectedEof(reason)) => assert_eq!(reason, "Comment"),
+            x => panic!(
+                r#"Expected `Err(UnexpectedEof("Comment"))`, but found {:?}"#,
+                x
+            ),
+        }
+        assert_eq!(reader.read_event_into(&mut buf).unwrap(), Event::Eof);
+    }
+
+    /// According to the grammar, `>` is allowed just in start of comment.
+    /// See https://www.w3.org/TR/xml11/#sec-comments
+    #[test]
+    fn short_valid() {
+        let data = b"<?xml version=\"1.0\"?><!-->-->";
+        let mut reader = Reader::from_reader(data.as_slice());
+        let mut buf = Vec::new();
+        assert_eq!(
+            reader.read_event_into(&mut buf).unwrap(),
+            Event::Decl(BytesDecl::new("1.0", None, None))
+        );
+        assert_eq!(
+            reader.read_event_into(&mut buf).unwrap(),
+            Event::Comment(BytesText::from_escaped(">"))
+        );
+        assert_eq!(reader.read_event_into(&mut buf).unwrap(), Event::Eof);
+    }
+
+    /// According to the grammar, `->` is allowed just in start of comment.
+    /// See https://www.w3.org/TR/xml11/#sec-comments
+    #[test]
+    fn long_valid() {
+        let data = b"<?xml version=\"1.0\"?><!--->-->";
+        let mut reader = Reader::from_reader(data.as_slice());
+        let mut buf = Vec::new();
+        assert_eq!(
+            reader.read_event_into(&mut buf).unwrap(),
+            Event::Decl(BytesDecl::new("1.0", None, None))
+        );
+        assert_eq!(
+            reader.read_event_into(&mut buf).unwrap(),
+            Event::Comment(BytesText::from_escaped("->"))
+        );
+        assert_eq!(reader.read_event_into(&mut buf).unwrap(), Event::Eof);
+    }
+}