[cairo] [PATCH] Generate better error message when loading invalid PNGs

[Bill Spitzak]

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.

Grepping libpng for png_error, I think perhaps this code will pick the most
appropriate error:

    if (strstr(message, "alloc") || strstr(message, "OOM") ||
strstr(message, "memory")
         return CAIRO_STATUS_NO_MEMORY;
    else
         return CAIRO_STATUS_READ_ERROR;

Grep was from here:
https://github.com/glennrp/libpng/search?utf8=%E2%9C%93&q=png_error


On Fri, Mar 25, 2016 at 10:23 AM, Uli Schlachter <psychon at znc.in> 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).
>
> Additionally, a unit test is added for testing this, but only for
> cairo_image_surface_create_from_png_stream().
>
> Signed-off-by: Uli Schlachter <psychon at znc.in>
> ---
>
> 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
> cairo at cairographics.org
> https://lists.cairographics.org/mailman/listinfo/cairo
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.cairographics.org/archives/cairo/attachments/20160325/397082eb/attachment.html>