From ea366c233434ebe13454206684231b12eebb5d49 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20Sch=C3=A4fer?= Date: Sat, 23 Apr 2022 15:29:03 +0200 Subject: [PATCH] Rip out quick-xml --- minidom/Cargo.toml | 3 +- minidom/src/element.rs | 184 +++++++++++++++------------------- minidom/src/error.rs | 19 ++-- minidom/src/lib.rs | 2 - minidom/src/node.rs | 19 ++-- minidom/src/tests.rs | 30 +++--- minidom/src/tree_builder.rs | 12 +-- parsers/src/iq.rs | 8 +- parsers/src/jingle.rs | 4 +- parsers/src/jingle_message.rs | 4 +- parsers/src/mix.rs | 16 +-- parsers/src/stanza_error.rs | 4 +- parsers/src/xhtml.rs | 2 +- tokio-xmpp/Cargo.toml | 2 +- tokio-xmpp/src/xmpp_codec.rs | 2 +- 15 files changed, 135 insertions(+), 176 deletions(-) diff --git a/minidom/Cargo.toml b/minidom/Cargo.toml index af8a1c8..cebdc75 100644 --- a/minidom/Cargo.toml +++ b/minidom/Cargo.toml @@ -21,5 +21,4 @@ edition = "2018" gitlab = { repository = "xmpp-rs/xmpp-rs" } [dependencies] -quick-xml = "0.22.0" -rxml = "^0.7.1" +rxml = "^0.8.0" diff --git a/minidom/src/element.rs b/minidom/src/element.rs index d3f4d1c..ea2e3a1 100644 --- a/minidom/src/element.rs +++ b/minidom/src/element.rs @@ -20,19 +20,58 @@ use crate::prefixes::{Namespace, Prefix, Prefixes}; use crate::tree_builder::TreeBuilder; use std::collections::{btree_map, BTreeMap}; +use std::convert::{TryFrom, TryInto}; use std::io::{BufRead, Write}; +use std::sync::Arc; use std::borrow::Cow; use std::str; -use quick_xml::events::{BytesDecl, BytesEnd, BytesStart, Event}; -use quick_xml::Writer as EventWriter; -use rxml::{EventRead, Lexer, PullDriver, RawParser}; +use rxml::writer::{Encoder, Item, TrackNamespace}; +use rxml::{EventRead, Lexer, PullDriver, RawParser, XmlVersion}; use std::str::FromStr; use std::slice; +fn encode_and_write( + item: Item<'_>, + enc: &mut Encoder, + mut w: W, +) -> rxml::Result<()> { + let mut buf = rxml::bytes::BytesMut::new(); + enc.encode_into_bytes(item, &mut buf) + .expect("encoder driven incorrectly"); + w.write_all(&buf[..])?; + Ok(()) +} + +/// Wrapper around a [`std::io::Write`] and an [`rxml::writer::Encoder`], to +/// provide a simple function to write an rxml Item to a writer. +pub struct CustomItemWriter { + writer: W, + encoder: Encoder, +} + +impl CustomItemWriter { + pub(crate) fn new(writer: W) -> Self { + Self { + writer, + encoder: Encoder::new(), + } + } +} + +impl CustomItemWriter { + pub(crate) fn write(&mut self, item: Item<'_>) -> rxml::Result<()> { + encode_and_write(item, &mut self.encoder, &mut self.writer) + } +} + +/// Type alias to simplify the use for the default namespace tracking +/// implementation. +pub type ItemWriter = CustomItemWriter; + /// helper function to escape a `&[u8]` and replace all /// xml special characters (<, >, &, ', ") with their corresponding /// xml escaped value. @@ -81,9 +120,6 @@ pub fn escape(raw: &[u8]) -> Cow<[u8]> { pub struct Element { name: String, namespace: String, - /// This is only used when deserializing. If you have to use a custom prefix use - /// `ElementBuilder::prefix`. - pub(crate) prefix: Option, /// Namespace declarations pub prefixes: Prefixes, attributes: BTreeMap, @@ -123,7 +159,6 @@ impl Element { pub(crate) fn new>( name: String, namespace: String, - prefix: Option, prefixes: P, attributes: BTreeMap, children: Vec, @@ -131,7 +166,6 @@ impl Element { Element { name, namespace, - prefix, prefixes: prefixes.into(), attributes, children, @@ -162,7 +196,6 @@ impl Element { name.as_ref().to_string(), namespace.into(), None, - None, BTreeMap::new(), Vec::new(), ), @@ -188,7 +221,6 @@ impl Element { name.into(), namespace.into(), None, - None, BTreeMap::new(), Vec::new(), ) @@ -316,119 +348,66 @@ impl Element { /// Output a document to a `Writer`. pub fn write_to(&self, writer: &mut W) -> Result<()> { - self.to_writer(&mut EventWriter::new(writer)) + self.to_writer(&mut ItemWriter::new(writer)) } /// Output a document to a `Writer`. pub fn write_to_decl(&self, writer: &mut W) -> Result<()> { - self.to_writer_decl(&mut EventWriter::new(writer)) + self.to_writer_decl(&mut ItemWriter::new(writer)) } /// Output the document to quick-xml `Writer` - pub fn to_writer(&self, writer: &mut EventWriter) -> Result<()> { - self.write_to_inner(writer, &mut BTreeMap::new()) + pub fn to_writer(&self, writer: &mut ItemWriter) -> Result<()> { + self.write_to_inner(writer) } /// Output the document to quick-xml `Writer` - pub fn to_writer_decl(&self, writer: &mut EventWriter) -> Result<()> { - writer.write_event(Event::Decl(BytesDecl::new(b"1.0", Some(b"utf-8"), None)))?; - self.write_to_inner(writer, &mut BTreeMap::new()) + pub fn to_writer_decl(&self, writer: &mut ItemWriter) -> Result<()> { + writer + .write(Item::XmlDeclaration(XmlVersion::V1_0)) + .unwrap(); // TODO: error return + self.write_to_inner(writer) } /// Like `write_to()` but without the `` prelude - pub fn write_to_inner( - &self, - writer: &mut EventWriter, - all_prefixes: &mut BTreeMap, - ) -> Result<()> { - let local_prefixes: &BTreeMap, String> = self.prefixes.declared_prefixes(); - - // Element namespace - // If the element prefix hasn't been set yet via a custom prefix, add it. - let mut existing_self_prefix: Option> = None; - for (prefix, ns) in local_prefixes.iter().chain(all_prefixes.iter()) { - if ns == &self.namespace { - existing_self_prefix = Some(prefix.clone()); - } + pub fn write_to_inner(&self, writer: &mut ItemWriter) -> Result<()> { + for (prefix, namespace) in self.prefixes.declared_prefixes() { + assert!(writer.encoder.inner_mut().declare_fixed( + prefix.as_ref().map(|x| (&**x).try_into()).transpose()?, + Some(Arc::new(namespace.clone().try_into()?)) + )); } - let mut all_keys = all_prefixes.keys().cloned(); - let mut local_keys = local_prefixes.keys().cloned(); - let self_prefix: (Option, bool) = match existing_self_prefix { - // No prefix exists already for our namespace - None => { - if !local_keys.any(|p| p.is_none()) { - // Use the None prefix if available - (None, true) - } else { - // Otherwise generate one. Check if it isn't already used, if so increase the - // number until we find a suitable one. - let mut prefix_n = 0u8; - while all_keys.any(|p| p == Some(format!("ns{}", prefix_n))) { - prefix_n += 1; - } - (Some(format!("ns{}", prefix_n)), true) - } - } - // Some prefix has already been declared (or is going to be) for our namespace. We - // don't need to declare a new one. We do however need to remember which one to use in - // the tag name. - Some(prefix) => (prefix, false), + let namespace = if self.namespace.len() == 0 { + None + } else { + Some(Arc::new(self.namespace.clone().try_into()?)) }; + writer.write(Item::ElementHeadStart(namespace, (*self.name).try_into()?))?; - let name = match self_prefix { - (Some(ref prefix), _) => Cow::Owned(format!("{}:{}", prefix, self.name)), - _ => Cow::Borrowed(&self.name), - }; - let mut start = BytesStart::borrowed(name.as_bytes(), name.len()); + for (key, value) in self.attributes.iter() { + let (prefix, name) = <&rxml::NameStr>::try_from(&**key) + .unwrap() + .split_name() + .unwrap(); + let namespace = match prefix { + Some(prefix) => match writer.encoder.inner().lookup_prefix(Some(prefix)) { + Ok(v) => Some(v), + Err(rxml::writer::PrefixError::Undeclared) => return Err(Error::InvalidPrefix), + }, + None => None, + }; + writer.write(Item::Attribute(namespace, name, (&**value).try_into()?))?; + } - // Write self prefix if necessary - match self_prefix { - (Some(ref p), true) => { - let key = format!("xmlns:{}", p); - start.push_attribute((key.as_bytes(), self.namespace.as_bytes())); - all_prefixes.insert(self_prefix.0, self.namespace.clone()); - } - (None, true) => { - let key = String::from("xmlns"); - start.push_attribute((key.as_bytes(), self.namespace.as_bytes())); - all_prefixes.insert(self_prefix.0, self.namespace.clone()); - } - _ => (), - }; - - // Custom prefixes/namespace sets - for (prefix, ns) in local_prefixes { - match all_prefixes.get(prefix) { - p @ Some(_) if p == prefix.as_ref() => (), - _ => { - let key = match prefix { - None => String::from("xmlns"), - Some(p) => format!("xmlns:{}", p), - }; - - start.push_attribute((key.as_bytes(), ns.as_ref())); - all_prefixes.insert(prefix.clone(), ns.clone()); - } + if !self.children.is_empty() { + writer.write(Item::ElementHeadEnd)?; + for child in self.children.iter() { + child.write_to_inner(writer)?; } } + writer.write(Item::ElementFoot)?; - for (key, value) in &self.attributes { - start.push_attribute((key.as_bytes(), escape(value.as_bytes()).as_ref())); - } - - if self.children.is_empty() { - writer.write_event(Event::Empty(start))?; - return Ok(()); - } - - writer.write_event(Event::Start(start))?; - - for child in &self.children { - child.write_to_inner(writer, &mut all_prefixes.clone())?; - } - - writer.write_event(Event::End(BytesEnd::borrowed(name.as_bytes())))?; Ok(()) } @@ -888,7 +867,6 @@ mod tests { let elem = Element::new( "name".to_owned(), "namespace".to_owned(), - None, (None, "namespace".to_owned()), BTreeMap::from_iter(vec![("name".to_string(), "value".to_string())].into_iter()), Vec::new(), diff --git a/minidom/src/error.rs b/minidom/src/error.rs index ba23d25..0ccfd6d 100644 --- a/minidom/src/error.rs +++ b/minidom/src/error.rs @@ -17,11 +17,8 @@ use std::error::Error as StdError; /// Our main error type. #[derive(Debug)] pub enum Error { - /// An error from quick_xml. - XmlError(::quick_xml::Error), - - /// Error from rxml parsing - ParserError(rxml::Error), + /// Error from rxml parsing or writing + XmlError(rxml::Error), /// An error which is returned when the end of the document was reached prematurely. EndOfDocument, @@ -41,7 +38,6 @@ impl StdError for Error { fn cause(&self) -> Option<&dyn StdError> { match self { Error::XmlError(e) => Some(e), - Error::ParserError(e) => Some(e), Error::EndOfDocument => None, Error::InvalidPrefix => None, Error::MissingNamespace => None, @@ -54,7 +50,6 @@ impl std::fmt::Display for Error { fn fmt(&self, fmt: &mut std::fmt::Formatter) -> std::fmt::Result { match self { Error::XmlError(e) => write!(fmt, "XML error: {}", e), - Error::ParserError(e) => write!(fmt, "XML parser error: {}", e), Error::EndOfDocument => { write!(fmt, "the end of the document has been reached prematurely") } @@ -65,15 +60,15 @@ impl std::fmt::Display for Error { } } -impl From<::quick_xml::Error> for Error { - fn from(err: ::quick_xml::Error) -> Error { +impl From for Error { + fn from(err: rxml::Error) -> Error { Error::XmlError(err) } } -impl From for Error { - fn from(err: rxml::Error) -> Error { - Error::ParserError(err) +impl From for Error { + fn from(err: rxml::error::XmlError) -> Error { + Error::XmlError(err.into()) } } diff --git a/minidom/src/lib.rs b/minidom/src/lib.rs index 21aa8b4..382b104 100644 --- a/minidom/src/lib.rs +++ b/minidom/src/lib.rs @@ -75,8 +75,6 @@ //! minidom = "*" //! ``` -pub use quick_xml; - pub mod convert; pub mod element; pub mod error; diff --git a/minidom/src/node.rs b/minidom/src/node.rs index 5d827e0..1ea5063 100644 --- a/minidom/src/node.rs +++ b/minidom/src/node.rs @@ -8,14 +8,13 @@ //! Provides the `Node` struct, which represents a node in the DOM. -use crate::element::{Element, ElementBuilder}; +use crate::element::{Element, ElementBuilder, ItemWriter}; use crate::error::Result; -use std::collections::BTreeMap; -use std::io::Write; +use rxml::writer::Item; -use quick_xml::events::{BytesText, Event}; -use quick_xml::Writer as EventWriter; +use std::convert::TryInto; +use std::io::Write; /// A node in an element tree. #[derive(Clone, Debug, Eq)] @@ -160,15 +159,11 @@ impl Node { } #[doc(hidden)] - pub(crate) fn write_to_inner( - &self, - writer: &mut EventWriter, - prefixes: &mut BTreeMap, String>, - ) -> Result<()> { + pub(crate) fn write_to_inner(&self, writer: &mut ItemWriter) -> Result<()> { match *self { - Node::Element(ref elmt) => elmt.write_to_inner(writer, prefixes)?, + Node::Element(ref elmt) => elmt.write_to_inner(writer)?, Node::Text(ref s) => { - writer.write_event(Event::Text(BytesText::from_plain_str(s)))?; + writer.write(Item::Text((&**s).try_into()?))?; } } diff --git a/minidom/src/tests.rs b/minidom/src/tests.rs index 63322e9..d209c0b 100644 --- a/minidom/src/tests.rs +++ b/minidom/src/tests.rs @@ -13,7 +13,7 @@ use crate::element::Element; use crate::error::Error; -const TEST_STRING: &'static [u8] = br#"meownya"#; +const TEST_STRING: &'static [u8] = br#"meownya"#; fn build_test_tree() -> Element { let mut root = Element::builder("root", "root_ns") @@ -151,7 +151,7 @@ fn writer_with_decl_works() { root.write_to_decl(&mut writer).unwrap(); } let result = format!( - r#"{}"#, + "\n{}", String::from_utf8(TEST_STRING.to_owned()).unwrap() ); assert_eq!(String::from_utf8(writer).unwrap(), result); @@ -167,7 +167,7 @@ fn writer_with_prefix() { .build(); assert_eq!( String::from(&root), - r#""#, + r#""#, ); } @@ -177,7 +177,7 @@ fn writer_no_prefix_namespace() { // TODO: Note that this isn't exactly equal to a None prefix. it's just that the None prefix is // the most obvious when it's not already used. Maybe fix tests so that it only checks that the // prefix used equals the one declared for the namespace. - assert_eq!(String::from(&root), r#""#); + assert_eq!(String::from(&root), r#""#); } #[test] @@ -185,7 +185,7 @@ fn writer_no_prefix_namespace_child() { let child = Element::builder("child", "ns1").build(); let root = Element::builder("root", "ns1").append(child).build(); // TODO: Same remark as `writer_no_prefix_namespace`. - assert_eq!(String::from(&root), r#""#); + assert_eq!(String::from(&root), r#""#); let child = Element::builder("child", "ns2") .prefix(None, "ns3") @@ -195,7 +195,7 @@ fn writer_no_prefix_namespace_child() { // TODO: Same remark as `writer_no_prefix_namespace`. assert_eq!( String::from(&root), - r#""# + r#""# ); } @@ -209,7 +209,7 @@ fn writer_prefix_namespace_child() { .build(); assert_eq!( String::from(&root), - r#""# + r#""# ); } @@ -227,7 +227,7 @@ fn writer_with_prefix_deduplicate() { .build(); assert_eq!( String::from(&root), - r#""#, + r#""#, ); // Ensure descendants don't just reuse ancestors' prefixes that have been shadowed in between @@ -236,7 +236,7 @@ fn writer_with_prefix_deduplicate() { let root = Element::builder("root", "ns1").append(child).build(); assert_eq!( String::from(&root), - r#""#, + r#""#, ); } @@ -251,7 +251,7 @@ fn writer_escapes_attributes() { } assert_eq!( String::from_utf8(writer).unwrap(), - r#""# + r#""# ); } @@ -264,7 +264,7 @@ fn writer_escapes_text() { } assert_eq!( String::from_utf8(writer).unwrap(), - r#"<3"# + r#"<3"# ); } @@ -431,15 +431,15 @@ fn fail_comments() { #[test] fn xml_error() { match "".parse::() { - Err(crate::error::Error::ParserError(rxml::Error::NotWellFormed( - rxml::error::WFError::ElementMismatch, + Err(crate::error::Error::XmlError(rxml::Error::Xml( + rxml::error::XmlError::ElementMismatch, ))) => (), err => panic!("No or wrong error: {:?}", err), } match "() { - Err(crate::error::Error::ParserError(rxml::Error::NotWellFormed( - rxml::error::WFError::InvalidEof(_), + Err(crate::error::Error::XmlError(rxml::Error::Xml( + rxml::error::XmlError::InvalidEof(_), ))) => (), err => panic!("No or wrong error: {:?}", err), } diff --git a/minidom/src/tree_builder.rs b/minidom/src/tree_builder.rs index 538fab6..aed524e 100644 --- a/minidom/src/tree_builder.rs +++ b/minidom/src/tree_builder.rs @@ -89,7 +89,7 @@ impl TreeBuilder { /// Process a Event that you got out of a RawParser pub fn process_event(&mut self, event: RawEvent) -> Result<(), Error> { match event { - RawEvent::XMLDeclaration(_, _) => {} + RawEvent::XmlDeclaration(_, _) => {} RawEvent::ElementHeadOpen(_, (prefix, name)) => { self.next_tag = Some(( @@ -132,14 +132,8 @@ impl TreeBuilder { .lookup_prefix(&prefix.clone().map(|prefix| prefix.as_str().to_owned())) .ok_or(Error::MissingNamespace)? .to_owned(); - let el = Element::new( - name.as_str().to_owned(), - namespace, - Some(prefix.map(|prefix| prefix.as_str().to_owned())), - prefixes, - attrs, - vec![], - ); + let el = + Element::new(name.as_str().to_owned(), namespace, prefixes, attrs, vec![]); self.stack.push(el); } } diff --git a/parsers/src/iq.rs b/parsers/src/iq.rs index b9c022f..4e63649 100644 --- a/parsers/src/iq.rs +++ b/parsers/src/iq.rs @@ -233,15 +233,15 @@ mod tests { #[cfg(target_pointer_width = "32")] #[test] fn test_size() { - assert_size!(IqType, 136); - assert_size!(Iq, 228); + assert_size!(IqType, 120); + assert_size!(Iq, 212); } #[cfg(target_pointer_width = "64")] #[test] fn test_size() { - assert_size!(IqType, 272); - assert_size!(Iq, 456); + assert_size!(IqType, 240); + assert_size!(Iq, 424); } #[test] diff --git a/parsers/src/jingle.rs b/parsers/src/jingle.rs index f4d8400..1a96469 100644 --- a/parsers/src/jingle.rs +++ b/parsers/src/jingle.rs @@ -689,7 +689,7 @@ mod tests { assert_size!(Senders, 1); assert_size!(Disposition, 1); assert_size!(ContentId, 12); - assert_size!(Content, 252); + assert_size!(Content, 228); assert_size!(Reason, 1); assert_size!(ReasonElement, 16); assert_size!(SessionId, 12); @@ -704,7 +704,7 @@ mod tests { assert_size!(Senders, 1); assert_size!(Disposition, 1); assert_size!(ContentId, 24); - assert_size!(Content, 504); + assert_size!(Content, 456); assert_size!(Reason, 1); assert_size!(ReasonElement, 32); assert_size!(SessionId, 24); diff --git a/parsers/src/jingle_message.rs b/parsers/src/jingle_message.rs index bfc8abb..69221e7 100644 --- a/parsers/src/jingle_message.rs +++ b/parsers/src/jingle_message.rs @@ -110,13 +110,13 @@ mod tests { #[cfg(target_pointer_width = "32")] #[test] fn test_size() { - assert_size!(JingleMI, 92); + assert_size!(JingleMI, 76); } #[cfg(target_pointer_width = "64")] #[test] fn test_size() { - assert_size!(JingleMI, 184); + assert_size!(JingleMI, 152); } #[test] diff --git a/parsers/src/mix.rs b/parsers/src/mix.rs index 2ca49f4..135a650 100644 --- a/parsers/src/mix.rs +++ b/parsers/src/mix.rs @@ -346,46 +346,46 @@ mod tests { fn serialise() { let elem: Element = Join::from_nick_and_nodes("coucou", &["foo", "bar"]).into(); let xml = String::from(&elem); - assert_eq!(xml, "coucou"); + assert_eq!(xml, "coucou"); let elem: Element = UpdateSubscription::from_nodes(&["foo", "bar"]).into(); let xml = String::from(&elem); - assert_eq!(xml, ""); + assert_eq!(xml, ""); let elem: Element = Leave.into(); let xml = String::from(&elem); - assert_eq!(xml, ""); + assert_eq!(xml, ""); let elem: Element = SetNick::new("coucou").into(); let xml = String::from(&elem); assert_eq!( xml, - "coucou" + "coucou" ); let elem: Element = Mix::new("coucou", "coucou@example").into(); let xml = String::from(&elem); assert_eq!( xml, - "coucoucoucou@example" + "coucoucoucou@example" ); let elem: Element = Create::new().into(); let xml = String::from(&elem); - assert_eq!(xml, ""); + assert_eq!(xml, ""); let elem: Element = Create::from_channel_id("coucou").into(); let xml = String::from(&elem); assert_eq!( xml, - "" + "" ); let elem: Element = Destroy::new("coucou").into(); let xml = String::from(&elem); assert_eq!( xml, - "" + "" ); } } diff --git a/parsers/src/stanza_error.rs b/parsers/src/stanza_error.rs index 865577e..9a2604e 100644 --- a/parsers/src/stanza_error.rs +++ b/parsers/src/stanza_error.rs @@ -318,7 +318,7 @@ mod tests { fn test_size() { assert_size!(ErrorType, 1); assert_size!(DefinedCondition, 1); - assert_size!(StanzaError, 132); + assert_size!(StanzaError, 116); } #[cfg(target_pointer_width = "64")] @@ -326,7 +326,7 @@ mod tests { fn test_size() { assert_size!(ErrorType, 1); assert_size!(DefinedCondition, 1); - assert_size!(StanzaError, 264); + assert_size!(StanzaError, 232); } #[test] diff --git a/parsers/src/xhtml.rs b/parsers/src/xhtml.rs index 2ce1892..7b2112f 100644 --- a/parsers/src/xhtml.rs +++ b/parsers/src/xhtml.rs @@ -596,7 +596,7 @@ mod tests { assert_eq!(html, "Hello world!"); let elem = Element::from(parsed2); - assert_eq!(String::from(&elem), "Hello world!"); + assert_eq!(String::from(&elem), "Hello world!"); } #[test] diff --git a/tokio-xmpp/Cargo.toml b/tokio-xmpp/Cargo.toml index 4818ce6..30da7fb 100644 --- a/tokio-xmpp/Cargo.toml +++ b/tokio-xmpp/Cargo.toml @@ -27,7 +27,7 @@ trust-dns-proto = "0.20" trust-dns-resolver = "0.20" xmpp-parsers = "0.19" minidom = "0.14" -rxml = "^0.7.1" +rxml = "^0.8.0" webpki-roots = { version = "0.22", optional = true } [build-dependencies] diff --git a/tokio-xmpp/src/xmpp_codec.rs b/tokio-xmpp/src/xmpp_codec.rs index 028dcbd..c3f2dea 100644 --- a/tokio-xmpp/src/xmpp_codec.rs +++ b/tokio-xmpp/src/xmpp_codec.rs @@ -336,7 +336,7 @@ mod tests { assert_eq!( framed.get_ref().get_ref(), &format!( - "{}", + "{}", text ) .as_bytes()