Repository: tafia/quick-xml (rust)

Issue:
Lifetime on Attributes is wrong
So `Attributes` has a generic parameter `'a`, but it doesn't correspond to the same lifetime of the slice inside the reader. It corresponds to the lifetime of the borrow of the reader itself when the `Attributes` object was created.

One can easily see that making this change will break the code:
```diff
diff --git a/src/events/mod.rs b/src/events/mod.rs
index 5e598bd..861ce7b 100644
--- a/src/events/mod.rs
+++ b/src/events/mod.rs
@@ -223,12 +223,12 @@ impl<'a> BytesStart<'a> {
     }
 
     /// Returns an iterator over the attributes of this tag.
-    pub fn attributes(&self) -> Attributes {
+    pub fn attributes(&self) -> Attributes<'a> {
         Attributes::new(&self.buf, self.name_len)
     }
 
     /// Returns an iterator over the HTML-like attributes of this tag (no mandatory quotes or `=`).
-    pub fn html_attributes(&self) -> Attributes {
+    pub fn html_attributes(&self) -> Attributes<'a> {
         Attributes::html(self, self.name_len)
     }
 
```

I'm working on fixing that using as little as possible reallocations, which would mean having a double lifetime on `Attributes`.

Pull request description:
Remove requirement to have Reader when decoding attributes and write anything converted to Event
This PR includes two changes, which both related to my work of splitting event into two event classes -- for reading and for writing. Events for reading then would only borrow data and this will help to fix #332 and also open doors to `no_std` reader (not sure why it would be needed although).

The first commit just removes unused import that I noticed while working on this.

The second commit relaxes requirements for decoding values from `Attribute`. If you created attribute for writing and then for whatever reason wants to read its value back, you need a `Reader` which you does not have. This commit fixes that.

The third commit makes writing methods take `Into<Event>` instead of `AsRef<Event>`. The former trait give a lesser abilities then `Into`. It allows you to pass your own struct to write, but you need to have event in your struct to be able to take a ref to it. `Into` would allow you to construct event on demand and also would allow to directly pass reader event to the writer method that would require writer event if (when?) event splitting will be implemented. In any case, I think, that taking `Into` is better.

If there are any objections, then unnecessary commits can be removed from PR.

Patch:
diff --git a/Changelog.md b/Changelog.md
index e022b44..255f2b3 100644
--- a/Changelog.md
+++ b/Changelog.md
@@ -19,6 +19,12 @@
 
 ### Misc Changes
 
