Repository: tafia/quick-xml
Pull request #91 (2017-09-14 10:28:16)

Escape attributes


Patch:
diff --git a/Cargo.toml b/Cargo.toml
index 962bcfe..4b11889 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "quick-xml"
-version = "0.9.4"
+version = "0.9.3"
 authors = ["Johann Tuffe <tafia973@gmail.com>"]
 description = "High performance xml reader and writer"
 
diff --git a/Changelog.md b/Changelog.md
index e864c26..7581a35 100644
--- a/Changelog.md
+++ b/Changelog.md
@@ -8,9 +8,6 @@
   - test: Adding missing tests
   - chore: Changes to the build process or auxiliary tools/libraries/documentation
 
-## 0.9.4
-- fix: bound tests in `read_bang` fn.
-
 ## 0.9.3
 - fix: escape was panicking at the 3rd character escaped.
 
diff --git a/README.md b/README.md
index 24cea56..a4314f3 100644
--- a/README.md
+++ b/README.md
@@ -18,7 +18,7 @@ Syntax is inspired by [xml-rs](https://github.com/netvl/xml-rs).
 
 ```toml
 [dependencies]
-quick-xml = "0.9.4"
+quick-xml = "0.9.3"
 ```
 ``` rust
 extern crate quick_xml;
diff --git a/src/events/attributes.rs b/src/events/attributes.rs
index f2930d5..2bfe840 100644
--- a/src/events/attributes.rs
+++ b/src/events/attributes.rs
@@ -5,7 +5,7 @@ use std::borrow::Cow;
 use std::ops::Range;
 use std::io::BufRead;
 use errors::Result;
-use escape::unescape;
+use escape::{escape, unescape};
 use reader::{is_whitespace, Reader};
 
 use memchr;
@@ -53,19 +53,20 @@ impl<'a> Attributes<'a> {
 
 /// A struct representing a key/value for a xml attribute
 ///
-/// Parses either `key="value"` or `key='value'`
+/// Parses either `key="value"` or `key='value'`.
+/// Field `value` stores raw bytes, possibly containing escape-sequences.
 #[derive(Debug, Clone, PartialEq)]
 pub struct Attribute<'a> {
     /// the key to uniquely define the attribute
     pub key: &'a [u8],
-    /// the value
-    pub value: &'a [u8],
+    /// the raw value of attribute
+    pub value: Cow<'a, [u8]>,
 }
 
 impl<'a> Attribute<'a> {
     /// unescapes the value
     pub fn unescaped_value(&self) -> Result<Cow<[u8]>> {
-        unescape(self.value)
+        unescape(&*self.value)
     }
 
     /// unescapes then decode the value
@@ -81,19 +82,39 @@ impl<'a> Attribute<'a> {
 }
 
 impl<'a> From<(&'a [u8], &'a [u8])> for Attribute<'a> {
+    /// Creates new attribute from raw bytes.
+    /// Does not apply any transformation to both key and value.
+    ///
+    /// # Example
+    /// ```
+    /// use quick_xml::events::attributes::Attribute;
+    ///
+    /// let features = Attribute::from(("features".as_bytes(), "Bells &amp; whistles".as_bytes()));
+    /// assert_eq!(features.value, "Bells &amp; whistles".as_bytes());
+    /// ```
     fn from(val: (&'a [u8], &'a [u8])) -> Attribute<'a> {
         Attribute {
             key: val.0,
-            value: val.1,
+            value: Cow::from(val.1),
         }
     }
 }
 
 impl<'a> From<(&'a str, &'a str)> for Attribute<'a> {
+    /// Creates new attribute from text representation.
+    /// Key is stored as-is, but the value will be escaped.
+    ///
+    /// # Example
+    /// ```
+    /// use quick_xml::events::attributes::Attribute;
+    ///
+    /// let features = Attribute::from(("features", "Bells & whistles"));
+    /// assert_eq!(features.value, "Bells &amp; whistles".as_bytes());
+    /// ```
     fn from(val: (&'a str, &'a str)) -> Attribute<'a> {
         Attribute {
             key: val.0.as_bytes(),
-            value: val.1.as_bytes(),
+            value: escape(val.1.as_bytes()),
         }
     }
 }
