Repository: tafia/quick-xml
Pull request #114 (2018-02-01 03:06:58)

Clarify and extend the API Surface of BytesText
Neither of the 3 constructors `borrowed`, `owned` and `from_str` documented if they expect pre-escaped data. This behavior also silently changed completely in #96 causing a few bugs here and there. The API was therefore completely revamped in order to allow constructing text events from either escaped or unescaped text.


Patch:
diff --git a/src/events/mod.rs b/src/events/mod.rs
index ee2b60f..a1febaa 100644
--- a/src/events/mod.rs
+++ b/src/events/mod.rs
@@ -298,34 +298,50 @@ impl<'a> BytesEnd<'a> {
 /// Data from various events (most notably, `Event::Text`).
 #[derive(Clone, Debug)]
 pub struct BytesText<'a> {
+    // Invariant: The content is always escaped.
     content: Cow<'a, [u8]>,
 }
 
 impl<'a> BytesText<'a> {
-    /// Creates a new `BytesText` borrowing a slice
+    /// Creates a new `BytesText` from an escaped byte sequence.
     #[inline]
-    pub fn borrowed(content: &'a [u8]) -> BytesText<'a> {
+    pub fn from_escaped<C: Into<Cow<'a, [u8]>>>(content: C) -> BytesText<'a> {
         BytesText {
-            content: Cow::Borrowed(content),
+            content: content.into(),
         }
     }
 
-    /// Creates a new `BytesText` owning its name
+    /// Creates a new `BytesText` from a byte sequence. The byte sequence is
+    /// expected not to be escaped.
     #[inline]
-    pub fn owned(content: Vec<u8>) -> BytesText<'static> {
+    pub fn from_plain(content: &'a [u8]) -> BytesText<'a> {
         BytesText {
-            content: Cow::Owned(content),
+            content: escape(content),
         }
     }
 
-    /// Creates a new `BytesText` from text.
-    ///
-    /// The text will be XML-escaped automatically.
+    /// Creates a new `BytesText` from an escaped string.
+    #[inline]
+    pub fn from_escaped_str<C: Into<Cow<'a, str>>>(content: C) -> BytesText<'a> {
+        Self::from_escaped(match content.into() {
+            Cow::Owned(o) => Cow::Owned(o.into_bytes()),
+            Cow::Borrowed(b) => Cow::Borrowed(b.as_bytes()),
+        })
+    }
+
+    /// Creates a new `BytesText` from a string. The string is expected not to
+    /// be escaped.
+    #[inline]
+    pub fn from_plain_str(content: &'a str) -> BytesText<'a> {
+        Self::from_plain(content.as_bytes())
+    }
+
+    /// Ensures that all data is owned to extend the object's lifetime if
+    /// necessary.
     #[inline]
-    pub fn from_str<S: AsRef<str>>(text: S) -> BytesText<'static> {
-        let bytes = escape(text.as_ref().as_bytes()).into_owned();
+    pub fn into_owned(self) -> BytesText<'static> {
         BytesText {
-            content: Cow::Owned(bytes),
+            content: self.content.into_owned().into(),
         }
     }
 
diff --git a/src/reader.rs b/src/reader.rs
index 3cd45a6..c4041a3 100644
--- a/src/reader.rs
+++ b/src/reader.rs
@@ -196,7 +196,7 @@ impl<B: BufRead> Reader<B> {
                 } else {
                     (buf_start, buf.len())
                 };
-                Ok(Event::Text(BytesText::borrowed(&buf[start..len])))
+                Ok(Event::Text(BytesText::from_escaped(&buf[start..len])))
             }
             Err(e) => Err(e),
         }
@@ -328,7 +328,7 @@ impl<B: BufRead> Reader<B> {
                     offset -= 1;
                 }
             }
-            Ok(Event::Comment(BytesText::borrowed(
+            Ok(Event::Comment(BytesText::from_escaped(
                 &buf[buf_start + 3..len - 2],
             )))
         } else if len >= buf_start + 8 {
@@ -347,7 +347,7 @@ impl<B: BufRead> Reader<B> {
                         }
                         len = buf.len();
                     }
-                    Ok(Event::CData(BytesText::borrowed(
+                    Ok(Event::CData(BytesText::from_escaped(
                         &buf[buf_start + 8..len - 2],
                     )))
                 }
@@ -370,7 +370,7 @@ impl<B: BufRead> Reader<B> {
                         }
                     }
                     let len = buf.len();
-                    Ok(Event::DocType(BytesText::borrowed(
+                    Ok(Event::DocType(BytesText::from_escaped(
                         &buf[buf_start + 8..len],
                     )))
                 }
@@ -395,7 +395,7 @@ impl<B: BufRead> Reader<B> {
                 }
                 Ok(Event::Decl(event))
             } else {
-                Ok(Event::PI(BytesText::borrowed(&buf[1..len - 1])))
+                Ok(Event::PI(BytesText::from_escaped(&buf[1..len - 1])))
             }
         } else {
             self.buf_position -= len;


Test patch:
diff --git a/tests/unit_tests.rs b/tests/unit_tests.rs
index 337de80..8431846 100644
--- a/tests/unit_tests.rs
+++ b/tests/unit_tests.rs
@@ -3,7 +3,7 @@ extern crate quick_xml;
 use std::str::from_utf8;
 use std::io::Cursor;
 
-use quick_xml::{Reader, Writer, Result};
+use quick_xml::{Reader, Result, Writer};
 use quick_xml::events::{BytesDecl, BytesEnd, BytesStart, BytesText, Event};
 use quick_xml::events::Event::*;
 
@@ -663,7 +663,7 @@ fn test_read_write_roundtrip_escape() {
                 let t = e.escaped();
                 assert!(
                     writer
-                        .write_event(Event::Text(BytesText::owned(t.to_vec())))
+                        .write_event(Event::Text(BytesText::from_escaped(t.to_vec())))
                         .is_ok()
                 );
             }
@@ -698,7 +698,7 @@ fn test_read_write_roundtrip_escape_text() {
                 let t = e.unescape_and_decode(&reader).unwrap();
                 assert!(
                     writer
-                        .write_event(Event::Text(BytesText::from_str(t)))
+                        .write_event(Event::Text(BytesText::from_plain_str(&t)))
                         .is_ok()
                 );
             }