Add Utf8BytesReader infrastructure

When the source of the bytes isn't UTF-8 (or isn't known to be), the
bytes need to be decoded first, or at least validated as such. Wrap
'Read'ers with Utf8BytesReader to ensure this happens.

Defer the validating portion for now.

Author: dralley

Cargo.toml

@@ -14,6 +14,7 @@ license = "MIT"
 [dependencies]
 document-features = { version = "0.2", optional = true }
 encoding_rs = { version = "0.8", optional = true }
+encoding_rs_io = { version = "0.1", optional = true }
 serde = { version = "1.0", optional = true }
 tokio = { version = "1.20", optional = true, default-features = false, features = ["io-util"] }
 memchr = "2.5"
@@ -102,7 +103,7 @@ async-tokio = ["tokio"]
 ## [UTF-16LE]: encoding_rs::UTF_16LE
 ## [ISO-2022-JP]: encoding_rs::ISO_2022_JP
 ## [#158]: https://github.com/tafia/quick-xml/issues/158
-encoding = ["encoding_rs"]
+encoding = ["encoding_rs", "encoding_rs_io"]
 
 ## Enables support for recognizing all [HTML 5 entities] in [`unescape`] and
 ## [`unescape_with`] functions. The full list of entities also can be found in

src/de/mod.rs

@@ -110,7 +110,7 @@ mod var;
 
 pub use crate::errors::serialize::DeError;
 use crate::{
-    encoding::Decoder,
+    encoding::{Decoder, Utf8BytesReader},
     errors::Error,
     events::{BytesCData, BytesEnd, BytesStart, BytesText, Event},
     name::QName,
@@ -592,7 +592,7 @@ impl<'de> Deserializer<'de, SliceReader<'de>> {
     }
 }
 