@@ -224,7 +245,7 @@ impl<'a> Iterator for Attributes<'a> {
 
         Some(Ok(Attribute {
             key: &self.bytes[start_key..end_key],
-            value: &self.bytes[start_val..end_val],
+            value: Cow::from(&self.bytes[start_val..end_val]),
         }))
     }
 }
diff --git a/src/events/mod.rs b/src/events/mod.rs
index 3b24a77..dbc1028 100644
--- a/src/events/mod.rs
+++ b/src/events/mod.rs
@@ -126,7 +126,7 @@ impl<'a> BytesStart<'a> {
         bytes.push(b' ');
         bytes.extend_from_slice(a.key);
         bytes.extend_from_slice(b"=\"");
-        bytes.extend_from_slice(a.value);
+        bytes.extend_from_slice(&*a.value);
         bytes.push(b'"');
     }
 }
@@ -148,7 +148,7 @@ impl<'a> BytesDecl<'a> {
     }
 
     /// Gets xml version, including quotes (' or ")
-    pub fn version(&self) -> Result<&[u8]> {
+    pub fn version(&self) -> Result<Cow<[u8]>> {
         match self.element.attributes().next() {
             Some(Err(e)) => Err(e),
             Some(
@@ -170,7 +170,7 @@ impl<'a> BytesDecl<'a> {
     }
 
     /// Gets xml encoding, including quotes (' or ")
-    pub fn encoding(&self) -> Option<Result<&[u8]>> {
+    pub fn encoding(&self) -> Option<Result<Cow<[u8]>>> {
         for a in self.element.attributes() {
             match a {
                 Err(e) => return Some(Err(e)),
@@ -185,7 +185,7 @@ impl<'a> BytesDecl<'a> {
     }
 
     /// Gets xml standalone, including quotes (' or ")
-    pub fn standalone(&self) -> Option<Result<&[u8]>> {
+    pub fn standalone(&self) -> Option<Result<Cow<[u8]>>> {
         for a in self.element.attributes() {
             match a {
                 Err(e) => return Some(Err(e)),
@@ -250,7 +250,7 @@ impl<'a> BytesDecl<'a> {
     pub fn encoder(&self) -> Option<&'static Encoding> {
         self.encoding()
             .and_then(|e| e.ok())
-            .and_then(|e| Encoding::for_label(e))
+            .and_then(|e| Encoding::for_label(&*e))
     }
 }
 
diff --git a/src/reader.rs b/src/reader.rs
index ee340f2..fe7a36d 100644
--- a/src/reader.rs
+++ b/src/reader.rs
@@ -103,7 +103,7 @@ impl<B: BufRead> Reader<B> {
         }
     }
 
-    /// Change expand_empty_elements default behaviour (false per default)
+    /// Change expand_empty_elements default behaviour (true per default)
     ///
     /// When set to true, all `Empty` events are expanded into an `Open` event
     /// followed by a `Close` Event.
@@ -268,16 +268,13 @@ impl<B: BufRead> Reader<B> {
 
     /// reads `BytesElement` starting with a `!`,
     /// return `Comment`, `CData` or `DocType` event
-    ///
-    /// Note: depending on the start of the Event, we may need to read more 
-    /// data, thus we need a mutable buffer
     fn read_bang<'a, 'b>(
         &'a mut self,
         buf_start: usize,
         buf: &'b mut Vec<u8>,
     ) -> Result<Event<'b>> {
         let len = buf.len();
-        if len >= buf_start + 3 && &buf[buf_start + 1..buf_start + 3] == b"--" {
+        if len >= 3 && &buf[buf_start + 1..buf_start + 3] == b"--" {
             let mut len = buf.len();
             while len < 5 || &buf[len - 2..] != b"--" {
                 buf.push(b'>');
@@ -304,7 +301,7 @@ impl<B: BufRead> Reader<B> {
             Ok(Event::Comment(
                 BytesText::borrowed(&buf[buf_start + 3..len - 2]),
             ))
-        } else if len >= buf_start + 8 {
+        } else if len >= 8 {
             match &buf[buf_start + 1..buf_start + 8] {
                 b"[CDATA[" => {
                     let mut len = buf.len();
@@ -892,7 +889,7 @@ impl NamespaceBufferIndex {
                     match k.get(5) {
                         None => {
                             let start = buffer.len();
-                            buffer.extend_from_slice(v);
+                            buffer.extend_from_slice(&*v);
                             self.slices.push(Namespace {
                                 start: start,
                                 prefix_len: 0,
@@ -903,7 +900,7 @@ impl NamespaceBufferIndex {
                         Some(&b':') => {
                             let start = buffer.len();
                             buffer.extend_from_slice(&k[6..]);
-                            buffer.extend_from_slice(v);
+                            buffer.extend_from_slice(&*v);
                             self.slices.push(Namespace {
                                 start: start,
                                 prefix_len: k.len() - 6,


Test patch:
diff --git a/tests/test.rs b/tests/test.rs
index 3aaf498..06ea0d5 100644
--- a/tests/test.rs
+++ b/tests/test.rs
@@ -4,6 +4,7 @@ use quick_xml::reader::Reader;
 use quick_xml::events::Event::*;
 use quick_xml::events::attributes::Attribute;
 use std::io::Cursor;
+use std::borrow::Cow;
 
 #[test]
 fn test_sample() {
@@ -36,7 +37,7 @@ fn test_attributes_empty() {
                 Some(
                     Ok(Attribute {
                         key: b"att1",
-                        value: b"a",
+                        value: Cow::Borrowed(b"a"),
                     }),
                 ) => (),
                 e => panic!("Expecting att1='a' attribute, found {:?}", e),
@@ -45,7 +46,7 @@ fn test_attributes_empty() {
                 Some(
                     Ok(Attribute {
                         key: b"att2",
-                        value: b"b",
+                        value: Cow::Borrowed(b"b"),
                     }),
                 ) => (),
                 e => panic!("Expecting att2='b' attribute, found {:?}", e),
@@ -72,7 +73,7 @@ fn test_attribute_equal() {
                 Some(
                     Ok(Attribute {
                         key: b"att1",
-                        value: b"a=b",
+                        value: Cow::Borrowed(b"a=b"),
                     }),
                 ) => (),
                 e => panic!("Expecting att1=\"a=b\" attribute, found {:?}", e),
@@ -112,11 +113,11 @@ fn test_attributes_empty_ns() {
                 (opt_ns, local_name, value)
             });
     match atts.next() {
-        Some((None, b"att1", b"a")) => (),
+        Some((None, b"att1", Cow::Borrowed(b"a"))) => (),
         e => panic!("Expecting att1='a' attribute, found {:?}", e),
     }
     match atts.next() {
-        Some((Some(ns), b"att2", b"b")) => {
+        Some((Some(ns), b"att2", Cow::Borrowed(b"b"))) => {
             assert_eq!(&ns[..], b"urn:example:r");
         }
         e => panic!(
@@ -156,11 +157,11 @@ fn test_attributes_empty_ns_expanded() {
                 (opt_ns, local_name, value)
             });
         match atts.next() {
-            Some((None, b"att1", b"a")) => (),
+            Some((None, b"att1", Cow::Borrowed(b"a"))) => (),
             e => panic!("Expecting att1='a' attribute, found {:?}", e),
         }
         match atts.next() {
-            Some((Some(ns), b"att2", b"b")) => {
+            Some((Some(ns), b"att2", Cow::Borrowed(b"b"))) => {
                 assert_eq!(&ns[..], b"urn:example:r");
             }
             e => panic!(
@@ -222,7 +223,7 @@ fn test_default_ns_shadowing_empty() {
         // the attribute should _not_ have a namespace name. The default namespace does not
         // apply to attributes.
         match atts.next() {
-            Some((None, b"att1", b"a")) => (),
+            Some((None, b"att1", Cow::Borrowed(b"a"))) => (),
             e => panic!("Expecting att1='a' attribute, found {:?}", e),
         }
         match atts.next() {
@@ -283,7 +284,7 @@ fn test_default_ns_shadowing_expanded() {
         // the attribute should _not_ have a namespace name. The default namespace does not
         // apply to attributes.
         match atts.next() {
-            Some((None, b"att1", b"a")) => (),
+            Some((None, b"att1", Cow::Borrowed(b"a"))) => (),
             e => panic!("Expecting att1='a' attribute, found {:?}", e),
         }
         match atts.next() {
@@ -342,20 +343,3 @@ fn fuzz_53() {
         }
     }
 }
-
-#[test]
-fn test_issue94() {
-    let data = br#"<Run>
-<!B>
-</Run>"#;
-    let mut reader = Reader::from_reader(&data[..]);
-    reader.trim_text(true);
-    let mut buf = vec![];
-    loop {
-        match reader.read_event(&mut buf) {
-            Ok(quick_xml::events::Event::Eof) | Err(..) => break,
-            _ => buf.clear(),
-        }
-        buf.clear();
-    }
-}
diff --git a/tests/unit_tests.rs b/tests/unit_tests.rs
index 01f6328..b40f6f3 100644
--- a/tests/unit_tests.rs
+++ b/tests/unit_tests.rs
@@ -128,19 +128,19 @@ fn test_xml_decl() {
         Decl(ref e) => {
             match e.version() {
                 Ok(v) => assert_eq!(
-                    v,
+                    &*v,
                     b"1.0",
                     "expecting version '1.0', got '{:?}",
-                    from_utf8(v)
+                    from_utf8(&*v)
                 ),
                 Err(e) => assert!(false, "{:?}", e),
             }
             match e.encoding() {
                 Some(Ok(v)) => assert_eq!(
-                    v,
+                    &*v,
                     b"utf-8",
                     "expecting encoding 'utf-8', got '{:?}",
-                    from_utf8(v)
+                    from_utf8(&*v)
                 ),
                 Some(Err(e)) => panic!("{:?}", e),
                 None => panic!("cannot find encoding"),
@@ -287,7 +287,7 @@ fn test_write_empty_element_attrs() {
 #[test]
 fn test_write_attrs() {
     let str_from = r#"<source attr="val"></source>"#;
-    let expected = r#"<copy attr="val" a="b" c="d" x="y"></copy>"#;
+    let expected = r#"<copy attr="val" a="b" c="d" x="y&quot;z"></copy>"#;
     let mut reader = Reader::from_str(str_from);
     reader.trim_text(true);
     let mut writer = Writer::new(Cursor::new(Vec::new()));
@@ -300,7 +300,7 @@ fn test_write_attrs() {
                 attrs.extend_from_slice(&[("a", "b").into(), ("c", "d").into()]);
                 let mut elem = BytesStart::owned(b"copy".to_vec(), 4);
                 elem.extend_attributes(attrs);
-                elem.push_attribute(("x", "y"));
+                elem.push_attribute(("x", "y\"z"));
                 Start(elem)
             }
             Ok(End(_)) => End(BytesEnd::borrowed(b"copy")),
@@ -640,11 +640,11 @@ fn test_closing_bracket_in_single_quote_attr() {
         Ok(Start(e)) => {
             let mut attrs = e.attributes();
             match attrs.next() {
-                Some(Ok(attr)) => assert_eq!(attr, ("attr", ">").into()),
+                Some(Ok(attr)) => assert_eq!(attr, ("attr".as_bytes(), ">".as_bytes()).into()),
                 x => panic!("expected attribute 'attr', got {:?}", x),
             }
             match attrs.next() {
-                Some(Ok(attr)) => assert_eq!(attr, ("check", "2").into()),
+                Some(Ok(attr)) => assert_eq!(attr, ("check".as_bytes(), "2".as_bytes()).into()),
                 x => panic!("expected attribute 'check', got {:?}", x),
             }
             assert!(attrs.next().is_none(), "expected only two attributes");
@@ -663,11 +663,11 @@ fn test_closing_bracket_in_double_quote_attr() {
         Ok(Start(e)) => {
             let mut attrs = e.attributes();
             match attrs.next() {
-                Some(Ok(attr)) => assert_eq!(attr, ("attr", ">").into()),
+                Some(Ok(attr)) => assert_eq!(attr, ("attr".as_bytes(), ">".as_bytes()).into()),
                 x => panic!("expected attribute 'attr', got {:?}", x),
             }
             match attrs.next() {
-                Some(Ok(attr)) => assert_eq!(attr, ("check", "2").into()),
+                Some(Ok(attr)) => assert_eq!(attr, ("check".as_bytes(), "2".as_bytes()).into()),
                 x => panic!("expected attribute 'check', got {:?}", x),
             }
             assert!(attrs.next().is_none(), "expected only two attributes");
@@ -686,11 +686,11 @@ fn test_closing_bracket_in_double_quote_mixed() {
         Ok(Start(e)) => {
             let mut attrs = e.attributes();
             match attrs.next() {
-                Some(Ok(attr)) => assert_eq!(attr, ("attr", "'>'").into()),
+                Some(Ok(attr)) => assert_eq!(attr, ("attr".as_bytes(), "'>'".as_bytes()).into()),
                 x => panic!("expected attribute 'attr', got {:?}", x),
             }
             match attrs.next() {
-                Some(Ok(attr)) => assert_eq!(attr, ("check", "'2'").into()),
+                Some(Ok(attr)) => assert_eq!(attr, ("check".as_bytes(), "'2'".as_bytes()).into()),
                 x => panic!("expected attribute 'check', got {:?}", x),
             }
             assert!(attrs.next().is_none(), "expected only two attributes");
@@ -709,11 +709,11 @@ fn test_closing_bracket_in_single_quote_mixed() {
         Ok(Start(e)) => {
             let mut attrs = e.attributes();
             match attrs.next() {
-                Some(Ok(attr)) => assert_eq!(attr, ("attr", "\">\"").into()),
+                Some(Ok(attr)) => assert_eq!(attr, ("attr".as_bytes(), "\">\"".as_bytes()).into()),
                 x => panic!("expected attribute 'attr', got {:?}", x),
             }
             match attrs.next() {
-                Some(Ok(attr)) => assert_eq!(attr, ("check", "\"2\"").into()),
+                Some(Ok(attr)) => assert_eq!(attr, ("check".as_bytes(), "\"2\"".as_bytes()).into()),
                 x => panic!("expected attribute 'check', got {:?}", x),
             }
             assert!(attrs.next().is_none(), "expected only two attributes");
diff --git a/tests/xmlrs_reader_tests.rs b/tests/xmlrs_reader_tests.rs
index 7fb15fc..8b8fedb 100644
--- a/tests/xmlrs_reader_tests.rs
+++ b/tests/xmlrs_reader_tests.rs
@@ -412,8 +412,10 @@ impl<'a, 'b> fmt::Display for OptEvent<'a, 'b> {
                 Err(ref e) => write!(f, "{}", e),
             },
             Ok((_, Event::Decl(ref e))) => {
-                let version = from_utf8(e.version().unwrap()).unwrap();
-                let encoding = from_utf8(e.encoding().unwrap().unwrap()).unwrap();
+                let version_cow = e.version().unwrap();
+                let version = from_utf8(version_cow.as_ref()).unwrap();
+                let encoding_cow = e.encoding().unwrap().unwrap();
+                let encoding = from_utf8(encoding_cow.as_ref()).unwrap();
                 write!(f, "StartDocument({}, {})", version, encoding)
             }
             Ok((_, Event::Eof)) => write!(f, "EndDocument"),