[librsvg: 2/5] Make attribute indexes a newtype around u16 instead of usize

[Marge Bot <marge-bot src gnome org>]

commit 29c0f0e13491bd284f3caac2d47da45d8ac4f663
Author: Michael Howell <michael notriddle com>
Date:   Thu Jul 14 17:50:39 2022 -0700

    Make attribute indexes a newtype around u16 instead of usize
    
    Part-of: <https://gitlab.gnome.org/GNOME/librsvg/-/merge_requests/715>

 src/element.rs        | 24 ++++++++++++------------
 src/xml/attributes.rs | 42 ++++++++++++++++++++++++++++++++++++------
 src/xml/mod.rs        |  2 +-
 src/xml/xml2_load.rs  | 12 ++++++++++--
 4 files changed, 59 insertions(+), 21 deletions(-)
---
diff --git a/src/element.rs b/src/element.rs
index 21c8e2fff..8b993cb6c 100644
--- a/src/element.rs
+++ b/src/element.rs
@@ -42,7 +42,7 @@ use crate::shapes::{Circle, Ellipse, Line, Path, Polygon, Polyline, Rect};
 use crate::structure::{ClipPath, Group, Link, Mask, NonRendering, Svg, Switch, Symbol, Use};
 use crate::style::Style;
 use crate::text::{TRef, TSpan, Text};
-use crate::xml::Attributes;
+use crate::xml::{AttributeIndex, Attributes};
 
 // After creating/parsing a Element, it will be in a success or an error state.
 // We represent this with a Result, aliased as a ElementResult.  There is no
