Avoid double svg error reporting and segfaults

* src/image.c (svg_load_image): Use g_error_free directly instead
of the helder g_clear_error (since we're only calling it with
non-nil values).
(svg_load_image): Avoid segfault and double reporting errors.
* test/manual/image-tests.el (image-tests-load-image/svg-too-big)
(image-tests-load-image/svg-invalid): Test it (bug#57755).

diff --git a/src/image.c b/src/image.c
index 549fe30ef7c1f1b62e9cc9f956d456aa7105a90c..2e04685e7f92ea5da98f8f5b34d6c157bccf61a8 100644 (file)
--- a/src/image.c
+++ b/src/image.c
@@ -10907,7 +10907,7 @@ DEF_DLL_FN (int, gdk_pixbuf_get_bits_per_sample, (const GdkPixbuf *));
 DEF_DLL_FN (void, g_type_init, (void));
 #  endif
 DEF_DLL_FN (void, g_object_unref, (gpointer));
-DEF_DLL_FN (void, g_clear_error, (GError **));
+DEF_DLL_FN (void, g_error_free, (GError *));
 
 static bool
 init_svg_functions (void)
@@ -10967,7 +10967,7 @@ init_svg_functions (void)
   LOAD_DLL_FN (gobject, g_type_init);
 #  endif
   LOAD_DLL_FN (gobject, g_object_unref);
-  LOAD_DLL_FN (glib, g_clear_error);
+  LOAD_DLL_FN (glib, g_error_free);
 
   return 1;
 }
@@ -10983,7 +10983,7 @@ init_svg_functions (void)
 #  undef gdk_pixbuf_get_pixels
 #  undef gdk_pixbuf_get_rowstride
 #  undef gdk_pixbuf_get_width
-#  undef g_clear_error
+#  undef g_error_free
 #  undef g_object_unref
 #  undef g_type_init
 #  if LIBRSVG_CHECK_VERSION (2, 52, 1)
@@ -11019,7 +11019,7 @@ init_svg_functions (void)
 #  define gdk_pixbuf_get_pixels fn_gdk_pixbuf_get_pixels
 #  define gdk_pixbuf_get_rowstride fn_gdk_pixbuf_get_rowstride
 #  define gdk_pixbuf_get_width fn_gdk_pixbuf_get_width
-#  define g_clear_error fn_g_clear_error
+#  define g_error_free fn_g_error_free
 #  define g_object_unref fn_g_object_unref
 #  if ! GLIB_CHECK_VERSION (2, 36, 0)
 #   define g_type_init fn_g_type_init
@@ -11353,7 +11353,7 @@ svg_load_image (struct frame *f, struct image *img, char *contents,
   if (! check_image_size (f, width, height))
     {
       image_size_error ();
-      goto rsvg_error;
+      goto done_error;
     }
 
   /* We are now done with the unmodified data.  */
@@ -11536,9 +11536,21 @@ svg_load_image (struct frame *f, struct image *img, char *contents,
     image_put_x_image (f, img, ximg, 0);
   }
 
+  eassume (err == NULL);
   return true;
 
  rsvg_error:
+  if (err == NULL)
+    image_error ("Error parsing SVG image");
+  else
+    {
+      image_error ("Error parsing SVG image: %s",
+                  call2 (Qstring_trim_right, build_string (err->message),
+                         Qnil));
+      g_error_free (err);
+    }
+
+ done_error:
   if (rsvg_handle)
     g_object_unref (rsvg_handle);
   if (wrapped_contents)
@@ -11547,10 +11559,6 @@ svg_load_image (struct frame *f, struct image *img, char *contents,
   if (css && !STRINGP (lcss))
     xfree (css);
 #endif
-  image_error ("Error parsing SVG image: %s",
-              /* The -1 removes an extra newline.  */
-              make_string (err->message, strlen (err->message) - 1));
-  g_clear_error (&err);
   return false;
 }
 
@@ -12265,4 +12273,5 @@ The options are:
   imagemagick_render_type = 0;
 #endif
 
+  DEFSYM (Qstring_trim_right, "string-trim-right");
 }

diff --git a/test/manual/image-tests.el b/test/manual/image-tests.el
index 7f1eab9512e643ed1d85f58794ab7fbcaa704bfd..400657132c745c29bd1fbe2c227e986f286561ba 100644 (file)
--- a/test/manual/image-tests.el
+++ b/test/manual/image-tests.el
@@ -79,6 +79,21 @@
 (image-tests-make-load-image-test 'xbm)
 (image-tests-make-load-image-test 'xpm)
 
+(ert-deftest image-tests-load-image/svg-too-big ()
+  (with-temp-buffer
+    (let* ((max-image-size 0)
+           (messages-buffer-name (buffer-name (current-buffer)))
+           (img (cdr (assq 'svg image-tests--images)))
+           (file (if (listp img)
+                     (plist-get (cdr img) :file)
+                   img)))
+      (save-excursion (find-file file))
+      (should (string-match-p "invalid image size" (buffer-string)))
+      ;; no annoying newlines
+      (should-not (string-match-p "^[ \t\n\r]+$" (buffer-string)))
+      ;; no annoying double error reporting
+      (should-not (string-match-p "error parsing" (buffer-string))))))
+
 (ert-deftest image-tests-load-image/svg-invalid ()
   (with-temp-buffer
     (let ((messages-buffer-name (buffer-name (current-buffer))))
@@ -90,7 +105,9 @@
                                              :type svg)))
         (redisplay))
       ;; librsvg error: "... Start tag expected, '<' not found [3 times]"
-      (should (string-match "[Ee]rror.+Start tag expected" (buffer-string))))))
+      (should (string-match-p "[Ee]rror.+Start tag expected" (buffer-string)))
+      ;; no annoying newlines
+      (should-not (string-match-p "^[ \t\n\r]+$" (buffer-string))))))
 
 \f
 ;;;; image-test-size