[cairo-bugs] [Bug 102142] New: cairo_image_surface_create_from_png() returns PNG errors as CAIRO_STATUS_NO_MEMORY

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=102142

            Bug ID: 102142
           Summary: cairo_image_surface_create_from_png() returns PNG
                    errors as CAIRO_STATUS_NO_MEMORY
           Product: cairo
           Version: unspecified
          Hardware: Other
                OS: All
            Status: NEW
          Severity: normal
          Priority: medium
         Component: png functions
          Assignee: cworth at cworth.org
          Reporter: federico at gnome.org
        QA Contact: cairo-bugs at cairographics.org

I was writing some tests for cairo-rs (the Rust binding to Cairo) and tried
feeding it an invalid PNG.

We get to cairo-png.c:read_png() where it does

    if (setjmp (png_jmpbuf (png))) {
        surface = _cairo_surface_create_in_error (status);
        goto BAIL;
    }

There, the status it gets was set by png_simple_error_callback():

static void
png_simple_error_callback (png_structp png,
                           png_const_charp error_msg)
{
    cairo_status_t *error = png_get_error_ptr (png);

    if (*error == CAIRO_STATUS_SUCCESS)
        *error = _cairo_error (CAIRO_STATUS_PNG_ERROR);

    longjmp (png_jmpbuf (png), 1);
}

I thought that I would get an error surface with CAIRO_STATUS_PNG_ERROR out of
the call to cairo_image_surface_get_from_png(), but read_png()'s call to
_cairo_surface_create_in_error() has this:

cairo_surface_t *
_cairo_surface_create_in_error (cairo_status_t status)
{
    switch (status) {
    ...
    case CAIRO_STATUS_PNG_ERROR:
    ... other fall throughs ...
    default:
        _cairo_error_throw (CAIRO_STATUS_NO_MEMORY);
        return (cairo_surface_t *) &_cairo_surface_nil;
    }
}

And _cairo_surface_nil is the one with CAIRO_STATUS_NO_MEMORY.

I *think* we could meaningfully distinguish "out of memory in libpng" from
"libpng reports an error in the data" from "I/O error in the reader functions"
by a combination of things.

1. Use png_create_read_struct_2() instead of png_create_read_struct().  That
would let us pass our own malloc() replacement, detect OOM, and store a flag to
that effect in Cairo's own png_read_closure_t.

2. Add a new error surface for PNG_ERROR (why are all those fall-throughs to
_cairo_surface_nil?).

Comments appreciated.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.cairographics.org/archives/cairo-bugs/attachments/20170810/08184ef0/attachment.html>