+- [#760]: `Attribute::decode_and_unescape_value` and `Attribute::decode_and_unescape_value_with` now
+  accepts `Decoder` instead of `Reader`. Use `Reader::decoder()` to get it.
+- [#760]: `Writer::write_event` now consumes event. Use `Event::borrow()` if you want to keep ownership.
+
+[#760]: https://github.com/tafia/quick-xml/pull/760
+
 
 ## 0.33.0 -- 2024-06-21
 
diff --git a/benches/macrobenches.rs b/benches/macrobenches.rs
index a8cbbd5..2b882b1 100644
--- a/benches/macrobenches.rs
+++ b/benches/macrobenches.rs
@@ -50,7 +50,7 @@ fn parse_document_from_str(doc: &str) -> XmlResult<()> {
         match criterion::black_box(r.read_event()?) {
             Event::Start(e) | Event::Empty(e) => {
                 for attr in e.attributes() {
-                    criterion::black_box(attr?.decode_and_unescape_value(&r)?);
+                    criterion::black_box(attr?.decode_and_unescape_value(r.decoder())?);
                 }
             }
             Event::Text(e) => {
@@ -75,7 +75,7 @@ fn parse_document_from_bytes(doc: &[u8]) -> XmlResult<()> {
         match criterion::black_box(r.read_event_into(&mut buf)?) {
             Event::Start(e) | Event::Empty(e) => {
                 for attr in e.attributes() {
-                    criterion::black_box(attr?.decode_and_unescape_value(&r)?);
+                    criterion::black_box(attr?.decode_and_unescape_value(r.decoder())?);
                 }
             }
             Event::Text(e) => {
@@ -101,7 +101,7 @@ fn parse_document_from_str_with_namespaces(doc: &str) -> XmlResult<()> {
             (resolved_ns, Event::Start(e) | Event::Empty(e)) => {
                 criterion::black_box(resolved_ns);
                 for attr in e.attributes() {
-                    criterion::black_box(attr?.decode_and_unescape_value(&r)?);
+                    criterion::black_box(attr?.decode_and_unescape_value(r.decoder())?);
                 }
             }
             (resolved_ns, Event::Text(e)) => {
@@ -129,7 +129,7 @@ fn parse_document_from_bytes_with_namespaces(doc: &[u8]) -> XmlResult<()> {
             (resolved_ns, Event::Start(e) | Event::Empty(e)) => {
                 criterion::black_box(resolved_ns);
                 for attr in e.attributes() {
-                    criterion::black_box(attr?.decode_and_unescape_value(&r)?);
+                    criterion::black_box(attr?.decode_and_unescape_value(r.decoder())?);
                 }
             }
             (resolved_ns, Event::Text(e)) => {
diff --git a/examples/custom_entities.rs b/examples/custom_entities.rs
index 5be5345..e68a682 100644
--- a/examples/custom_entities.rs
+++ b/examples/custom_entities.rs
@@ -47,7 +47,7 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
                         .attributes()
                         .map(|a| {
                             a.unwrap()
-                                .decode_and_unescape_value_with(&reader, |ent| {
+                                .decode_and_unescape_value_with(reader.decoder(), |ent| {
                                     custom_entities.get(ent).map(|s| s.as_str())
                                 })
                                 .unwrap()
diff --git a/examples/read_nodes.rs b/examples/read_nodes.rs
index 776f71d..6c32f48 100644
--- a/examples/read_nodes.rs
+++ b/examples/read_nodes.rs
@@ -70,8 +70,8 @@ impl Translation {
         for attr_result in element.attributes() {
             let a = attr_result?;
             match a.key.as_ref() {
-                b"Language" => lang = a.decode_and_unescape_value(reader)?,
-                b"Tag" => tag = a.decode_and_unescape_value(reader)?,
+                b"Language" => lang = a.decode_and_unescape_value(reader.decoder())?,
+                b"Tag" => tag = a.decode_and_unescape_value(reader.decoder())?,
                 _ => (),
             }
         }
@@ -138,7 +138,7 @@ fn main() -> Result<(), AppError> {
                                             Ok::<Cow<'_, str>, Infallible>(std::borrow::Cow::from(""))
                                         })
                                         .unwrap().to_string();
-                                    let value = a.decode_and_unescape_value(&reader).or_else(|err| {
+                                    let value = a.decode_and_unescape_value(reader.decoder()).or_else(|err| {
                                             dbg!("unable to read key in DefaultSettings attribute {:?}, utf8 error {:?}", &a, err);
                                             Ok::<Cow<'_, str>, Infallible>(std::borrow::Cow::from(""))
                                     }).unwrap().to_string();
diff --git a/fuzz/fuzz_targets/fuzz_target_1.rs b/fuzz/fuzz_targets/fuzz_target_1.rs
index 9809526..08d983e 100644
--- a/fuzz/fuzz_targets/fuzz_target_1.rs
+++ b/fuzz/fuzz_targets/fuzz_target_1.rs
@@ -26,7 +26,7 @@ where
             let _event = black_box(event.borrow());
             let _event = black_box(event.as_ref());
             debug_format!(event);
-            debug_format!(writer.write_event(event));
+            debug_format!(writer.write_event(event.borrow()));
         }
         match event_result {
             Ok(Event::Start(ref e)) | Ok(Event::Empty(ref e)) => {
diff --git a/fuzz/fuzz_targets/structured_roundtrip.rs b/fuzz/fuzz_targets/structured_roundtrip.rs
index 2eb6962..6432cb9 100644
--- a/fuzz/fuzz_targets/structured_roundtrip.rs
+++ b/fuzz/fuzz_targets/structured_roundtrip.rs
@@ -51,7 +51,7 @@ fn fuzz_round_trip(driver: Driver) -> quick_xml::Result<()> {
         // TODO: Handle error cases.
         use WriterFunc::*;
         match writer_func {
-            WriteEvent(event) => writer.write_event(event)?,
+            WriteEvent(event) => writer.write_event(event.borrow())?,
             WriteBom => writer.write_bom()?,
             WriteIndent => writer.write_indent()?,
             CreateElement {
diff --git a/src/events/attributes.rs b/src/events/attributes.rs
index 2d1abfd..5f56c04 100644
--- a/src/events/attributes.rs
+++ b/src/events/attributes.rs
@@ -2,11 +2,12 @@
 //!
 //! Provides an iterator over attributes key/value pairs
 
+use crate::encoding::Decoder;
 use crate::errors::Result as XmlResult;
 use crate::escape::{escape, resolve_predefined_entity, unescape_with};
 use crate::name::QName;
-use crate::reader::Reader;
 use crate::utils::{is_whitespace, write_byte_string, write_cow_string, Bytes};
+
 use std::fmt::{self, Debug, Display, Formatter};
 use std::iter::FusedIterator;
 use std::{borrow::Cow, ops::Range};
@@ -84,23 +85,23 @@ impl<'a> Attribute<'a> {
     ///
     /// This will allocate if the value contains any escape sequences or in
     /// non-UTF-8 encoding.
-    pub fn decode_and_unescape_value<B>(&self, reader: &Reader<B>) -> XmlResult<Cow<'a, str>> {
-        self.decode_and_unescape_value_with(reader, resolve_predefined_entity)
+    pub fn decode_and_unescape_value(&self, decoder: Decoder) -> XmlResult<Cow<'a, str>> {
+        self.decode_and_unescape_value_with(decoder, resolve_predefined_entity)
     }
 
     /// Decodes then unescapes the value with custom entities.
     ///
     /// This will allocate if the value contains any escape sequences or in
     /// non-UTF-8 encoding.
-    pub fn decode_and_unescape_value_with<'entity, B>(
+    pub fn decode_and_unescape_value_with<'entity>(
         &self,
-        reader: &Reader<B>,
+        decoder: Decoder,
         resolve_entity: impl FnMut(&str) -> Option<&'entity str>,
     ) -> XmlResult<Cow<'a, str>> {
         let decoded = match &self.value {
-            Cow::Borrowed(bytes) => reader.decoder().decode(bytes)?,
+            Cow::Borrowed(bytes) => decoder.decode(bytes)?,
             // Convert to owned, because otherwise Cow will be bound with wrong lifetime
-            Cow::Owned(bytes) => reader.decoder().decode(bytes)?.into_owned().into(),
+            Cow::Owned(bytes) => decoder.decode(bytes)?.into_owned().into(),
         };
 
         match unescape_with(&decoded, resolve_entity)? {
diff --git a/src/reader/ns_reader.rs b/src/reader/ns_reader.rs
index cfeb73d..0722081 100644
--- a/src/reader/ns_reader.rs
+++ b/src/reader/ns_reader.rs
@@ -337,7 +337,6 @@ impl<R> NsReader<R> {
     /// ```
     /// # use pretty_assertions::assert_eq;
     /// use quick_xml::events::Event;
-    /// use quick_xml::events::attributes::Attribute;
     /// use quick_xml::name::{Namespace, QName, ResolveResult::*};
     /// use quick_xml::reader::NsReader;
     ///
diff --git a/src/writer.rs b/src/writer.rs
index 7dec0cc..81b7df3 100644
--- a/src/writer.rs
+++ b/src/writer.rs
@@ -51,7 +51,7 @@ use {crate::de::DeError, serde::Serialize};
 ///         },
 ///         Ok(Event::Eof) => break,
 ///         // we can either move or borrow the event to write, depending on your use-case
-///         Ok(e) => assert!(writer.write_event(e).is_ok()),
+///         Ok(e) => assert!(writer.write_event(e.borrow()).is_ok()),
 ///         Err(e) => panic!("Error at position {}: {:?}", reader.buffer_position(), e),
 ///     }
 /// }
@@ -192,37 +192,37 @@ impl<W: Write> Writer<W> {
     }
 
     /// Writes the given event to the underlying writer.
-    pub fn write_event<'a, E: AsRef<Event<'a>>>(&mut self, event: E) -> Result<()> {
+    pub fn write_event<'a, E: Into<Event<'a>>>(&mut self, event: E) -> Result<()> {
         let mut next_should_line_break = true;
-        let result = match *event.as_ref() {
-            Event::Start(ref e) => {
-                let result = self.write_wrapped(b"<", e, b">");
+        let result = match event.into() {
+            Event::Start(e) => {
+                let result = self.write_wrapped(b"<", &e, b">");
                 if let Some(i) = self.indent.as_mut() {
                     i.grow();
                 }
                 result
             }
-            Event::End(ref e) => {
+            Event::End(e) => {
                 if let Some(i) = self.indent.as_mut() {
                     i.shrink();
                 }
-                self.write_wrapped(b"</", e, b">")
+                self.write_wrapped(b"</", &e, b">")
             }
-            Event::Empty(ref e) => self.write_wrapped(b"<", e, b"/>"),
-            Event::Text(ref e) => {
+            Event::Empty(e) => self.write_wrapped(b"<", &e, b"/>"),
+            Event::Text(e) => {
                 next_should_line_break = false;
-                self.write(e)
+                self.write(&e)
             }
-            Event::Comment(ref e) => self.write_wrapped(b"<!--", e, b"-->"),
-            Event::CData(ref e) => {
+            Event::Comment(e) => self.write_wrapped(b"<!--", &e, b"-->"),
+            Event::CData(e) => {
                 next_should_line_break = false;
                 self.write(b"<![CDATA[")?;
-                self.write(e)?;
+                self.write(&e)?;
                 self.write(b"]]>")
             }
-            Event::Decl(ref e) => self.write_wrapped(b"<?", e, b"?>"),
-            Event::PI(ref e) => self.write_wrapped(b"<?", e, b"?>"),
-            Event::DocType(ref e) => self.write_wrapped(b"<!DOCTYPE ", e, b">"),
+            Event::Decl(e) => self.write_wrapped(b"<?", &e, b"?>"),
+            Event::PI(e) => self.write_wrapped(b"<?", &e, b"?>"),
+            Event::DocType(e) => self.write_wrapped(b"<!DOCTYPE ", &e, b">"),
             Event::Eof => Ok(()),
         };
         if let Some(i) = self.indent.as_mut() {
diff --git a/src/writer/async_tokio.rs b/src/writer/async_tokio.rs
index dfecbe4..dab4c5b 100644
--- a/src/writer/async_tokio.rs
+++ b/src/writer/async_tokio.rs
@@ -9,37 +9,37 @@ use crate::{ElementWriter, Writer};
 
 impl<W: AsyncWrite + Unpin> Writer<W> {
     /// Writes the given event to the underlying writer. Async version of [`Writer::write_event`].
-    pub async fn write_event_async<'a, E: AsRef<Event<'a>>>(&mut self, event: E) -> Result<()> {
+    pub async fn write_event_async<'a, E: Into<Event<'a>>>(&mut self, event: E) -> Result<()> {
         let mut next_should_line_break = true;
-        let result = match *event.as_ref() {
-            Event::Start(ref e) => {
-                let result = self.write_wrapped_async(b"<", e, b">").await;
+        let result = match event.into() {
+            Event::Start(e) => {
+                let result = self.write_wrapped_async(b"<", &e, b">").await;
                 if let Some(i) = self.indent.as_mut() {
                     i.grow();
                 }
                 result
             }
-            Event::End(ref e) => {
+            Event::End(e) => {
                 if let Some(i) = self.indent.as_mut() {
                     i.shrink();
                 }
-                self.write_wrapped_async(b"</", e, b">").await
+                self.write_wrapped_async(b"</", &e, b">").await
             }
-            Event::Empty(ref e) => self.write_wrapped_async(b"<", e, b"/>").await,
-            Event::Text(ref e) => {
+            Event::Empty(e) => self.write_wrapped_async(b"<", &e, b"/>").await,
+            Event::Text(e) => {
                 next_should_line_break = false;
-                self.write_async(e).await
+                self.write_async(&e).await
             }
-            Event::Comment(ref e) => self.write_wrapped_async(b"<!--", e, b"-->").await,
-            Event::CData(ref e) => {
+            Event::Comment(e) => self.write_wrapped_async(b"<!--", &e, b"-->").await,
+            Event::CData(e) => {
                 next_should_line_break = false;
                 self.write_async(b"<![CDATA[").await?;
-                self.write_async(e).await?;
+                self.write_async(&e).await?;
                 self.write_async(b"]]>").await
             }
-            Event::Decl(ref e) => self.write_wrapped_async(b"<?", e, b"?>").await,
-            Event::PI(ref e) => self.write_wrapped_async(b"<?", e, b"?>").await,
-            Event::DocType(ref e) => self.write_wrapped_async(b"<!DOCTYPE ", e, b">").await,
+            Event::Decl(e) => self.write_wrapped_async(b"<?", &e, b"?>").await,
+            Event::PI(e) => self.write_wrapped_async(b"<?", &e, b"?>").await,
+            Event::DocType(e) => self.write_wrapped_async(b"<!DOCTYPE ", &e, b">").await,
             Event::Eof => Ok(()),
         };
         if let Some(i) = self.indent.as_mut() {


Test patch:
diff --git a/tests/fuzzing.rs b/tests/fuzzing.rs
index 4324253..deca382 100644
--- a/tests/fuzzing.rs
+++ b/tests/fuzzing.rs
@@ -30,9 +30,9 @@ fn fuzz_101() {
         match reader.read_event_into(&mut buf) {
             Ok(Event::Start(e)) | Ok(Event::Empty(e)) => {
                 for a in e.attributes() {
-                    if a.ok()
-                        .map_or(true, |a| a.decode_and_unescape_value(&reader).is_err())
-                    {
+                    if a.ok().map_or(true, |a| {
+                        a.decode_and_unescape_value(reader.decoder()).is_err()
+                    }) {
                         break;
                     }
                 }
diff --git a/tests/roundtrip.rs b/tests/roundtrip.rs
index 0f0ed0f..6872619 100644
--- a/tests/roundtrip.rs
+++ b/tests/roundtrip.rs
@@ -180,7 +180,7 @@ fn with_trim_ref() {
     loop {
         match reader.read_event().unwrap() {
             Eof => break,
-            e => assert!(writer.write_event(&e).is_ok()), // either `e` or `&e`
+            e => assert!(writer.write_event(e.borrow()).is_ok()), // either `e` or `&e`
         }
     }