[librsvg: 3/5] image: use the same code to parse href in image and filters/image

[Federico Mena Quintero <federico src gnome org>]

commit 68b83affea124b66a882e15989369c28b2c17651
Author: Paolo Borelli <pborelli gnome org>
Date:   Thu Jan 17 11:26:36 2019 +0100

    image: use the same code to parse href in image and filters/image
    
    Also remove Href::without_fragment: we do not need to check this
    when parsing the attribute, I think it is ok to just report the
    error when we actually try to load the image and that fails.

 rsvg_internals/src/allowed_url.rs   | 27 ---------------------------
 rsvg_internals/src/filters/image.rs | 17 +++++++----------
 rsvg_internals/src/image.rs         | 12 ++++++------
 3 files changed, 13 insertions(+), 43 deletions(-)
---
diff --git a/rsvg_internals/src/allowed_url.rs b/rsvg_internals/src/allowed_url.rs
index 662493f1..232b419c 100644
--- a/rsvg_internals/src/allowed_url.rs
+++ b/rsvg_internals/src/allowed_url.rs
@@ -240,15 +240,6 @@ impl Href {
         }
     }
 
-    pub fn without_fragment(href: &str) -> Result<Href, HrefError> {
-        use self::Href::*;
-
-        match Href::parse(href)? {
-            r @ PlainUrl(_) => Ok(r),
-            WithFragment(_) => Err(HrefError::FragmentForbidden),
-        }
-    }
-
     pub fn with_fragment(href: &str) -> Result<Href, HrefError> {
         use self::Href::*;
 
@@ -386,24 +377,6 @@ mod tests {
         assert_eq!(Href::parse("uri#"), Err(HrefError::ParseError));
     }
 
-    #[test]
-    fn href_without_fragment() {
-        assert_eq!(
-            Href::without_fragment("uri").unwrap(),
-            Href::PlainUrl("uri".to_string())
-        );
-
-        assert_eq!(
-            Href::without_fragment("#foo"),
-            Err(HrefError::FragmentForbidden)
-        );
-
-        assert_eq!(
-            Href::without_fragment("uri#foo"),
-            Err(HrefError::FragmentForbidden)
-        );
-    }
-
     #[test]
     fn href_with_fragment() {
         assert_eq!(
diff --git a/rsvg_internals/src/filters/image.rs b/rsvg_internals/src/filters/image.rs
index 259ac2d4..e604a9d5 100644
--- a/rsvg_internals/src/filters/image.rs
+++ b/rsvg_internals/src/filters/image.rs
@@ -36,15 +36,6 @@ impl Image {
         }
     }
 
-    fn set_href(&self, attr: Attribute, href_str: &str) -> NodeResult {
-        let href = Href::parse(href_str)
-            .map_err(|_| NodeError::parse_error(attr, ParseError::new("could not parse href")))?;
-
-        *self.href.borrow_mut() = Some(href);
-
-        Ok(())
-    }
-
     /// Renders the filter if the source is an existing node.
     fn render_node(
         &self,
@@ -185,7 +176,13 @@ impl NodeTrait for Image {
                 Attribute::PreserveAspectRatio => self.aspect.set(attr.parse(value, ())?),
 
                 // "path" is used by some older Adobe Illustrator versions
-                Attribute::XlinkHref | Attribute::Path => self.set_href(attr, value)?,
+                Attribute::XlinkHref | Attribute::Path => {
+                    let href = Href::parse(value).map_err(|_| {
+                        NodeError::parse_error(attr, ParseError::new("could not parse href"))
+                    })?;
+
+                    *self.href.borrow_mut() = Some(href);
+                }
 
                 _ => (),
             }
diff --git a/rsvg_internals/src/image.rs b/rsvg_internals/src/image.rs
index d1ebf1e1..65f7620f 100644
--- a/rsvg_internals/src/image.rs
+++ b/rsvg_internals/src/image.rs
@@ -11,7 +11,7 @@ use error::{NodeError, RenderingError};
 use float_eq_cairo::ApproxEqCairo;
 use length::*;
 use node::*;
-use parsers::ParseValue;
+use parsers::{ParseError, ParseValue};
 use property_bag::PropertyBag;
 
 pub struct NodeImage {
@@ -61,11 +61,11 @@ impl NodeTrait for NodeImage {
 
                 // "path" is used by some older Adobe Illustrator versions
                 Attribute::XlinkHref | Attribute::Path => {
-                    // FIXME: use better errors here; these should be loading errors
-                    *self.href.borrow_mut() =
-                        Some(Href::without_fragment(value).map_err(|_| {
-                            NodeError::value_error(attr, "fragment not allowed here")
-                        })?);
+                    let href = Href::parse(value).map_err(|_| {
+                        NodeError::parse_error(attr, ParseError::new("could not parse href"))
+                    })?;
+
+                    *self.href.borrow_mut() = Some(href);
                 }
 
                 _ => (),