Re: [PATCH] Generate better error message when loading invalid PNGs

Adrian Johnson <[email protected]>
Newsgroups gmane.comp.lib.cairo
Message-ID <[email protected]>
On 26/03/16 03:53, Uli Schlachter wrote:
> Based on an idea from Cyril Roelandt, this patch makes cairo generate better
> error messages when cairo_image_surface_create_from_png{,_stream} is called on
> e.g. JPEG files. To do so, we don't let libpng check if the file starts with a
> PNG signature, but do so by hand. On mismatch, a surface with status
> CAIRO_STATUS_READ_ERROR is returned where previously the status was
> CAIRO_STATUS_NO_MEMORY (the status used for all errors in libpng).

What about if it is a PNG file but libpng is unable to decode it? It is
still going to return the unhelpful CAIRO_STATUS_NO_MEMORY. It is also
useful restrict CAIRO_STATUS_READ_ERROR to errors reading the from the
stream.

I think we need an error status for backends that can fail. The attached
patch adds CAIRO_STATUS_PNG_ERROR. I can also write a patch for win32 as
it is returning CAIRO_STATUS_NO_MEMORY for GDI errors.

-- 
cairo mailing list
[email protected]
https://lists.cairographics.org/mailman/listinfo/cairo
0001-Add-CAIRO_STATUS_PNG_ERROR-for-errors-returned-by-li.patch (text/x-patch, 6.9 KB)
From ec854c8d55789a3891347679be22643b440335c3 Mon Sep 17 00:00:00 2001
From: Adrian Johnson <[email protected]>
Date: Sat, 26 Mar 2016 07:24:26 +1030
Subject: [PATCH] Add CAIRO_STATUS_PNG_ERROR for errors returned by libpng

---
 src/cairo-device.c        |  1 +
 src/cairo-error-private.h |  1 +
 src/cairo-misc.c          |  2 ++
 src/cairo-png.c           | 10 +++++++---
 src/cairo-region.c        |  1 +
 src/cairo-spans.c         |  1 +
 src/cairo-surface.c       |  1 +
 src/cairo.c               |  3 ++-
 src/cairo.h               |  2 ++
 9 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/src/cairo-device.c b/src/cairo-device.c
index 585a9c1..e270666 100644
--- a/src/cairo-device.c
+++ b/src/cairo-device.c
@@ -159,6 +159,7 @@ _cairo_device_create_in_error (cairo_status_t status)
     case CAIRO_STATUS_INVALID_MESH_CONSTRUCTION:
     case CAIRO_STATUS_DEVICE_FINISHED:
     case CAIRO_STATUS_JBIG2_GLOBAL_MISSING:
+    case CAIRO_STATUS_PNG_ERROR:
     default:
 	_cairo_error_throw (CAIRO_STATUS_NO_MEMORY);
 	return (cairo_device_t *) &_nil_device;
