[librsvg: 5/7] render_to_pixbuf_at_size(): Handle a zero size by returning an empty pixbuf

[Federico Mena Quintero <federico src gnome org>]

commit c1e064b6d1ad54d4c23142e97b0214387cff9a6e
Author: Federico Mena Quintero <federico gnome org>
Date:   Mon Feb 25 17:20:05 2019 -0600

    render_to_pixbuf_at_size(): Handle a zero size by returning an empty pixbuf
    
    SharedImageSurface: disallow creating with a zero-sized Cairo surface
    
    gdk_pixbuf_get_file_info() uses a GdkPixbufLoader, but in its
    "size-prepared" callback it saves the computed size, and then calls
    gdk_pixbuf_loader_set_size(loader, 0, 0).  Presumably it does to tell
    loaders that it only wanted to know the size, but that they shouldn't
    decode or render the image to a pixbuf buffer.
    
    Librsvg used to panic when getting (0, 0) from the size_callback:
    
    - Handle.get_dimensions() would return (0, 0)
    
    - Handle.get_pixbuf_sub() would create an ImageSurface of size (0, 0),
      which is valid in Cairo's terms.
    
    - But SharedImageSurface used to do
    
        data_ptr = NonNull::new(cairo_image_surface_get_data(...)).unwrap()
    
      which panics when cairo_image_surface_get_data() returs NULL.  This
      is not even guaranteed; I think cairo just passes the (0, 0) down to
      pixman, and pixman ends up doing malloc(0), whose result is
      implementation-defined.  Thus, we can't guarantee that NonNull will
      work.  The caller has to be responsible for not creating zero-sized
      shared surfaces.

 rsvg_internals/src/pixbuf_utils.rs                 | 25 +++++++++
 rsvg_internals/src/surface_utils/shared_surface.rs |  9 +++-
 tests/api.c                                        | 61 ++++++++++++++++++++--
 3 files changed, 88 insertions(+), 7 deletions(-)
