[librsvg: 1/4] Prevent storing IRI::None in a FilterValueList

[Federico Mena Quintero <federico src gnome org>]

commit 3d664db68f224a74cf3f282baed5f3174d9eb555
Author: John Ledbetter <john ledbetter gmail com>
Date:   Tue Sep 8 13:42:24 2020 -0400

    Prevent storing IRI::None in a FilterValueList
    
    Previously an (invalid) css property like: `filter: url(#foo) none`
    would parse an IRI::None into the FilterValueList and cause a panic.
    
    This removes the usage of IRI for this case and stores the Fragment
    directly in the FilterValue::URL instead.
    
    This (partially) addresses https://gitlab.gnome.org/GNOME/librsvg/-/issues/621

 rsvg_internals/src/drawing_ctx.rs | 22 ++++++--------
 rsvg_internals/src/filter.rs      | 62 ++++++++++++++++++---------------------
 2 files changed, 38 insertions(+), 46 deletions(-)
---
diff --git a/rsvg_internals/src/drawing_ctx.rs b/rsvg_internals/src/drawing_ctx.rs
index 647691ec..a4f359b5 100644
--- a/rsvg_internals/src/drawing_ctx.rs
+++ b/rsvg_internals/src/drawing_ctx.rs
@@ -22,7 +22,6 @@ use crate::filter::{FilterValue, FilterValueList};
 use crate::filters;
 use crate::float_eq_cairo::ApproxEqCairo;
 use crate::gradient::{Gradient, GradientUnits, GradientVariant, SpreadMethod};
-use crate::iri::IRI;
 use crate::marker;
 use crate::node::{CascadedValues, Node, NodeBorrow, NodeDraw};
 use crate::paint_server::{PaintServer, PaintSource};
@@ -605,18 +604,15 @@ impl DrawingCtx {
                             .try_fold(
                                 child_surface,
                                 |surface, filter| -> Result<_, RenderingError> {
-                                    if let FilterValue::URL(IRI::Resource(filter_uri)) = filter {
-                                        dc.run_filter(
-                                            acquired_nodes,
-                                            &filter_uri,
-                                            node,
-                                            values,
-                                            surface,
-                                            bbox,
-                                        )
-                                    } else {
-                                        Ok(surface)
-                                    }
+                                    let FilterValue::URL(filter_uri) = filter;
+                                    dc.run_filter(
+                                        acquired_nodes,
+                                        &filter_uri,
+                                        node,
+                                        values,
+                                        surface,
+                                        bbox,
+                                    )
                                 },
                             )?
                             .into_image_surface()?;
diff --git a/rsvg_internals/src/filter.rs b/rsvg_internals/src/filter.rs
index 90718732..ef5c8bac 100644
--- a/rsvg_internals/src/filter.rs
+++ b/rsvg_internals/src/filter.rs
@@ -3,6 +3,7 @@
 use cssparser::Parser;
 use markup5ever::{expanded_name, local_name, namespace_url, ns};
 
+use crate::allowed_url::Fragment;
 use crate::bbox::BoundingBox;
 use crate::coord_units::CoordUnits;
 use crate::document::AcquiredNodes;
@@ -192,7 +193,7 @@ impl Draw for Filter {}
 
 #[derive(Debug, Clone, PartialEq)]
 pub enum FilterValue {
-    URL(IRI),
+    URL(Fragment),
 }
 #[derive(Debug, Clone, PartialEq)]
 pub struct FilterValueList(pub Vec<FilterValue>);
@@ -216,36 +217,31 @@ impl FilterValueList {
         }
 
         self.0.iter().all(|filter| match filter {
-            FilterValue::URL(IRI::Resource(filter_uri)) => {
-                match acquired_nodes.acquire(filter_uri) {
-                    Ok(acquired) => {
-                        let filter_node = acquired.get();
-
-                        match *filter_node.borrow_element() {
-                            Element::Filter(_) => true,
-                            _ => {
-                                rsvg_log!(
-                                    "element {} will not be filtered since \"{}\" is not a filter",
-                                    node,
-                                    filter_uri,
-                                );
-                                false
-                            }
+            FilterValue::URL(filter_uri) => match acquired_nodes.acquire(filter_uri) {
+                Ok(acquired) => {
+                    let filter_node = acquired.get();
+
+                    match *filter_node.borrow_element() {
+                        Element::Filter(_) => true,
+                        _ => {
+                            rsvg_log!(
+                                "element {} will not be filtered since \"{}\" is not a filter",
+                                node,
+                                filter_uri,
+                            );
+                            false
                         }
                     }
-                    _ => {
-                        rsvg_log!(
-                            "element {} will not be filtered since its filter \"{}\" was not found",
-                            node,
-                            filter_uri,
-                        );
-                        false
-                    }
                 }
-            }
-            FilterValue::URL(IRI::None) => {
-                unreachable!("Unexpected IRI::None in FilterValueList");
-            }
+                _ => {
+                    rsvg_log!(
+                        "element {} will not be filtered since its filter \"{}\" was not found",
+                        node,
+                        filter_uri,
+                    );
+                    false
+                }
+            },
         })
     }
 }
@@ -264,8 +260,8 @@ impl Parse for FilterValueList {
         while !parser.is_exhausted() {
             let loc = parser.current_source_location();
 
-            if let Ok(iri) = IRI::parse(parser) {
-                result.0.push(FilterValue::URL(iri));
+            if let Ok(IRI::Resource(uri)) = IRI::parse(parser) {
+                result.0.push(FilterValue::URL(uri));
             } else {
                 let token = parser.next()?;
                 return Err(loc.new_basic_unexpected_token_error(token.clone()).into());
@@ -281,7 +277,6 @@ impl Parse for FilterValueList {
 fn parses_filter_value_list() {
     use crate::allowed_url::Fragment;
     use crate::filter::FilterValue;
-    use crate::iri::IRI;
 
     assert_eq!(
         FilterValueList::parse_str("none"),
@@ -293,10 +288,11 @@ fn parses_filter_value_list() {
     assert_eq!(
         FilterValueList::parse_str("url(foo.svg#bar) url(test.svg#baz)"),
         Ok(FilterValueList(vec![
-            FilterValue::URL(IRI::Resource(f1)),
-            FilterValue::URL(IRI::Resource(f2))
+            FilterValue::URL(f1),
+            FilterValue::URL(f2)
         ]))
     );
 
     assert!(FilterValueList::parse_str("fail").is_err());
+    assert!(FilterValueList::parse_str("url(#test) none").is_err());
 }