cairo_image_surface_create_from_png() returns PNG errors as CAIRO_STATUS_NO_MEMORY
@federico
Submitted by Federico Quintero Assigned to Carl Worth @cworth
Link to original bug (#102142)
Description
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.
-
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.
-
Add a new error surface for PNG_ERROR (why are all those fall-throughs to _cairo_surface_nil?).
Comments appreciated.