[librsvg: 10/53] Make paint servers with circular references fall back to the alternate color

[Federico Mena Quintero <federico src gnome org>]

commit 3e39724814de9405cef70f80a9fd3f957094d530
Author: Federico Mena Quintero <federico gnome org>
Date:   Tue Oct 13 16:24:43 2020 -0500

    Make paint servers with circular references fall back to the alternate color
    
    We were taking all reference cycles to be an invalid SVG document; the
    SVG spec doesn't say this, but instead that we should be more lenient.
    
    This adds a new test for that behavior, in the librsvg_crate tests.
    
    Also removes the old infinite_loop tests in C, since they expected
    different behavior - that rendering would exit early with an error.

 librsvg_crate/tests/standalone/bugs.rs             | 68 ++++++++++++++++++++++
 rsvg_internals/src/paint_server.rs                 | 10 ++--
 tests/Makefile.am                                  |  8 +--
 .../infinite-loop/341-recursive-pattern.svg        |  5 --
 .../infinite-loop/398-recursive-gradient.svg       |  5 --
 tests/infinite-loop.c                              | 68 ----------------------
 6 files changed, 74 insertions(+), 90 deletions(-)
---
diff --git a/librsvg_crate/tests/standalone/bugs.rs b/librsvg_crate/tests/standalone/bugs.rs
index 41c5a06b..4e9b1263 100644
--- a/librsvg_crate/tests/standalone/bugs.rs
+++ b/librsvg_crate/tests/standalone/bugs.rs
@@ -169,3 +169,71 @@ fn nonexistent_filter_leaves_object_unfiltered() {
         "nonexistent_filter_leaves_object_unfiltered",
     );
 }
+
+// https://www.w3.org/TR/SVG2/painting.html#SpecifyingPaint says this:
+//
+// A <paint> allows a paint server reference, to be optionally
+// followed by a <color> or the keyword none. When this optional value
+// is given, the <color> value or the value none is a fallback value
+// to use if the paint server reference in the layer is invalid (due
+// to pointing to an element that does not exist or which is not a
+// valid paint server).
+//
+// I'm interpreting this to mean that if we have
+// fill="url(#recursive_paint_server) fallback_color", then the
+// recursive paint server is not valid, and should fall back to to the
+// specified color.
+#[test]
+fn recursive_paint_servers_fallback_to_color() {
+    let svg = load_svg(
+        br##"<?xml version="1.0" encoding="UTF-8"?>
+<svg xmlns="http://www.w3.org/2000/svg"; xmlns:xlink="http://www.w3.org/1999/xlink";
+     width="200" height="200">
+  <defs>
+    <pattern id="p" width="10" height="10" xlink:href="#p"/>
+    <linearGradient id="l" xlink:href="#r"/>
+    <radialGradient id="r" xlink:href="#l"/>
+  </defs>
+
+  <!-- These two should not render as there is no fallback color -->
+  <rect fill="url(#p)" x="0" y="0" width="100" height="100" />
+  <rect fill="url(#l)" x="100" y="0" width="100" height="100" />
+
+  <!-- These two should render with the fallback color -->
+  <rect fill="url(#p) lime" x="0" y="100" width="100" height="100" />
+  <rect fill="url(#l) lime" x="100" y="100" width="100" height="100" />
+</svg>
+"##,
+    );
+
+    let output_surf = render_document(
+        &svg,
+        SurfaceSize(200, 200),
+        |_| (),
+        cairo::Rectangle {
+            x: 0.0,
+            y: 0.0,
+            width: 200.0,
+            height: 200.0,
+        },
+    )
+    .unwrap();
+
+    let reference_surf = cairo::ImageSurface::create(cairo::Format::ARgb32, 200, 200).unwrap();
+
+    {
+        let cr = cairo::Context::new(&reference_surf);
+
+        cr.rectangle(0.0, 100.0, 200.0, 100.0);
+        cr.set_source_rgba(0.0, 1.0, 0.0, 1.0);
+        cr.fill();
+    }
+
+    let reference_surf = SharedImageSurface::wrap(reference_surf, SurfaceType::SRgb).unwrap();
+
+    compare_to_surface(
+        &output_surf,
+        &reference_surf,
+        "recursive_paint_servers_fallback_to_color",
+    );
+}
diff --git a/rsvg_internals/src/paint_server.rs b/rsvg_internals/src/paint_server.rs
index e482e807..b5f521b7 100644
--- a/rsvg_internals/src/paint_server.rs
+++ b/rsvg_internals/src/paint_server.rs
@@ -92,16 +92,16 @@ impl PaintServer {
                     }
                 })
                 .or_else(|err| match (err, alternate) {
-                    (AcquireError::CircularReference(node), _) => {
-                        rsvg_log!("circular reference in paint server {}", node);
-                        Err(RenderingError::CircularReference)
-                    }
-
                     (AcquireError::MaxReferencesExceeded, _) => {
                         rsvg_log!("maximum number of references exceeded");
                         Err(RenderingError::InstancingLimit)
                     }
 
+                    // The following two cases catch AcquireError::CircularReference, which for
+                    // paint servers may mean that there is a pattern or gradient with a reference
+                    // cycle in its "href" attribute.  This is an invalid paint server, and per
+                    // https://www.w3.org/TR/SVG2/painting.html#SpecifyingPaint we should try to
+                    // fall back to the alternate color.
                     (_, Some(color)) => {
                         rsvg_log!(
                             "could not resolve paint server \"{}\", using alternate color",
diff --git a/tests/Makefile.am b/tests/Makefile.am
index da4811ce..f5649ec1 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -18,8 +18,7 @@ EXTRA_DIST +=         \
 test_programs =                \
        api             \
        rsvg-test       \
-       errors          \
-       infinite-loop
+       errors
 
 test_utils_common_sources = \
        test-utils.c    \
@@ -33,10 +32,6 @@ errors_SOURCES =     \
        errors.c        \
        $(test_utils_common_sources)
 
-infinite_loop_SOURCES =        \
-       infinite-loop.c         \
-       $(test_utils_common_sources)
-
 rsvg_test_SOURCES = \
        rsvg-test.c     \
        $(test_utils_common_sources)
@@ -60,7 +55,6 @@ dist_installed_test_data =                                            \
        $(wildcard $(srcdir)/fixtures/crash/*.svg)                      \
        $(wildcard $(srcdir)/fixtures/crash/*.png)                      \
        $(wildcard $(srcdir)/fixtures/errors/*)                         \
-       $(wildcard $(srcdir)/fixtures/infinite-loop/*)                  \
        $(wildcard $(srcdir)/fixtures/loading/*)                        \
        $(wildcard $(srcdir)/fixtures/reftests/*.css)                   \
        $(wildcard $(srcdir)/fixtures/reftests/*.svg)                   \