minidom: Remove comments support. Forbid them as per XMPP RFC.

Maxime “pep” Buquet created

Signed-off-by: Maxime “pep” Buquet <pep@bouah.net>

Change summary

minidom-rs/Cargo.toml     |  4 ----
minidom-rs/src/element.rs | 32 ++------------------------------
minidom-rs/src/error.rs   | 11 ++++-------
minidom-rs/src/tests.rs   | 40 +++++++---------------------------------
xmpp-parsers/src/xhtml.rs |  2 --
5 files changed, 13 insertions(+), 76 deletions(-)

Detailed changes

minidom-rs/Cargo.toml 🔗

@@ -22,7 +22,3 @@ gitlab = { repository = "xmpp-rs/xmpp-rs" }
 
 [dependencies]
 quick-xml = "0.18"
-
-[features]
-default = ["comments"]
-comments = []

minidom-rs/src/element.rs 🔗

@@ -335,12 +335,9 @@ impl Element {
                 Event::Eof => {
                     return Err(Error::EndOfDocument);
                 }
-                #[cfg(not(feature = "comments"))]
                 Event::Comment { .. } => {
-                    return Err(Error::CommentsDisabled);
+                    return Err(Error::NoComments);
                 }
-                #[cfg(feature = "comments")]
-                Event::Comment { .. } => (),
                 Event::Text { .. }
                 | Event::End { .. }
                 | Event::CData { .. }
@@ -418,16 +415,7 @@ impl Element {
                 Event::Eof => {
                     break;
                 }
-                #[cfg(not(feature = "comments"))]
-                Event::Comment(_) => return Err(Error::CommentsDisabled),
-                #[cfg(feature = "comments")]
-                Event::Comment(s) => {
-                    let comment = reader.decode(&s)?.to_owned();
-                    if comment != "" {
-                        let current_elem = stack.last_mut().unwrap();
-                        current_elem.append_comment_node(comment);
-                    }
-                }
+                Event::Comment(_) => return Err(Error::NoComments),
                 Event::Decl { .. } | Event::PI { .. } | Event::DocType { .. } => (),
             }
         }
@@ -632,22 +620,6 @@ impl Element {
         self.children.push(Node::Text(child.into()));
     }
 
-    /// Appends a comment node to an `Element`.
-    ///
-    /// # Examples
-    ///
-    /// ```rust
-    /// use minidom::Element;
-    ///
-    /// let mut elem = Element::bare("node");
-    ///
-    /// elem.append_comment_node("comment");
-    /// ```
-    #[cfg(feature = "comments")]
-    pub fn append_comment_node<S: Into<String>>(&mut self, child: S) {
-        self.children.push(Node::Comment(child.into()));
-    }
-
     /// Appends a node to an `Element`.
     ///
     /// # Examples

minidom-rs/src/error.rs 🔗

@@ -36,8 +36,7 @@ pub enum Error {
     InvalidElement,
 
     /// An error which is returned when a comment is to be parsed by minidom
-    #[cfg(not(comments))]
-    CommentsDisabled,
+    NoComments,
 }
 
 impl StdError for Error {
@@ -49,8 +48,7 @@ impl StdError for Error {
             Error::EndOfDocument => None,
             Error::InvalidElementClosed => None,
             Error::InvalidElement => None,
-            #[cfg(not(comments))]
-            Error::CommentsDisabled => None,
+            Error::NoComments => None,
         }
     }
 }
@@ -68,10 +66,9 @@ impl std::fmt::Display for Error {
                 write!(fmt, "the XML is invalid, an element was wrongly closed")
             }
             Error::InvalidElement => write!(fmt, "the XML element is invalid"),
-            #[cfg(not(comments))]
-            Error::CommentsDisabled => write!(
+            Error::NoComments => write!(
                 fmt,
-                "a comment has been found even though comments are disabled by feature"
+                "a comment has been found even though comments are forbidden"
             ),
         }
     }

minidom-rs/src/tests.rs 🔗

@@ -11,6 +11,7 @@
 // file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 use crate::element::Element;
+use crate::error::Error;
 
 use quick_xml::Reader;
 
@@ -35,21 +36,6 @@ fn build_test_tree() -> Element {
     root
 }
 
-#[cfg(feature = "comments")]
-const COMMENT_TEST_STRING: &'static str = r#"<root><!--This is a child.--><child attr="val"><!--This is a grandchild.--><grandchild/></child></root>"#;
-
-#[cfg(feature = "comments")]
-fn build_comment_test_tree() -> Element {
-    let mut root = Element::builder("root").build();
-    root.append_comment_node("This is a child.");
-    let mut child = Element::builder("child").attr("attr", "val").build();
-    child.append_comment_node("This is a grandchild.");
-    let grand_child = Element::builder("grandchild").build();
-    child.append_child(grand_child);
-    root.append_child(child);
-    root
-}
-
 #[test]
 fn reader_works() {
     let mut reader = Reader::from_str(TEST_STRING);
@@ -265,25 +251,13 @@ fn namespace_inherited_prefixed2() {
     assert_eq!(child.ns(), Some("jabber:client".to_owned()));
 }
 
-#[cfg(feature = "comments")]
 #[test]
-fn read_comments() {
-    let mut reader = Reader::from_str(COMMENT_TEST_STRING);
-    assert_eq!(
-        Element::from_reader(&mut reader).unwrap(),
-        build_comment_test_tree()
-    );
-}
-
-#[cfg(feature = "comments")]
-#[test]
-fn write_comments() {
-    let root = build_comment_test_tree();
-    let mut writer = Vec::new();
-    {
-        root.write_to(&mut writer).unwrap();
-    }
-    assert_eq!(String::from_utf8(writer).unwrap(), COMMENT_TEST_STRING);
+fn fail_comments() {
+    let elem: Result<Element, Error> = "<foo><!-- bar --></foo>".parse();
+    match elem {
+        Err(Error::NoComments) => (),
+        _ => panic!(),
+    };
 }
 
 #[test]

xmpp-parsers/src/xhtml.rs 🔗

@@ -160,7 +160,6 @@ impl TryFrom<Element> for Body {
             match child {
                 Node::Element(child) => children.push(Child::Tag(Tag::try_from(child.clone())?)),
                 Node::Text(text) => children.push(Child::Text(text.clone())),
-                Node::Comment(_) => unimplemented!(), // XXX: remove!
             }
         }
 
@@ -309,7 +308,6 @@ impl TryFrom<Element> for Tag {
             match child {
                 Node::Element(child) => children.push(Child::Tag(Tag::try_from(child.clone())?)),
                 Node::Text(text) => children.push(Child::Text(text.clone())),
-                Node::Comment(_) => unimplemented!(), // XXX: remove!
             }
         }