-impl<'de, R> Deserializer<'de, IoReader<R>>
+impl<'de, R> Deserializer<'de, IoReader<Utf8BytesReader<R>>>
 where
     R: BufRead,
 {

src/encoding.rs

@@ -1,9 +1,12 @@
 //! A module for wrappers that encode / decode data.
 
 use std::borrow::Cow;
+use std::io;
 
 #[cfg(feature = "encoding")]
 use encoding_rs::{Encoding, UTF_16BE, UTF_16LE, UTF_8};
+#[cfg(feature = "encoding")]
+use encoding_rs_io::{DecodeReaderBytes, DecodeReaderBytesBuilder};
 
 #[cfg(feature = "encoding")]
 use crate::Error;
@@ -21,6 +24,57 @@ pub(crate) const UTF16_LE_BOM: &[u8] = &[0xFF, 0xFE];
 #[cfg(feature = "encoding")]
 pub(crate) const UTF16_BE_BOM: &[u8] = &[0xFE, 0xFF];
 
+/// A struct for transparently decoding / validating bytes as UTF-8.
+#[derive(Debug)]
+pub struct Utf8BytesReader<R> {
+    #[cfg(feature = "encoding")]
+    reader: io::BufReader<DecodeReaderBytes<R, Vec<u8>>>,
+    #[cfg(not(feature = "encoding"))]
+    reader: io::BufReader<R>,
+}
+
+impl<R: io::Read> Utf8BytesReader<R> {
+    /// Build a new reader which decodes a stream of bytes in an unknown encoding into UTF-8.
+    /// Note: The consumer is responsible for finding the correct character boundaries when
+    /// treating a given range of bytes as UTF-8.
+    #[cfg(feature = "encoding")]
+    pub fn new(reader: R) -> Self {
+        let decoder = DecodeReaderBytesBuilder::new()
+            .bom_override(true)
+            .build(reader);
+
+        Self {
+            reader: io::BufReader::new(decoder),
+        }
+    }
+
+    /// Build a new reader which (will eventually) validate UTF-8.
+    /// Note: The consumer is responsible for finding the correct character boundaries when
+    /// treating a given range of bytes as UTF-8.
+    #[cfg(not(feature = "encoding"))]
+    pub fn new(reader: R) -> Self {
+        Self {
+            reader: io::BufReader::new(reader),
+        }
+    }
+}
+
+impl<R: io::Read> io::Read for Utf8BytesReader<R> {
+    fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> {
+        self.reader.read(buf)
+    }
+}
+
+impl<R: io::Read> io::BufRead for Utf8BytesReader<R> {
+    fn fill_buf(&mut self) -> io::Result<&[u8]> {
+        self.reader.fill_buf()
+    }
+
+    fn consume(&mut self, amt: usize) {
+        self.reader.consume(amt)
+    }
+}
+
 /// Decoder of byte slices into strings.
 ///
 /// If feature `encoding` is enabled, this encoding taken from the `"encoding"`

src/reader/buffered_reader.rs

@@ -2,11 +2,12 @@
 //! underlying byte stream.
 
 use std::fs::File;
-use std::io::{self, BufRead, BufReader};
+use std::io::{self, BufRead};
 use std::path::Path;
 
 use memchr;
 
+use crate::encoding::Utf8BytesReader;
 use crate::errors::{Error, Result};
 use crate::events::Event;
 use crate::name::QName;
@@ -34,6 +35,7 @@ macro_rules! impl_buffered_source {
 
         #[cfg(feature = "encoding")]
         $($async)? fn detect_encoding(&mut self) -> Result<Option<&'static encoding_rs::Encoding>> {
+            // TODO: broken because decoder sends UTF-8
             loop {
                 break match self $(.$reader)? .fill_buf() $(.$await)? {
                     Ok(n) => if let Some((enc, bom_len)) = crate::encoding::detect_encoding(n) {
@@ -395,15 +397,13 @@ impl<R: BufRead> Reader<R> {
     }
 }
 
-impl Reader<BufReader<File>> {
+impl Reader<Utf8BytesReader<File>> {
     /// Creates an XML reader from a file path.
     pub fn from_file<P: AsRef<Path>>(path: P) -> Result<Self> {
         let file = File::open(path).map_err(Error::Io)?;
-        let reader = BufReader::new(file);
-        Ok(Self::from_reader(reader))
+        Ok(Self::from_reader(file))
     }
 }
-
 #[cfg(test)]
 mod test {
     use crate::reader::test::check;

src/reader/mod.rs

@@ -1,10 +1,12 @@
 //! Contains high-level interface for a pull-based XML parser.
 
+use std::io::Read;
+use std::ops::Range;
+
 #[cfg(feature = "encoding")]
 use encoding_rs::Encoding;
-use std::ops::Range;
 
-use crate::encoding::Decoder;
+use crate::encoding::{Decoder, Utf8BytesReader};
 use crate::errors::{Error, Result};
 use crate::events::Event;
 use crate::reader::parser::Parser;
@@ -359,7 +361,7 @@ enum ParseState {
 ///   BomDetected -- "encoding=..." --> XmlDetected
 /// ```
 #[cfg(feature = "encoding")]
-#[derive(Clone, Copy)]
+#[derive(Clone, Copy, Debug)]
 enum EncodingRef {
     /// Encoding was implicitly assumed to have a specified value. It can be refined
     /// using BOM or by the XML declaration event (`<?xml encoding=... ?>`)
@@ -459,73 +461,22 @@ pub struct Reader<R> {
 }
 
 /// Builder methods
-impl<R> Reader<R> {
+impl<R: Read> Reader<Utf8BytesReader<R>> {
     /// Creates a `Reader` that reads from a given reader.
     pub fn from_reader(reader: R) -> Self {
         Self {
-            reader,
+            reader: Utf8BytesReader::new(reader),
             parser: Parser::default(),
         }
     }
-
-    configure_methods!();
 }
 
-/// Getters
+/// Public implementation-independent functionality
 impl<R> Reader<R> {
-    /// Consumes `Reader` returning the underlying reader
-    ///
-    /// Can be used to compute line and column of a parsing error position
-    ///
-    /// # Examples
-    ///
-    /// ```
-    /// # use pretty_assertions::assert_eq;
-    /// use std::{str, io::Cursor};
-    /// use quick_xml::events::Event;
-    /// use quick_xml::reader::Reader;
-    ///
-    /// let xml = r#"<tag1 att1 = "test">
-    ///                 <tag2><!--Test comment-->Test</tag2>
-    ///                 <tag3>Test 2</tag3>
-    ///              </tag1>"#;
-    /// let mut reader = Reader::from_reader(Cursor::new(xml.as_bytes()));
-    /// let mut buf = Vec::new();
-    ///
-    /// fn into_line_and_column(reader: Reader<Cursor<&[u8]>>) -> (usize, usize) {
-    ///     let end_pos = reader.buffer_position();
-    ///     let mut cursor = reader.into_inner();
-    ///     let s = String::from_utf8(cursor.into_inner()[0..end_pos].to_owned())
-    ///         .expect("can't make a string");
-    ///     let mut line = 1;
-    ///     let mut column = 0;
-    ///     for c in s.chars() {
-    ///         if c == '\n' {
-    ///             line += 1;
-    ///             column = 0;
-    ///         } else {
-    ///             column += 1;
-    ///         }
-    ///     }
-    ///     (line, column)
-    /// }
-    ///
-    /// loop {
-    ///     match reader.read_event_into(&mut buf) {
-    ///         Ok(Event::Start(ref e)) => match e.name().as_ref() {
-    ///             b"tag1" | b"tag2" => (),
-    ///             tag => {
-    ///                 assert_eq!(b"tag3", tag);
-    ///                 assert_eq!((3, 22), into_line_and_column(reader));
-    ///                 break;
-    ///             }
-    ///         },
-    ///         Ok(Event::Eof) => unreachable!(),
-    ///         _ => (),
-    ///     }
-    ///     buf.clear();
-    /// }
-    /// ```
+    // Configuration setters
+    configure_methods!();
+
+    /// Consumes `Reader` returning the underlying reader.
     pub fn into_inner(self) -> R {
         self.reader
     }
@@ -1612,7 +1563,7 @@ mod test {
                 /// character should be stripped for consistency
                 #[$test]
                 $($async)? fn bom_from_reader() {
-                    let mut reader = Reader::from_reader("\u{feff}\u{feff}".as_bytes());
+                    let mut reader = Reader::from_str("\u{feff}\u{feff}");
 
                     assert_eq!(
                         reader.$read_event($buf) $(.$await)? .unwrap(),

src/reader/ns_reader.rs

@@ -6,15 +6,15 @@
 
 use std::borrow::Cow;
 use std::fs::File;
-use std::io::{BufRead, BufReader};
+use std::io::{BufRead, Read};
 use std::ops::Deref;
 use std::path::Path;
 
+use crate::encoding::Utf8BytesReader;
 use crate::errors::Result;
 use crate::events::Event;
 use crate::name::{LocalName, NamespaceResolver, QName, ResolveResult};
 use crate::reader::{Reader, Span, XmlSource};
-
 /// A low level encoding-agnostic XML event reader that performs namespace resolution.
 ///
 /// Consumes a [`BufRead`] and streams XML `Event`s.
@@ -33,14 +33,12 @@ pub struct NsReader<R> {
 }
 
 /// Builder methods
-impl<R> NsReader<R> {
+impl<R: Read> NsReader<Utf8BytesReader<R>> {
     /// Creates a `NsReader` that reads from a reader.
     #[inline]
     pub fn from_reader(reader: R) -> Self {
         Self::new(Reader::from_reader(reader))
     }
-
-    configure_methods!(reader);
 }
 
 /// Private methods
@@ -118,8 +116,11 @@ impl<R> NsReader<R> {
     }
 }
 
-/// Getters
+/// Public implementation-independent functionality
 impl<R> NsReader<R> {
+    // Configuration setters
+    configure_methods!(reader);
+
     /// Consumes `NsReader` returning the underlying reader
     ///
     /// See the [`Reader::into_inner`] for examples
@@ -528,7 +529,7 @@ impl<R: BufRead> NsReader<R> {
     }
 }
 
-impl NsReader<BufReader<File>> {
+impl NsReader<Utf8BytesReader<File>> {
     /// Creates an XML reader from a file path.
     pub fn from_file<P: AsRef<Path>>(path: P) -> Result<Self> {
         Ok(Self::new(Reader::from_file(path)?))

src/reader/parser.rs

@@ -171,7 +171,6 @@ impl Parser {
         if len > 2 && buf[len - 1] == b'?' {
             if len > 5 && &buf[1..4] == b"xml" && is_whitespace(buf[4]) {
                 let event = BytesDecl::from_start(BytesStart::wrap(&buf[1..len - 1], 3));
-
                 // Try getting encoding from the declaration event
                 #[cfg(feature = "encoding")]
                 if self.encoding.can_be_refined() {

src/reader/slice_reader.rs

@@ -16,22 +16,32 @@ use crate::reader::{is_whitespace, BangType, ReadElementState, Reader, Span, Xml
 
 use memchr;
 
-/// This is an implementation for reading from a `&[u8]` as underlying byte stream.
-/// This implementation supports not using an intermediate buffer as the byte slice
-/// itself can be used to borrow from.
+use super::parser::Parser;
+
+/// This is an implementation of [`Reader`] for reading from a `&[u8]` as
+/// underlying byte stream. This implementation supports not using an
+/// intermediate buffer as the byte slice itself can be used to borrow from.
 impl<'a> Reader<&'a [u8]> {
     /// Creates an XML reader from a string slice.
     pub fn from_str(s: &'a str) -> Self {
         // Rust strings are guaranteed to be UTF-8, so lock the encoding
         #[cfg(feature = "encoding")]
         {
-            let mut reader = Self::from_reader(s.as_bytes());
-            reader.parser.encoding = EncodingRef::Explicit(UTF_8);
-            reader
+            let mut parser = Parser::default();
+            parser.encoding = EncodingRef::Explicit(UTF_8);
+            Self {
+                reader: s.as_bytes(),
+                parser: parser,
+            }
         }
 
         #[cfg(not(feature = "encoding"))]
-        Self::from_reader(s.as_bytes())
+        {
+            Self {
+                reader: s.as_bytes(),
+                parser: Parser::default(),
+            }
+        }
     }
 
     /// Read an event that borrows from the input rather than a buffer.

tests/test.rs

@@ -100,7 +100,7 @@ fn test_issue94() {
     let mut reader = Reader::from_reader(&data[..]);
     reader.trim_text(true);
     loop {
-        match reader.read_event() {
+        match reader.read_event_into(&mut Vec::new()) {
             Ok(Eof) | Err(..) => break,
             _ => (),
         }

tests/xmlrs_reader_tests.rs

@@ -392,7 +392,7 @@ fn test_bytes(input: &[u8], output: &[u8], trim: bool) {
 
     let mut decoder = reader.decoder();
     loop {
-        let line = match reader.read_resolved_event() {
+        let line = match reader.read_resolved_event_into(&mut Vec::new()) {
             Ok((_, Event::Decl(e))) => {
                 // Declaration could change decoder
                 decoder = reader.decoder();