More correctly normalize attribute values

Author: dralley

Changelog.md

@@ -22,6 +22,19 @@
   - `Deserializer::buffering_with_resolver`
 - [#878]: Add ability to serialize structs in `$value` fields. The struct name will
   be used as a tag name. Previously only enums was allowed there.
+- [#371]: Improved compliance with the XML attribute value normalization process by adding
+  - `Attribute::normalized_value()`
+  - `Attribute::normalized_value_with()`
+  - `Attribute::decoded_and_normalized_value()`
+  - `Attribute::decoded_and_normalized_value_with()`
+
+  which ought to be used in place of deprecated
+  - `Attribute::unescape_value()`
+  - `Attribute::unescape_value_with()`
+  - `Attribute::decode_and_unescape_value()`
+  - `Attribute::decode_and_unescape_value_with()`
+
+  Deprecated functions now behaves the same as newly added.
 
 ### Bug Fixes
 

benches/macrobenches.rs

@@ -44,14 +44,13 @@ static INPUTS: &[(&str, &str)] = &[
     ("players.xml", PLAYERS),
 ];
 
-// TODO: use fully normalized attribute values
 fn parse_document_from_str(doc: &str) -> XmlResult<()> {
     let mut r = Reader::from_str(doc);
     loop {
         match black_box(r.read_event()?) {
             Event::Start(e) | Event::Empty(e) => {
                 for attr in e.attributes() {
-                    black_box(attr?.decode_and_unescape_value(r.decoder())?);
+                    black_box(attr?.decoded_and_normalized_value(r.decoder())?);
                 }
             }
             Event::Text(e) => {
@@ -68,15 +67,14 @@ fn parse_document_from_str(doc: &str) -> XmlResult<()> {
     Ok(())
 }
 
-// TODO: use fully normalized attribute values
 fn parse_document_from_bytes(doc: &[u8]) -> XmlResult<()> {
     let mut r = Reader::from_reader(doc);
     let mut buf = Vec::new();
     loop {
         match black_box(r.read_event_into(&mut buf)?) {
             Event::Start(e) | Event::Empty(e) => {
                 for attr in e.attributes() {
-                    black_box(attr?.decode_and_unescape_value(r.decoder())?);
+                    black_box(attr?.decoded_and_normalized_value(r.decoder())?);
                 }
             }
             Event::Text(e) => {
@@ -94,15 +92,14 @@ fn parse_document_from_bytes(doc: &[u8]) -> XmlResult<()> {
     Ok(())
 }
 
-// TODO: use fully normalized attribute values
 fn parse_document_from_str_with_namespaces(doc: &str) -> XmlResult<()> {
     let mut r = NsReader::from_str(doc);
     loop {
         match black_box(r.read_resolved_event()?) {
             (resolved_ns, Event::Start(e) | Event::Empty(e)) => {
                 black_box(resolved_ns);
                 for attr in e.attributes() {
-                    black_box(attr?.decode_and_unescape_value(r.decoder())?);
+                    black_box(attr?.decoded_and_normalized_value(r.decoder())?);
                 }
             }
             (resolved_ns, Event::Text(e)) => {
@@ -121,7 +118,6 @@ fn parse_document_from_str_with_namespaces(doc: &str) -> XmlResult<()> {
     Ok(())
 }
 
-// TODO: use fully normalized attribute values
 fn parse_document_from_bytes_with_namespaces(doc: &[u8]) -> XmlResult<()> {
     let mut r = NsReader::from_reader(doc);
     let mut buf = Vec::new();
@@ -130,7 +126,7 @@ fn parse_document_from_bytes_with_namespaces(doc: &[u8]) -> XmlResult<()> {
             (resolved_ns, Event::Start(e) | Event::Empty(e)) => {
                 black_box(resolved_ns);
                 for attr in e.attributes() {
-                    black_box(attr?.decode_and_unescape_value(r.decoder())?);
+                    black_box(attr?.decoded_and_normalized_value(r.decoder())?);
                 }
             }
             (resolved_ns, Event::Text(e)) => {

benches/microbenches.rs

@@ -243,6 +243,50 @@ fn attributes(c: &mut Criterion) {
             assert_eq!(count, 150);
         })
     });
+
+    group.finish();
+}
+
+/// Benchmarks normalizing attribute values
+fn attribute_value_normalization(c: &mut Criterion) {
+    let mut group = c.benchmark_group("attribute_value_normalization");
+
+    group.bench_function("noop_short", |b| {
+        b.iter(|| {
+            black_box(unescape("foobar")).unwrap();
+        })
+    });
+
+    group.bench_function("noop_long", |b| {
+        b.iter(|| {
+            black_box(unescape("just a bit of text without any entities")).unwrap();
+        })
+    });
+
+    group.bench_function("replacement_chars", |b| {
+        b.iter(|| {
+            black_box(unescape("just a bit\n of text without\tany entities")).unwrap();
+        })
+    });
+
+    group.bench_function("char_reference", |b| {
+        b.iter(|| {
+            let text = "prefix "some stuff","more stuff"";
+            black_box(unescape(text)).unwrap();
+            let text = "&<";
+            black_box(unescape(text)).unwrap();
+        })
+    });
+
+    group.bench_function("entity_reference", |b| {
+        b.iter(|| {
+            let text = "age > 72 && age < 21";
+            black_box(unescape(text)).unwrap();
+            let text = ""what's that?"";
+            black_box(unescape(text)).unwrap();
+        })
+    });
+
     group.finish();
 }
 
@@ -355,6 +399,7 @@ criterion_group!(
     read_resolved_event_into,
     one_event,
     attributes,
+    attribute_value_normalization,
     escaping,
     unescaping,
 );

examples/custom_entities.rs

@@ -154,7 +154,8 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
         let label = attrs.next().unwrap()?;
         assert_eq!(label.key, QName(b"label"));
         assert_eq!(
-            label.decode_and_unescape_value_with(reader.decoder(), |ent| reader.get_entity(ent))?,
+            label
+                .decoded_and_normalized_value_with(reader.decoder(), 9, |e| reader.get_entity(e))?,
             "Message: hello world"
         );
 
@@ -185,7 +186,7 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
         let attr = attrs.next().unwrap()?;
         assert_eq!(attr.key, QName(b"attr"));
         assert_eq!(
-            attr.decode_and_unescape_value_with(reader.decoder(), |ent| reader.get_entity(ent))?,
+            attr.decoded_and_normalized_value_with(reader.decoder(), 9, |e| reader.get_entity(e))?,
             "Message: hello world"
         );
 

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.decoder())?,
-                b"Tag" => tag = a.decode_and_unescape_value(reader.decoder())?,
+                b"Language" => lang = a.decoded_and_normalized_value(reader.decoder())?,
+                b"Tag" => tag = a.decoded_and_normalized_value(reader.decoder())?,
                 _ => (),
             }
         }
@@ -141,7 +141,7 @@ fn main() -> Result<(), AppError> {
                                             Ok::<Cow<'_, str>, Infallible>(std::borrow::Cow::from(""))
                                         })
                                         .unwrap().to_string();
-                                    let value = a.decode_and_unescape_value(reader.decoder()).or_else(|err| {
+                                    let value = a.decoded_and_normalized_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();

fuzz/fuzz_targets/fuzz_target_1.rs

@@ -34,7 +34,7 @@ where
                 debug_format!(e.name());
                 for a in e.attributes() {
                     debug_format!(a);
-                    if a.ok().map_or(false, |a| a.unescape_value().is_err()) {
+                    if a.ok().map_or(false, |a| a.normalized_value().is_err()) {
                         break;
                     }
                 }