@@ -94,8 +94,8 @@ pub trait Draw {
 
 pub struct ElementInner<T: SetAttributes + Draw> {
     element_name: QualName,
-    id_idx: Option<usize>,    // id attribute from XML element
-    class_idx: Option<usize>, // class attribute from XML element
+    id_idx: Option<AttributeIndex>,    // id attribute from XML element
+    class_idx: Option<AttributeIndex>, // class attribute from XML element
     attributes: Attributes,
     specified_values: SpecifiedValues,
     important_styles: HashSet<QualName>,
@@ -110,8 +110,8 @@ pub struct ElementInner<T: SetAttributes + Draw> {
 impl<T: SetAttributes + Draw> ElementInner<T> {
     fn new(
         element_name: QualName,
-        id_idx: Option<usize>,
-        class_idx: Option<usize>,
+        id_idx: Option<AttributeIndex>,
+        class_idx: Option<AttributeIndex>,
         attributes: Attributes,
         result: Result<(), ElementError>,
         element_impl: T,
@@ -154,12 +154,12 @@ impl<T: SetAttributes + Draw> ElementInner<T> {
 
     fn get_id(&self) -> Option<&str> {
         self.id_idx
-            .and_then(|id_idx| self.attributes.get_by_idx(id_idx))
+            .and_then(|id_idx| self.attributes.get_by_index(id_idx))
     }
 
     fn get_class(&self) -> Option<&str> {
         self.class_idx
-            .and_then(|class_idx| self.attributes.get_by_idx(class_idx))
+            .and_then(|class_idx| self.attributes.get_by_index(class_idx))
     }
 
     fn inherit_xml_lang(&mut self, parent: Option<Node>) {
@@ -457,7 +457,7 @@ impl Element {
         let mut id_idx = None;
         let mut class_idx = None;
 
-        for (idx, (attr, _)) in attrs.iter().enumerate() {
+        for (idx, attr, _) in attrs.iter_indexed() {
             match attr.expanded() {
                 expanded_name!("", "id") => id_idx = Some(idx),
                 expanded_name!("", "class") => class_idx = Some(idx),
@@ -607,8 +607,8 @@ macro_rules! e {
         pub fn $name(
             element_name: &QualName,
             attributes: Attributes,
-            id_idx: Option<usize>,
-            class_idx: Option<usize>,
+            id_idx: Option<AttributeIndex>,
+            class_idx: Option<AttributeIndex>,
         ) -> Element {
             let mut element_impl = <$element_type>::default();
 
@@ -704,8 +704,8 @@ use creators::*;
 type ElementCreateFn = fn(
     element_name: &QualName,
     attributes: Attributes,
-    id_idx: Option<usize>,
-    class_idx: Option<usize>,
+    id_idx: Option<AttributeIndex>,
+    class_idx: Option<AttributeIndex>,
 ) -> Element;
 
 #[derive(Copy, Clone, PartialEq)]
diff --git a/src/xml/attributes.rs b/src/xml/attributes.rs
index c65c01eb4..bf9bd5d7a 100644
--- a/src/xml/attributes.rs
+++ b/src/xml/attributes.rs
@@ -14,6 +14,13 @@ use crate::util::{opt_utf8_cstr, utf8_cstr};
 /// string_cache crate.
 pub type AttributeValue = DefaultAtom;
 
+/// Type used to store an index into the attributes list.
+///
+/// Searching by name requires a linear scan, while looking up by index happens in
+/// constant time.
+#[derive(Clone, Copy)]
+pub struct AttributeIndex(u16);
+
 /// Iterable wrapper for libxml2's representation of attribute/value.
 ///
 /// See the [`new_from_xml2_attributes`] function for information.
@@ -25,6 +32,11 @@ pub struct Attributes(Box<[(QualName, AttributeValue)]>);
 /// Iterator from `Attributes.iter`.
 pub struct AttributesIter<'a>(slice::Iter<'a, (QualName, AttributeValue)>);
 
+/// Error struct returned when there are too many attributes.
+/// This libraries has a hardcoded limit of [`u16::MAX`].
+#[derive(Clone, Copy, Debug)]
+pub struct TooManyAttributesError;
+
 impl Attributes {
     #[cfg(test)]
     pub fn new() -> Attributes {
@@ -50,9 +62,13 @@ impl Attributes {
     pub unsafe fn new_from_xml2_attributes(
         n_attributes: usize,
         attrs: *const *const libc::c_char,
-    ) -> Attributes {
+    ) -> Result<Attributes, TooManyAttributesError> {
         let mut array = Vec::with_capacity(n_attributes);
 
+        if n_attributes > u16::MAX.into() {
+            return Err(TooManyAttributesError);
+        }
+
         if n_attributes > 0 && !attrs.is_null() {
             for attr in slice::from_raw_parts(attrs, n_attributes * 5).chunks_exact(5) {
                 let localname = attr[0];
@@ -92,7 +108,7 @@ impl Attributes {
             }
         }
 
-        Attributes(array.into())
+        Ok(Attributes(array.into()))
     }
 
     /// Returns the number of attributes.
@@ -105,8 +121,22 @@ impl Attributes {
         AttributesIter(self.0.iter())
     }
 
-    pub fn get_by_idx(&self, idx: usize) -> Option<&str> {
-        self.0.get(idx).map(|(_name, value)| &value[..])
+    /// Creates an iterator that yields `(AttributeIndex, QualName, &'a str)` tuples.
+    pub fn iter_indexed(&self) -> impl Iterator<Item = (AttributeIndex, QualName, &'_ str)> + '_ {
+        self.iter().enumerate().map(|(index, (name, value))| {
+            let index = AttributeIndex(
+                index
+                    .try_into()
+                    .expect("overlong indexes are filtered at creation time"),
+            );
+            (index, name, value)
+        })
+    }
+
+    pub fn get_by_index(&self, idx: AttributeIndex) -> Option<&str> {
+        self.0
+            .get(usize::from(idx.0))
+            .map(|(_name, value)| &value[..])
     }
 }
 
@@ -127,7 +157,7 @@ mod tests {
 
     #[test]
     fn empty_attributes() {
-        let map = unsafe { Attributes::new_from_xml2_attributes(0, ptr::null()) };
+        let map = unsafe { Attributes::new_from_xml2_attributes(0, ptr::null()).unwrap() };
         assert_eq!(map.len(), 0);
     }
 
@@ -176,7 +206,7 @@ mod tests {
             v.push(val_end); // value_end
         }
 
-        let attrs = unsafe { Attributes::new_from_xml2_attributes(3, v.as_ptr()) };
+        let attrs = unsafe { Attributes::new_from_xml2_attributes(3, v.as_ptr()).unwrap() };
 
         let mut had_href: bool = false;
         let mut had_ry: bool = false;
diff --git a/src/xml/mod.rs b/src/xml/mod.rs
index 8a6c7ed40..e5b0eb2f3 100644
--- a/src/xml/mod.rs
+++ b/src/xml/mod.rs
@@ -33,7 +33,7 @@ mod attributes;
 mod xml2;
 mod xml2_load;
 
-pub use attributes::Attributes;
+pub use attributes::{AttributeIndex, Attributes, TooManyAttributesError};
 
 #[derive(Clone)]
 enum Context {
diff --git a/src/xml/xml2_load.rs b/src/xml/xml2_load.rs
index 4318ffbda..7e0d889eb 100644
--- a/src/xml/xml2_load.rs
+++ b/src/xml/xml2_load.rs
@@ -19,8 +19,8 @@ use crate::error::LoadingError;
 use crate::util::{cstr, opt_utf8_cstr, utf8_cstr};
 
 use super::xml2::*;
-use super::Attributes;
 use super::XmlState;
+use super::{Attributes, TooManyAttributesError};
 
 #[rustfmt::skip]
 fn get_xml2_sax_handler() -> xmlSAXHandler {
@@ -215,7 +215,15 @@ unsafe extern "C" fn sax_start_element_ns_cb(
     let qual_name = make_qual_name(prefix, uri, localname);
 
     let nb_attributes = nb_attributes as usize;
-    let attrs = Attributes::new_from_xml2_attributes(nb_attributes, attributes as *const *const _);
+    let attrs =
+        match Attributes::new_from_xml2_attributes(nb_attributes, attributes as *const *const _) {
+            Ok(attrs) => attrs,
+            Err(TooManyAttributesError) => {
+                let parser = xml2_parser.parser.get();
+                xmlStopParser(parser);
+                return;
+            }
+        };
 
     if let Err(e) = xml2_parser.state.start_element(qual_name, attrs) {
         let _: () = e; // guard in case we change the error type later