diff --git a/src/cairo-error-private.h b/src/cairo-error-private.h
index 178078a..f94a893 100644
--- a/src/cairo-error-private.h
+++ b/src/cairo-error-private.h
@@ -94,6 +94,7 @@ enum _cairo_int_status {
     CAIRO_INT_STATUS_INVALID_MESH_CONSTRUCTION,
     CAIRO_INT_STATUS_DEVICE_FINISHED,
     CAIRO_INT_STATUS_JBIG2_GLOBAL_MISSING,
+    CAIRO_INT_STATUS_PNG_ERROR,
 
     CAIRO_INT_STATUS_LAST_STATUS,
 
diff --git a/src/cairo-misc.c b/src/cairo-misc.c
index 3c7c959..d1ab7b3 100644
--- a/src/cairo-misc.c
+++ b/src/cairo-misc.c
@@ -158,6 +158,8 @@ cairo_status_to_string (cairo_status_t status)
 	return "the target device has been finished";
     case CAIRO_STATUS_JBIG2_GLOBAL_MISSING:
 	return "CAIRO_MIME_TYPE_JBIG2_GLOBAL_ID used but no CAIRO_MIME_TYPE_JBIG2_GLOBAL data provided";
+    case CAIRO_STATUS_PNG_ERROR:
+	return "error occurred in libpng while reading from or writing to a PNG file";
     default:
     case CAIRO_STATUS_LAST_STATUS:
 	return "<unknown error status>";
diff --git a/src/cairo-png.c b/src/cairo-png.c
index 068617d..b160cac 100644
--- a/src/cairo-png.c
+++ b/src/cairo-png.c
@@ -136,7 +136,7 @@ png_simple_error_callback (png_structp png,
 
     /* default to the most likely error */
     if (*error == CAIRO_STATUS_SUCCESS)
-	*error = _cairo_error (CAIRO_STATUS_NO_MEMORY);
+	*error = _cairo_error (CAIRO_STATUS_PNG_ERROR);
 
 #ifdef PNG_SETJMP_SUPPORTED
     longjmp (png_jmpbuf (png), 1);
@@ -349,7 +349,8 @@ stdio_write_func (png_structp png, png_bytep data, png_size_t size)
  * be allocated for the operation or
  * %CAIRO_STATUS_SURFACE_TYPE_MISMATCH if the surface does not have
  * pixel contents, or %CAIRO_STATUS_WRITE_ERROR if an I/O error occurs
- * while attempting to write the file.
+ * while attempting to write the file, or %CAIRO_STATUS_PNG_ERROR if libpng
+ * returned an error.
  *
  * Since: 1.0
  **/
@@ -417,7 +418,8 @@ stream_write_func (png_structp png, png_bytep data, png_size_t size)
  * successfully.  Otherwise, %CAIRO_STATUS_NO_MEMORY is returned if
  * memory could not be allocated for the operation,
  * %CAIRO_STATUS_SURFACE_TYPE_MISMATCH if the surface does not have
- * pixel contents.
+ * pixel contents, or %CAIRO_STATUS_PNG_ERROR if libpng
+ * returned an error.
  *
  * Since: 1.0
  **/
@@ -744,6 +746,7 @@ read_png (struct png_read_closure_t *png_closure)
  *	%CAIRO_STATUS_NO_MEMORY
  *	%CAIRO_STATUS_FILE_NOT_FOUND
  *	%CAIRO_STATUS_READ_ERROR
+ *      %CAIRO_STATUS_PNG_ERROR
  *
  * Alternatively, you can allow errors to propagate through the drawing
  * operations and check the status on the context upon completion
@@ -799,6 +802,7 @@ cairo_image_surface_create_from_png (const char *filename)
  *
  *	%CAIRO_STATUS_NO_MEMORY
  *	%CAIRO_STATUS_READ_ERROR
+ *      %CAIRO_STATUS_PNG_ERROR
  *
  * Alternatively, you can allow errors to propagate through the drawing
  * operations and check the status on the context upon completion
diff --git a/src/cairo-region.c b/src/cairo-region.c
index 6a51225..6def842 100644
--- a/src/cairo-region.c
+++ b/src/cairo-region.c
@@ -107,6 +107,7 @@ _cairo_region_create_in_error (cairo_status_t status)
     case CAIRO_STATUS_INVALID_MESH_CONSTRUCTION:
     case CAIRO_STATUS_DEVICE_FINISHED:
     case CAIRO_STATUS_JBIG2_GLOBAL_MISSING:
+    case CAIRO_STATUS_PNG_ERROR:
     default:
 	_cairo_error_throw (CAIRO_STATUS_NO_MEMORY);
 	return (cairo_region_t *) &_cairo_region_nil;
diff --git a/src/cairo-spans.c b/src/cairo-spans.c
index 182390c..e8fa7c1 100644
--- a/src/cairo-spans.c
+++ b/src/cairo-spans.c
@@ -128,6 +128,7 @@ _cairo_scan_converter_create_in_error (cairo_status_t status)
     case CAIRO_STATUS_INVALID_MESH_CONSTRUCTION: RETURN_NIL;
     case CAIRO_STATUS_DEVICE_FINISHED: RETURN_NIL;
     case CAIRO_STATUS_JBIG2_GLOBAL_MISSING:
+    case CAIRO_STATUS_PNG_ERROR:
     default:
 	break;
     }
diff --git a/src/cairo-surface.c b/src/cairo-surface.c
index 46f6894..9cf3f09 100644
--- a/src/cairo-surface.c
+++ b/src/cairo-surface.c
@@ -2725,6 +2725,7 @@ _cairo_surface_create_in_error (cairo_status_t status)
     case CAIRO_STATUS_INVALID_MESH_CONSTRUCTION:
     case CAIRO_STATUS_DEVICE_FINISHED:
     case CAIRO_STATUS_JBIG2_GLOBAL_MISSING:
+    case CAIRO_STATUS_PNG_ERROR:
     default:
 	_cairo_error_throw (CAIRO_STATUS_NO_MEMORY);
 	return (cairo_surface_t *) &_cairo_surface_nil;
diff --git a/src/cairo.c b/src/cairo.c
index e3acf4d..05fd86b 100644
--- a/src/cairo.c
+++ b/src/cairo.c
@@ -153,7 +153,8 @@ static const cairo_t _cairo_nil[] = {
     DEFINE_NIL_CONTEXT (CAIRO_STATUS_DEVICE_ERROR),
     DEFINE_NIL_CONTEXT (CAIRO_STATUS_INVALID_MESH_CONSTRUCTION),
     DEFINE_NIL_CONTEXT (CAIRO_STATUS_DEVICE_FINISHED),
-    DEFINE_NIL_CONTEXT (CAIRO_STATUS_JBIG2_GLOBAL_MISSING)
+    DEFINE_NIL_CONTEXT (CAIRO_STATUS_JBIG2_GLOBAL_MISSING),
+    DEFINE_NIL_CONTEXT (CAIRO_STATUS_PNG_ERROR)
 
 };
 COMPILE_TIME_ASSERT (ARRAY_LENGTH (_cairo_nil) == CAIRO_STATUS_LAST_STATUS - 1);
diff --git a/src/cairo.h b/src/cairo.h
index 3104d47..a1808bd 100644
--- a/src/cairo.h
+++ b/src/cairo.h
@@ -292,6 +292,7 @@ typedef struct _cairo_user_data_key {
  * @CAIRO_STATUS_DEVICE_FINISHED: target device has been finished (Since 1.12)
  * @CAIRO_STATUS_JBIG2_GLOBAL_MISSING: %CAIRO_MIME_TYPE_JBIG2_GLOBAL_ID has been used on at least one image
  *   but no image provided %CAIRO_MIME_TYPE_JBIG2_GLOBAL (Since 1.14)
+ * @CAIRO_STATUS_PNG_ERROR: error occurred in libpng while reading from or writing to a PNG file (Since 1.16)
  * @CAIRO_STATUS_LAST_STATUS: this is a special value indicating the number of
  *   status values defined in this enumeration.  When using this value, note
  *   that the version of cairo at run-time may have additional status values
@@ -348,6 +349,7 @@ typedef enum _cairo_status {
     CAIRO_STATUS_INVALID_MESH_CONSTRUCTION,
     CAIRO_STATUS_DEVICE_FINISHED,
     CAIRO_STATUS_JBIG2_GLOBAL_MISSING,
+    CAIRO_STATUS_PNG_ERROR,
 
     CAIRO_STATUS_LAST_STATUS
 } cairo_status_t;
-- 
2.1.4
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.