---
diff --git a/rsvg_internals/src/pixbuf_utils.rs b/rsvg_internals/src/pixbuf_utils.rs
index 1bea47c9..c31e6869 100644
--- a/rsvg_internals/src/pixbuf_utils.rs
+++ b/rsvg_internals/src/pixbuf_utils.rs
@@ -20,6 +20,8 @@ use surface_utils::{
 // Pixbuf::new() doesn't return out-of-memory errors properly
 // See https://github.com/gtk-rs/gdk-pixbuf/issues/96
 fn pixbuf_new(width: i32, height: i32) -> Result<Pixbuf, RenderingError> {
+    assert!(width > 0 && height > 0);
+
     unsafe {
         let raw_pixbuf = gdk_pixbuf_sys::gdk_pixbuf_new(
             Colorspace::Rgb.to_glib(),
@@ -37,6 +39,25 @@ fn pixbuf_new(width: i32, height: i32) -> Result<Pixbuf, RenderingError> {
     }
 }
 
+fn empty_pixbuf() -> Result<Pixbuf, RenderingError> {
+    // GdkPixbuf does not allow zero-sized pixbufs, but Cairo allows zero-sized
+    // surfaces.  In this case, return a 1-pixel transparent pixbuf.
+
+    unsafe {
+        let raw_pixbuf =
+            gdk_pixbuf_sys::gdk_pixbuf_new(Colorspace::Rgb.to_glib(), true.to_glib(), 8, 1, 1);
+
+        if raw_pixbuf.is_null() {
+            return Err(RenderingError::OutOfMemory);
+        }
+
+        let pixbuf: Pixbuf = from_glib_full(raw_pixbuf);
+        pixbuf.put_pixel(0, 0, 0, 0, 0, 0);
+
+        Ok(pixbuf)
+    }
+}
+
 pub fn pixbuf_from_surface(surface: &SharedImageSurface) -> Result<Pixbuf, RenderingError> {
     let width = surface.width();
     let height = surface.height();
@@ -140,6 +161,10 @@ fn render_to_pixbuf_at_size(
     width: i32,
     height: i32,
 ) -> Result<Pixbuf, RenderingError> {
+    if width == 0 || height == 0 {
+        return empty_pixbuf();
+    }
+
     let surface = ImageSurface::create(cairo::Format::ARgb32, width, height)?;
 
     {
diff --git a/rsvg_internals/src/surface_utils/shared_surface.rs 
b/rsvg_internals/src/surface_utils/shared_surface.rs
index e91f1ceb..1319fb11 100644
--- a/rsvg_internals/src/surface_utils/shared_surface.rs
+++ b/rsvg_internals/src/surface_utils/shared_surface.rs
@@ -123,6 +123,13 @@ impl SharedImageSurface {
             unsafe { cairo_sys::cairo_surface_get_reference_count(surface.to_raw_none()) };
         assert_eq!(reference_count, 1);
 
+        let width = surface.get_width();
+        let height = surface.get_height();
+        // Cairo allows zero-sized surfaces, but it does malloc(0), whose result
+        // is implementation-defined.  So, we can't assume NonNull below.  This is
+        // why we disallow zero-sized surfaces here.
+        assert!(width > 0 && height > 0);
+
         surface.flush();
         if surface.status() != cairo::Status::Success {
             return Err(surface.status());
@@ -132,8 +139,6 @@ impl SharedImageSurface {
             NonNull::new(unsafe { cairo_sys::cairo_image_surface_get_data(surface.to_raw_none()) })
                 .unwrap();
 
-        let width = surface.get_width();
-        let height = surface.get_height();
         let stride = surface.get_stride() as isize;
 
         Ok(Self {
diff --git a/tests/api.c b/tests/api.c
index af9f938e..729549a9 100644
--- a/tests/api.c
+++ b/tests/api.c
@@ -623,6 +623,7 @@ dimensions_and_position (void)
 struct size_func_data {
     gboolean called;
     gboolean destroyed;
+    gboolean testing_size_func_calls;
 };
 
 static void
@@ -630,10 +631,12 @@ size_func (gint *width, gint *height, gpointer user_data)
 {
     struct size_func_data *data = user_data;
 
-    g_assert (!data->called);
-    data->called = TRUE;
+    if (data->testing_size_func_calls) {
+        g_assert (!data->called);
+        data->called = TRUE;
 
-    g_assert (!data->destroyed);
+        g_assert (!data->destroyed);
+    }
 
     *width = 42;
     *height = 43;
@@ -644,8 +647,10 @@ size_func_destroy (gpointer user_data)
 {
     struct size_func_data *data = user_data;
 
-    g_assert (!data->destroyed);
-    data->destroyed = TRUE;
+    if (data->testing_size_func_calls) {
+        g_assert (!data->destroyed);
+        data->destroyed = TRUE;
+    }
 }
 
 static void
@@ -665,6 +670,7 @@ set_size_callback (void)
 
     data.called = FALSE;
     data.destroyed = FALSE;
+    data.testing_size_func_calls = TRUE;
 
     rsvg_handle_set_size_callback (handle, size_func, &data, size_func_destroy);
 
@@ -695,11 +701,13 @@ reset_size_callback (void)
 
     data_1.called = FALSE;
     data_1.destroyed = FALSE;
+    data_1.testing_size_func_calls = TRUE;
 
     rsvg_handle_set_size_callback (handle, size_func, &data_1, size_func_destroy);
 
     data_2.called = FALSE;
     data_2.destroyed = FALSE;
+    data_2.testing_size_func_calls = TRUE;
 
     rsvg_handle_set_size_callback (handle, size_func, &data_2, size_func_destroy);
     g_assert (data_1.destroyed);
@@ -709,6 +717,48 @@ reset_size_callback (void)
     g_assert (data_2.destroyed);
 }
 
+static void
+zero_size_func (gint *width, gint *height, gpointer user_data)
+{
+    *width = 0;
+    *height = 0;
+}
+
+static void
+render_with_zero_size_callback (void)
+{
+    /* gdk_pixbuf_get_file_info() uses a GdkPixbufLoader, but in its
+     * "size-prepared" callback it saves the computed size, and then calls
+     * gdk_pixbuf_loader_set_size(loader, 0, 0).  Presumably it does to tell
+     * loaders that it only wanted to know the size, but that they shouldn't
+     * decode or render the image to a pixbuf buffer.
+     *
+     * Librsvg used to panic when getting (0, 0) from the size_callback; this
+     * test is to check that there is no such crash now.  Instead, librsvg
+     * will return a 1x1 transparent pixbuf.
+     */
+    char *filename = get_test_filename ("example.svg");
+    GError *error = NULL;
+    RsvgHandle *handle;
+    GdkPixbuf *pixbuf;
+
+    handle = rsvg_handle_new_from_file (filename, &error);
+    g_free (filename);
+
+    g_assert (handle != NULL);
+    g_assert (error == NULL);
+
+    rsvg_handle_set_size_callback (handle, zero_size_func, NULL, NULL);
+
+    pixbuf = rsvg_handle_get_pixbuf (handle);
+    g_assert (pixbuf != NULL);
+    g_assert (gdk_pixbuf_get_width (pixbuf) == 1);
+    g_assert (gdk_pixbuf_get_height (pixbuf) == 1);
+
+    g_object_unref (pixbuf);
+    g_object_unref (handle);
+}
+
 static void
 detects_cairo_context_in_error (void)
 {
@@ -1041,6 +1091,7 @@ main (int argc, char **argv)
     g_test_add_func ("/api/dimensions_and_position", dimensions_and_position);
     g_test_add_func ("/api/set_size_callback", set_size_callback);
     g_test_add_func ("/api/reset_size_callback", reset_size_callback);
+    g_test_add_func ("/api/render_with_zero_size_callback", render_with_zero_size_callback);
     g_test_add_func ("/api/detects_cairo_context_in_error", detects_cairo_context_in_error);
     g_test_add_func ("/api/can_draw_to_non_image_surface", can_draw_to_non_image_surface);
     g_test_add_func ("/api/render_cairo_sub", render_cairo_sub);