[PATCH] Generate better error message when loading invalid PNGs

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).

Additionally, a unit test is added for testing this, but only for
cairo_image_surface_create_from_png_stream().

Signed-off-by: Uli Schlachter <[email protected]>
---

Hi Cyril, what do you think about this patch?

 src/cairo-png.c        |  8 ++++++++
 test/create-from-png.c | 20 ++++++++++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/src/cairo-png.c b/src/cairo-png.c
index 068617d..e4356aa 100644
--- a/src/cairo-png.c
+++ b/src/cairo-png.c
@@ -549,6 +549,7 @@ read_png (struct png_read_closure_t *png_closure)
     cairo_status_t status;
     unsigned char *mime_data;
     unsigned long mime_data_length;
+    png_byte png_magic[8];
 
     png_closure->png_data = _cairo_memory_stream_create ();
 
@@ -578,6 +579,13 @@ read_png (struct png_read_closure_t *png_closure)
     }
 #endif
 
+    stream_read_func (png, png_magic, ARRAY_LENGTH (png_magic));
+    if (png_sig_cmp (png_magic, 0, ARRAY_LENGTH (png_magic))) {
+	surface = _cairo_surface_create_in_error (CAIRO_STATUS_READ_ERROR);
+	goto BAIL;
+    }
+    png_set_sig_bytes (png, ARRAY_LENGTH (png_magic));
+
     png_read_info (png, info);
 
     png_get_IHDR (png, info,
diff --git a/test/create-from-png.c b/test/create-from-png.c
index 2ca1fa2..1f18c47 100644
--- a/test/create-from-png.c
+++ b/test/create-from-png.c
@@ -42,6 +42,13 @@ read_error (void *closure, unsigned char *data, unsigned int size)
     return CAIRO_STATUS_READ_ERROR;
 }
 
+static cairo_status_t
+not_a_png (void *closure, unsigned char *data, unsigned int size)
+{
+    memset (data, 42, size);
+    return CAIRO_STATUS_SUCCESS;
+}
+
 static cairo_test_status_t
 draw (cairo_t *cr, int width, int height)
 {
@@ -130,6 +137,19 @@ preamble (cairo_test_context_t *ctx)
     if (result != CAIRO_TEST_SUCCESS)
 	return result;
 
+    surface = cairo_image_surface_create_from_png_stream (not_a_png, NULL);
+    if (cairo_surface_status (surface) != CAIRO_STATUS_READ_ERROR) {
+	result = cairo_test_status_from_status (ctx,
+						cairo_surface_status (surface));
+	if (result == CAIRO_TEST_FAILURE) {
+	    cairo_test_log (ctx, "Error: expected \"read error\", but got: %s\n",
+			    cairo_status_to_string (cairo_surface_status (surface)));
+	}
+    }
+    cairo_surface_destroy (surface);
+    if (result != CAIRO_TEST_SUCCESS)
+	return result;
+
     /* cheekily test error propagation from the user write funcs as well ... */
     xasprintf (&path, "%s/reference", ctx->srcdir);
     xasprintf (&filename, "%s/%s", path, "create-from-png.ref.png");
-- 
2.8.0.rc3

-- 
cairo mailing list
[email protected]
https://lists.cairographics.org/mailman/listinfo/cairo