<div dir="ltr">I really don't see this as better, because now you are relying on local code arriving at exactly the same "is this a png file" conclusion as libpng. Also there are lots of other errors that are not no-memory.<div><br></div><div>Grepping libpng for png_error, I think perhaps this code will pick the most appropriate error:</div><div><br></div><div>    if (strstr(message, "alloc") || strstr(message, "OOM") || strstr(message, "memory")</div><div>         return CAIRO_STATUS_NO_MEMORY;</div><div>    else</div><div>         return CAIRO_STATUS_READ_ERROR;</div><div><br></div><div>Grep was from here: <a href="https://github.com/glennrp/libpng/search?utf8=%E2%9C%93&q=png_error">https://github.com/glennrp/libpng/search?utf8=%E2%9C%93&q=png_error</a></div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Mar 25, 2016 at 10:23 AM, Uli Schlachter <span dir="ltr"><<a href="mailto:psychon@znc.in" target="_blank">psychon@znc.in</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Based on an idea from Cyril Roelandt, this patch makes cairo generate better<br>
error messages when cairo_image_surface_create_from_png{,_stream} is called on<br>
e.g. JPEG files. To do so, we don't let libpng check if the file starts with a<br>
PNG signature, but do so by hand. On mismatch, a surface with status<br>
CAIRO_STATUS_READ_ERROR is returned where previously the status was<br>
CAIRO_STATUS_NO_MEMORY (the status used for all errors in libpng).<br>
<br>
Additionally, a unit test is added for testing this, but only for<br>
cairo_image_surface_create_from_png_stream().<br>
<br>
Signed-off-by: Uli Schlachter <<a href="mailto:psychon@znc.in">psychon@znc.in</a>><br>
---<br>
<br>
Hi Cyril, what do you think about this patch?<br>
<br>
 src/cairo-png.c        |  8 ++++++++<br>
 test/create-from-png.c | 20 ++++++++++++++++++++<br>
 2 files changed, 28 insertions(+)<br>
<br>
diff --git a/src/cairo-png.c b/src/cairo-png.c<br>
index 068617d..e4356aa 100644<br>
--- a/src/cairo-png.c<br>
+++ b/src/cairo-png.c<br>
@@ -549,6 +549,7 @@ read_png (struct png_read_closure_t *png_closure)<br>
     cairo_status_t status;<br>
     unsigned char *mime_data;<br>
     unsigned long mime_data_length;<br>
+    png_byte png_magic[8];<br>
<br>
     png_closure->png_data = _cairo_memory_stream_create ();<br>
<br>
@@ -578,6 +579,13 @@ read_png (struct png_read_closure_t *png_closure)<br>
     }<br>
 #endif<br>
<br>
+    stream_read_func (png, png_magic, ARRAY_LENGTH (png_magic));<br>
+    if (png_sig_cmp (png_magic, 0, ARRAY_LENGTH (png_magic))) {<br>
+       surface = _cairo_surface_create_in_error (CAIRO_STATUS_READ_ERROR);<br>
+       goto BAIL;<br>
+    }<br>
+    png_set_sig_bytes (png, ARRAY_LENGTH (png_magic));<br>
+<br>
     png_read_info (png, info);<br>
<br>
     png_get_IHDR (png, info,<br>
diff --git a/test/create-from-png.c b/test/create-from-png.c<br>
index 2ca1fa2..1f18c47 100644<br>
--- a/test/create-from-png.c<br>
+++ b/test/create-from-png.c<br>
@@ -42,6 +42,13 @@ read_error (void *closure, unsigned char *data, unsigned int size)<br>
     return CAIRO_STATUS_READ_ERROR;<br>
 }<br>
<br>
+static cairo_status_t<br>
+not_a_png (void *closure, unsigned char *data, unsigned int size)<br>
+{<br>
+    memset (data, 42, size);<br>
+    return CAIRO_STATUS_SUCCESS;<br>
+}<br>
+<br>
 static cairo_test_status_t<br>
 draw (cairo_t *cr, int width, int height)<br>
 {<br>
@@ -130,6 +137,19 @@ preamble (cairo_test_context_t *ctx)<br>
     if (result != CAIRO_TEST_SUCCESS)<br>
        return result;<br>
<br>
+    surface = cairo_image_surface_create_from_png_stream (not_a_png, NULL);<br>
+    if (cairo_surface_status (surface) != CAIRO_STATUS_READ_ERROR) {<br>
+       result = cairo_test_status_from_status (ctx,<br>
+                                               cairo_surface_status (surface));<br>
+       if (result == CAIRO_TEST_FAILURE) {<br>
+           cairo_test_log (ctx, "Error: expected \"read error\", but got: %s\n",<br>
+                           cairo_status_to_string (cairo_surface_status (surface)));<br>
+       }<br>
+    }<br>
+    cairo_surface_destroy (surface);<br>
+    if (result != CAIRO_TEST_SUCCESS)<br>
+       return result;<br>
+<br>
     /* cheekily test error propagation from the user write funcs as well ... */<br>
     xasprintf (&path, "%s/reference", ctx->srcdir);<br>
     xasprintf (&filename, "%s/%s", path, "create-from-png.ref.png");<br>
<span class="HOEnZb"><font color="#888888">--<br>
2.8.0.rc3<br>
<br>
--<br>
cairo mailing list<br>
<a href="mailto:cairo@cairographics.org">cairo@cairographics.org</a><br>
<a href="https://lists.cairographics.org/mailman/listinfo/cairo" rel="noreferrer" target="_blank">https://lists.cairographics.org/mailman/listinfo/cairo</a></font></span></blockquote></div><br></div>