Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove requirement to have Reader when decoding attributes and write anything converted to Event #760

Merged
merged 3 commits into from
Jun 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions Changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
8 changes: 4 additions & 4 deletions benches/macrobenches.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand All @@ -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) => {
Expand All @@ -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)) => {
Expand Down Expand Up @@ -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)) => {
Expand Down
2 changes: 1 addition & 1 deletion examples/custom_entities.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
6 changes: 3 additions & 3 deletions examples/read_nodes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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())?,
_ => (),
}
}
Expand Down Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion fuzz/fuzz_targets/fuzz_target_1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)) => {
Expand Down
2 changes: 1 addition & 1 deletion fuzz/fuzz_targets/structured_roundtrip.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
15 changes: 8 additions & 7 deletions src/events/attributes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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)? {
Expand Down
1 change: 0 additions & 1 deletion src/reader/ns_reader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
///
Expand Down
32 changes: 16 additions & 16 deletions src/writer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
/// }
/// }
Expand Down Expand Up @@ -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() {
Expand Down
30 changes: 15 additions & 15 deletions src/writer/async_tokio.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
6 changes: 3 additions & 3 deletions tests/fuzzing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand Down
2 changes: 1 addition & 1 deletion tests/roundtrip.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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`
}
}

Expand Down