Repository: tafia/quick-xml (rust)

Issue:
Strip whitespaces at the end of closing markups.
Spaces after the end of the markup name in a closing tag are valid XML (See: https://www.w3.org/TR/REC-xml/#sec-starttags ). Yet the pares considers them part of the markup name and so checking for matching start and close may fail.

Pull request:
strip whitespaces at the end of closing tags
fixes #144. `read_end` now also always returns the markup name without trailing whitespaces.

Patch:
diff --git a/src/errors.rs b/src/errors.rs
index a6c3fc1..ce7ae8a 100644
--- a/src/errors.rs
+++ b/src/errors.rs
@@ -52,9 +52,7 @@ pub enum Error {
 
     #[fail(
         display = "error while parsing attribute at position {}: Duplicate attribute at position {} and {}",
-        _0,
-        _1,
-        _0
+        _0, _1, _0
     )]
     DuplicatedAttribute(usize, usize),
 
diff --git a/src/escape.rs b/src/escape.rs
index 965f47b..8694e26 100644
--- a/src/escape.rs
+++ b/src/escape.rs
@@ -14,8 +14,7 @@ pub enum EscapeError {
 
     #[fail(
         display = "Error while escaping character at range {:?}: Unrecognized escape symbol: {:?}",
-        _0,
-        _1
+        _0, _1
     )]
     UnrecognizedSymbol(
         ::std::ops::Range<usize>,
diff --git a/src/reader.rs b/src/reader.rs
index e59dd46..3076bb4 100644
--- a/src/reader.rs
+++ b/src/reader.rs
@@ -71,6 +71,8 @@ pub struct Reader<B: BufRead> {
     expand_empty_elements: bool,
     /// trims Text events, skip the element if text is empty
     trim_text: bool,
+    /// trims trailing whitespaces from markup names in closing tags `</a >`
+    trim_markup_names_in_closing_tags: bool,
     /// check if End nodes match last Start node
     check_end_names: bool,
     /// check if comments contains `--` (false per default)
@@ -96,6 +98,7 @@ impl<B: BufRead> Reader<B> {
             tag_state: TagState::Closed,
             expand_empty_elements: false,
             trim_text: false,
+            trim_markup_names_in_closing_tags: true,
             check_end_names: true,
             buf_position: 0,
             check_comments: false,
@@ -133,6 +136,22 @@ impl<B: BufRead> Reader<B> {
         self
     }
 
+    /// Changes wether trailing whitespaces after the markup name are trimmed in closing tags
+    /// `</a >`.
+    ///
+    /// If true the emitted [`End`] event is stripped of trailing whitespace after the markup name.
+    ///
+    /// Note that if set to `false` and `check_end_names` is true the comparison of markup names is
+    /// going to fail erronously if a closing tag contains trailing whitespaces.
+    ///
+    /// (`true` by default)
+    ///
+    /// [`End`]: events/enum.Event.html#variant.End
+    pub fn trim_markup_names_in_closing_tags(&mut self, val: bool) -> &mut Reader<B> {
+        self.trim_markup_names_in_closing_tags = val;
+        self
+    }
+
     /// Changes whether mismatched closing tag names should be detected.
     ///
     /// When set to `false`, it won't check if a closing tag matches the corresponding opening tag.
@@ -273,26 +292,41 @@ impl<B: BufRead> Reader<B> {
     /// return `End` event
     fn read_end<'a, 'b>(&'a mut self, buf: &'b [u8]) -> Result<Event<'b>> {
         let len = buf.len();
+        // XML standard permits whitespaces after the markup name in closing tags.
+        // Let's strip them from the buffer before comparing tag names.
+        let name = if self.trim_markup_names_in_closing_tags {
+            if let Some(pos_end_name) = buf[1..].iter().rposition(|&b| !b.is_ascii_whitespace()) {
+                let (name, _) = buf[1..].split_at(pos_end_name + 1);
+                name
+            } else {
+                &buf[1..]
+            }
+        } else {
+            &buf[1..]
+        };
         if self.check_end_names {
-            let mismatch_err = |expected: &[u8], buf_position: &mut usize| {
+            let mismatch_err = |expected: &[u8], found: &[u8], buf_position: &mut usize| {
                 *buf_position -= len;
                 Err(Error::EndEventMismatch {
                     expected: from_utf8(expected).unwrap_or("").to_owned(),
-                    found: from_utf8(&buf[1..]).unwrap_or("").to_owned(),
+                    found: from_utf8(found).unwrap_or("").to_owned(),
                 })
             };
             match self.opened_starts.pop() {
                 Some(start) => {
-                    if buf[1..] != self.opened_buffer[start..] {
+                    if name != &self.opened_buffer[start..] {
                         let expected = &self.opened_buffer[start..];
-                        return mismatch_err(expected, &mut self.buf_position);
+                        mismatch_err(expected, name, &mut self.buf_position)
+                    } else {
+                        self.opened_buffer.truncate(start);
+                        Ok(Event::End(BytesEnd::borrowed(name)))
                     }
-                    self.opened_buffer.truncate(start);
                 }
-                None => return mismatch_err(b"", &mut self.buf_position),
+                None => mismatch_err(b"", &buf[1..], &mut self.buf_position),
             }
+        } else {
+            Ok(Event::End(BytesEnd::borrowed(name)))
         }
-        Ok(Event::End(BytesEnd::borrowed(&buf[1..])))
     }
 
     /// reads `BytesElement` starting with a `!`,
diff --git a/src/writer.rs b/src/writer.rs
index d8c225b..3c37dab 100644
--- a/src/writer.rs
+++ b/src/writer.rs
@@ -135,10 +135,11 @@ impl<W: Write> Writer<W> {
         let mut wrote = 0;
         if let Some(ref i) = self.indent {
             if i.should_line_break {
-                wrote = self.writer.write(b"\n").map_err(Error::Io)? + self
-                    .writer
-                    .write(&i.indents[..i.indents_len])
-                    .map_err(Error::Io)?;
+                wrote = self.writer.write(b"\n").map_err(Error::Io)?
+                    + self
+                        .writer
+                        .write(&i.indents[..i.indents_len])
+                        .map_err(Error::Io)?;
             }
         }
         Ok(wrote + self.write(before)? + self.write(value)? + self.write(after)?)


Test patch:
diff --git a/tests/test.rs b/tests/test.rs
index a8dae4e..845382c 100644
--- a/tests/test.rs
+++ b/tests/test.rs
@@ -113,14 +113,15 @@ fn test_attributes_empty_ns() {
         e => panic!("Expecting Empty event, got {:?}", e),
     };
 
-    let mut atts = e.attributes()
-            .map(|ar| ar.expect("Expecting attribute parsing to succeed."))
-            // we don't care about xmlns attributes for this test
-            .filter(|kv| !kv.key.starts_with(b"xmlns"))
-            .map(|Attribute { key: name, value }| {
-                let (opt_ns, local_name) = r.attribute_namespace(name, &ns_buf);
-                (opt_ns, local_name, value)
-            });
+    let mut atts = e
+        .attributes()
+        .map(|ar| ar.expect("Expecting attribute parsing to succeed."))
+        // we don't care about xmlns attributes for this test
+        .filter(|kv| !kv.key.starts_with(b"xmlns"))
+        .map(|Attribute { key: name, value }| {
+            let (opt_ns, local_name) = r.attribute_namespace(name, &ns_buf);
+            (opt_ns, local_name, value)
+        });
     match atts.next() {
         Some((None, b"att1", Cow::Borrowed(b"a"))) => (),
         e => panic!("Expecting att1='a' attribute, found {:?}", e),
@@ -157,7 +158,8 @@ fn test_attributes_empty_ns_expanded() {
             e => panic!("Expecting Empty event, got {:?}", e),
         };
 
-        let mut atts = e.attributes()
+        let mut atts = e
+            .attributes()
             .map(|ar| ar.expect("Expecting attribute parsing to succeed."))
             // we don't care about xmlns attributes for this test
             .filter(|kv| !kv.key.starts_with(b"xmlns"))
@@ -221,7 +223,8 @@ fn test_default_ns_shadowing_empty() {
             e => panic!("Expecting Empty event, got {:?}", e),
         };
 
-        let mut atts = e.attributes()
+        let mut atts = e
+            .attributes()
             .map(|ar| ar.expect("Expecting attribute parsing to succeed."))
             // we don't care about xmlns attributes for this test
             .filter(|kv| !kv.key.starts_with(b"xmlns"))
@@ -282,7 +285,8 @@ fn test_default_ns_shadowing_expanded() {
             }
             e => panic!("Expecting Start event (<inner>), got {:?}", e),
         };
-        let mut atts = e.attributes()
+        let mut atts = e
+            .attributes()
             .map(|ar| ar.expect("Expecting attribute parsing to succeed."))
             // we don't care about xmlns attributes for this test
             .filter(|kv| !kv.key.starts_with(b"xmlns"))
diff --git a/tests/unit_tests.rs b/tests/unit_tests.rs
index ad25498..85c744f 100644
--- a/tests/unit_tests.rs
+++ b/tests/unit_tests.rs
@@ -66,6 +66,13 @@ fn test_start_end() {
     next_eq!(r, Start, b"a", End, b"a");
 }
 
+#[test]
+fn test_start_end_with_ws() {
+    let mut r = Reader::from_str("<a></a >");
+    r.trim_text(true);
+    next_eq!(r, Start, b"a", End, b"a");
+}
+
 #[test]
 fn test_start_end_attr() {
     let mut r = Reader::from_str("<a b=\"test\"></a>");
@@ -533,12 +540,14 @@ fn test_escaped_content() {
                 );
             }
             match e.unescaped() {
-                Ok(ref c) => if &**c != b"<test>" {
-                    panic!(
-                        "unescaped content unexpected: expecting '&lt;test&lt;', got '{:?}'",
-                        from_utf8(c)
-                    )
-                },
+                Ok(ref c) => {
+                    if &**c != b"<test>" {
+                        panic!(
+                            "unescaped content unexpected: expecting '&lt;test&lt;', got '{:?}'",
+                            from_utf8(c)
+                        )
+                    }
+                }
                 Err(e) => panic!(
                     "cannot escape content at position {}: {:?}",
                     r.buffer_position(),
@@ -630,11 +639,9 @@ fn test_read_write_roundtrip_escape() {
             Ok(Eof) => break,
             Ok(Text(e)) => {
                 let t = e.escaped();
-                assert!(
-                    writer
-                        .write_event(Event::Text(BytesText::from_escaped(t.to_vec())))
-                        .is_ok()
-                );
+                assert!(writer
+                    .write_event(Event::Text(BytesText::from_escaped(t.to_vec())))
+                    .is_ok());
             }
             Ok(e) => assert!(writer.write_event(e).is_ok()),
             Err(e) => panic!(e),
@@ -665,11 +672,9 @@ fn test_read_write_roundtrip_escape_text() {
             Ok(Eof) => break,
             Ok(Text(e)) => {
                 let t = e.unescape_and_decode(&reader).unwrap();
-                assert!(
-                    writer
-                        .write_event(Event::Text(BytesText::from_plain_str(&t)))
-                        .is_ok()
-                );
+                assert!(writer
+                    .write_event(Event::Text(BytesText::from_plain_str(&t)))
+                    .is_ok());
             }
             Ok(e) => assert!(writer.write_event(e).is_ok()),
             Err(e) => panic!(e),
diff --git a/tests/xmlrs_reader_tests.rs b/tests/xmlrs_reader_tests.rs
index 435473d..cfd85a4 100644
--- a/tests/xmlrs_reader_tests.rs
+++ b/tests/xmlrs_reader_tests.rs
@@ -344,13 +344,15 @@ fn make_attrs(e: &BytesStart) -> ::std::result::Result<String, String> {
     let mut atts = Vec::new();
     for a in e.attributes() {
         match a {
-            Ok(a) => if a.key.len() < 5 || !a.key.starts_with(b"xmlns") {
-                atts.push(format!(
-                    "{}=\"{}\"",
-                    from_utf8(a.key).unwrap(),
-                    from_utf8(&*a.unescaped_value().unwrap()).unwrap()
-                ));
-            },
+            Ok(a) => {
+                if a.key.len() < 5 || !a.key.starts_with(b"xmlns") {
+                    atts.push(format!(
+                        "{}=\"{}\"",
+                        from_utf8(a.key).unwrap(),
+                        from_utf8(&*a.unescaped_value().unwrap()).unwrap()
+                    ));
+                }
+            }
             Err(e) => return Err(e.to_string()),
         }
     }
@@ -408,7 +410,8 @@ impl<'a> Iterator for SpecIter<'a> {
                 b' ' | b'\r' | b'\n' | b'\t' | b'|' | b':' => false,
                 b'0'...b'9' => false,
                 _ => true,
-            }).unwrap_or(0);
+            })
+            .unwrap_or(0);
         if let Some(p) = self.0.windows(2).position(|w| w == b")\n") {
             let (prev, next) = self.0.split_at(p + 1);
             self.0 = next;