Repository: tafia/quick-xml (rust)

Issue:
Panic when parsing invalid comments `<!-->`
On the following input quick XML 0.28-2 panics with ` 'slice index starts at 3 but ends at 1', quick-xml-0.28.2/src/reader/parser.rs:104:22`

```xml
<?xml version="1.0" encoding="utf-8"?>
<sparql xmlns="http://www.w3.org/2005/sparql-resu:lts#">
<head>
        <variable name="str"/>
</head>
<results>
        <result></result> <!-->
        <result></resulhtmlns>=
```

I believe this error is caused by [this line](https://github.com/tafia/quick-xml/blob/84b07b4e5c9a201b3d3d7fa95c1e8c33ecbe6dd2/src/reader/mod.rs#L828) where the `buf.len() + i > 4` check is wrong.

Pull request:
Fails on empty comments without 4 -"
The parser was crashing because of bad slice bounds

Closes #604

Patch:
diff --git a/Changelog.md b/Changelog.md
index 33e5e96..b25b1c5 100644
--- a/Changelog.md
+++ b/Changelog.md
@@ -18,9 +18,12 @@
 
 ### 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
 
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);


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);
+    }
+}