From 3d71e37e0c1c802013cda8d85737b76a9d896568 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maxime=20=E2=80=9Cpep=E2=80=9D=20Buquet?= Date: Mon, 30 Mar 2020 15:12:36 +0200 Subject: [PATCH] minidom: Ensure there is no colon in name when creating element MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Maxime “pep” Buquet --- minidom-rs/src/element.rs | 17 +++++++---- minidom-rs/src/error.rs | 2 +- minidom-rs/src/tests.rs | 62 +++++++++++++++++++++++---------------- 3 files changed, 49 insertions(+), 32 deletions(-) diff --git a/minidom-rs/src/element.rs b/minidom-rs/src/element.rs index 7c4bc55..00e92a9 100644 --- a/minidom-rs/src/element.rs +++ b/minidom-rs/src/element.rs @@ -123,6 +123,14 @@ impl PartialEq for Element { } } +fn ensure_no_prefix>(s: &S) -> Result<()> { + let name_parts = s.as_ref().split(':').collect::>(); + match name_parts.len() { + 1 => Ok(()), + _ => Err(Error::InvalidElement), + } +} + impl Element { fn new>( name: String, @@ -132,8 +140,8 @@ impl Element { attributes: BTreeMap, children: Vec, ) -> Element { - // TODO: Check that "name" doesn't contain ":". We've stopped accepting the "prefix:local" - // format. + ensure_no_prefix(&name).unwrap(); + // TODO: Return Result instead. Element { name, namespace, @@ -863,7 +871,7 @@ fn build_element(reader: &EventReader, event: &BytesStart, prefix } }; - let element = Element::new( + Ok(Element::new( name, namespace.clone(), // Note that this will always be Some(_) as we can't distinguish between the None case and @@ -872,8 +880,7 @@ fn build_element(reader: &EventReader, event: &BytesStart, prefix local_prefixes, attributes, Vec::new() - ); - Ok(element) + )) } /// An iterator over references to child elements of an `Element`. diff --git a/minidom-rs/src/error.rs b/minidom-rs/src/error.rs index 9a0e0e2..2510f4b 100644 --- a/minidom-rs/src/error.rs +++ b/minidom-rs/src/error.rs @@ -32,7 +32,7 @@ pub enum Error { /// An error which is returned when an element is closed when it shouldn't be InvalidElementClosed, - /// An error which is returned when an elemet's name contains more than one colon + /// An error which is returned when an elemet's name contains more colons than permitted InvalidElement, /// An error which is returned when an element being serialized doesn't contain a prefix diff --git a/minidom-rs/src/tests.rs b/minidom-rs/src/tests.rs index 3ba15a9..6190e65 100644 --- a/minidom-rs/src/tests.rs +++ b/minidom-rs/src/tests.rs @@ -77,8 +77,13 @@ fn test_real_data() { .append(correction) .build(); let stream = Element::builder("stream", "http://etherx.jabber.org/streams") - .prefix(Some(String::from("stream")), "http://etherx.jabber.org/streams").unwrap() - .prefix(None, "jabber:client").unwrap() + .prefix( + Some(String::from("stream")), + "http://etherx.jabber.org/streams", + ) + .unwrap() + .prefix(None, "jabber:client") + .unwrap() .append(message) .build(); println!("{}", String::from(&stream)); @@ -109,8 +114,13 @@ fn test_real_data() { .append(pubsub) .build(); let stream = Element::builder("stream", "http://etherx.jabber.org/streams") - .prefix(Some(String::from("stream")), "http://etherx.jabber.org/streams").unwrap() - .prefix(None, "jabber:client").unwrap() + .prefix( + Some(String::from("stream")), + "http://etherx.jabber.org/streams", + ) + .unwrap() + .prefix(None, "jabber:client") + .unwrap() .append(iq) .build(); @@ -141,8 +151,10 @@ fn writer_with_decl_works() { #[test] fn writer_with_prefix() { let root = Element::builder("root", "ns1") - .prefix(Some(String::from("p1")), "ns1").unwrap() - .prefix(None, "ns2").unwrap() + .prefix(Some(String::from("p1")), "ns1") + .unwrap() + .prefix(None, "ns2") + .unwrap() .build(); assert_eq!(String::from(&root), r#""#, @@ -161,18 +173,15 @@ fn writer_no_prefix_namespace() { #[test] fn writer_no_prefix_namespace_child() { let child = Element::builder("child", "ns1").build(); - let root = Element::builder("root", "ns1") - .append(child) - .build(); + let root = Element::builder("root", "ns1").append(child).build(); // TODO: Same remark as `writer_no_prefix_namespace`. assert_eq!(String::from(&root), r#""#); let child = Element::builder("child", "ns2") - .prefix(None, "ns3").unwrap() - .build(); - let root = Element::builder("root", "ns1") - .append(child) + .prefix(None, "ns3") + .unwrap() .build(); + let root = Element::builder("root", "ns1").append(child).build(); // TODO: Same remark as `writer_no_prefix_namespace`. assert_eq!(String::from(&root), r#""#); } @@ -181,7 +190,8 @@ fn writer_no_prefix_namespace_child() { fn writer_prefix_namespace_child() { let child = Element::builder("child", "ns1").build(); let root = Element::builder("root", "ns1") - .prefix(Some(String::from("p1")), "ns1").unwrap() + .prefix(Some(String::from("p1")), "ns1") + .unwrap() .append(child) .build(); assert_eq!(String::from(&root), r#""#); @@ -193,8 +203,10 @@ fn writer_with_prefix_deduplicate() { // .prefix(Some(String::from("p1")), "ns1") .build(); let root = Element::builder("root", "ns1") - .prefix(Some(String::from("p1")), "ns1").unwrap() - .prefix(None, "ns2").unwrap() + .prefix(Some(String::from("p1")), "ns1") + .unwrap() + .prefix(None, "ns2") + .unwrap() .append(child) .build(); assert_eq!(String::from(&root), @@ -202,22 +214,20 @@ fn writer_with_prefix_deduplicate() { ); // Ensure descendants don't just reuse ancestors' prefixes that have been shadowed in between - let grandchild = Element::builder("grandchild", "ns1") - .build(); - let child = Element::builder("child", "ns2") - .append(grandchild) - .build(); - let root = Element::builder("root", "ns1") - .append(child) - .build(); - assert_eq!(String::from(&root), + let grandchild = Element::builder("grandchild", "ns1").build(); + let child = Element::builder("child", "ns2").append(grandchild).build(); + let root = Element::builder("root", "ns1").append(child).build(); + assert_eq!( + String::from(&root), r#""#, ); } #[test] fn writer_escapes_attributes() { - let root = Element::builder("root", "ns1").attr("a", "\"Air\" quotes").build(); + let root = Element::builder("root", "ns1") + .attr("a", "\"Air\" quotes") + .build(); let mut writer = Vec::new(); { root.write_to(&mut writer).unwrap();