Proper attribute value normalization

Changelog.md

@@ -18,6 +18,20 @@
 
 - [#938]: Add new enumeration `XmlVersion` and typified getter `BytesDecl::xml_version()`.
 - [#938]: Add new error variant `IllFormedError::UnknownVersion`.
+- [#371]: Add new error variant `EscapeError::TooManyNestedEntities`.
+- [#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
 
@@ -31,6 +45,7 @@
 - [#938]: Now `BytesText::xml_content`, `BytesCData::xml_content` and `BytesRef::xml_content`
   accepts `XmlVersion` parameter to apply correct EOL normalization rules.
 
+[#371]: https://github.com/tafia/quick-xml/issues/371
 [#914]: https://github.com/tafia/quick-xml/pull/914
 [#938]: https://github.com/tafia/quick-xml/pull/938
 

benches/macrobenches.rs

@@ -5,7 +5,7 @@
 use criterion::{self, criterion_group, criterion_main, Criterion, Throughput};
 use quick_xml::events::Event;
 use quick_xml::reader::{NsReader, Reader};
-use quick_xml::Result as XmlResult;
+use quick_xml::{Result as XmlResult, XmlVersion};
 use std::hint::black_box;
 
 static RPM_PRIMARY: &str = include_str!("../tests/documents/rpm_primary.xml");
@@ -48,14 +48,17 @@ 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);
+    let mut version = XmlVersion::V1_0;
     loop {
         match black_box(r.read_event()?) {
+            Event::Decl(e) => {
+                version = e.xml_version()?;
+            }
             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(version, r.decoder())?);
                 }
             }
             Event::Text(e) => {
@@ -72,15 +75,18 @@ 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 version = XmlVersion::V1_0;
     let mut buf = Vec::new();
     loop {
         match black_box(r.read_event_into(&mut buf)?) {
+            Event::Decl(e) => {
+                version = e.xml_version()?;
+            }
             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(version, r.decoder())?);
                 }
             }
             Event::Text(e) => {
@@ -98,15 +104,18 @@ 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);
+    let mut version = XmlVersion::V1_0;
     loop {
         match black_box(r.read_resolved_event()?) {
+            (_, Event::Decl(e)) => {
+                version = e.xml_version()?;
+            }
             (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(version, r.decoder())?);
                 }
             }
             (resolved_ns, Event::Text(e)) => {
@@ -125,16 +134,19 @@ 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 version = XmlVersion::V1_0;
     let mut buf = Vec::new();
     loop {
         match black_box(r.read_resolved_event_into(&mut buf)?) {
+            (_, Event::Decl(e)) => {
+                version = e.xml_version()?;
+            }
             (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(version, r.decoder())?);
                 }
             }
             (resolved_ns, Event::Text(e)) => {

benches/microbenches.rs

@@ -247,6 +247,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();
 }
 
@@ -359,6 +403,7 @@ criterion_group!(
     read_resolved_event_into,
     one_event,
     attributes,
+    attribute_value_normalization,
     escaping,
     unescaping,
 );

examples/custom_entities.rs

@@ -4,6 +4,7 @@
 //! - decode attribute values
 //!
 //! NB: this example is deliberately kept simple:
+//! * it assumes only XML 1.0 dialect (most widely used in the world)
 //! * it assumes that the XML file is UTF-8 encoded (custom_entities must only contain UTF-8 data)
 //! * it only handles internal entities;
 //! * the regex in this example is simple but brittle;
@@ -19,6 +20,7 @@ use quick_xml::escape::EscapeError;
 use quick_xml::events::{BytesEnd, BytesStart, BytesText, Event};
 use quick_xml::name::QName;
 use quick_xml::reader::Reader;
+use quick_xml::XmlVersion;
 use regex::bytes::Regex;
 
 use pretty_assertions::assert_eq;
@@ -154,7 +156,12 @@ 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(
+                XmlVersion::V1_0,
+                reader.decoder(),
+                9,
+                |e| reader.get_entity(e)
+            )?,
             "Message: hello world"
         );
 
@@ -185,7 +192,9 @@ 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(XmlVersion::V1_0, reader.decoder(), 9, |e| {
+                reader.get_entity(e)
+            })?,
             "Message: hello world"
         );
 

examples/read_nodes.rs

@@ -6,6 +6,7 @@ use quick_xml::events::attributes::AttrError;
 use quick_xml::events::{BytesStart, Event};
 use quick_xml::name::QName;
 use quick_xml::reader::Reader;
+use quick_xml::XmlVersion;
 use std::borrow::Cow;
 use std::collections::HashMap;
 use std::convert::Infallible;
@@ -70,8 +71,12 @@ 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(XmlVersion::V1_0, reader.decoder())?
+                }
+                b"Tag" => {
+                    tag = a.decoded_and_normalized_value(XmlVersion::V1_0, reader.decoder())?
+                }
                 _ => (),
             }
         }
@@ -141,7 +146,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(XmlVersion::V1_0, 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

@@ -2,7 +2,7 @@
 use libfuzzer_sys::fuzz_target;
 use std::hint::black_box;
 
-use quick_xml::{events::Event, reader::Reader, writer::Writer};
+use quick_xml::{events::Event, reader::Reader, writer::Writer, XmlVersion};
 use std::io::Cursor;
 
 macro_rules! debug_format {
@@ -15,6 +15,7 @@ fn round_trip<R>(reader: &mut Reader<R>) -> ()
 where
     R: std::io::BufRead,
 {
+    let mut version = XmlVersion::V1_0;
     let mut writer = Writer::new(Cursor::new(Vec::new()));
     let mut buf = vec![];
     let config = reader.config_mut();
@@ -34,14 +35,14 @@ 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(version).is_err())
+                    {
                         break;
                     }
                 }
             }
-            Ok(Event::Text(ref e))
-            | Ok(Event::Comment(ref e))
-            | Ok(Event::DocType(ref e)) => {
+            Ok(Event::Text(ref e)) | Ok(Event::Comment(ref e)) | Ok(Event::DocType(ref e)) => {
                 debug_format!(e);
                 if let Err(err) = e.decode() {
                     debug_format!(err);
@@ -68,6 +69,10 @@ where
                 let _ = black_box(e.version());
                 let _ = black_box(e.encoding());
                 let _ = black_box(e.standalone());
+                match e.xml_version() {
+                    Ok(v) => version = v,
+                    Err(_) => break,
+                }
             }
             Ok(Event::End(e)) => {
                 debug_format!(e.local_name());