sergiotarxz/libvips
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

improve GIF edarly close again

Browse Source 
We were trying to keep the FILE open for gifload between header and
load, but this meant some corrupt GIFs could keep the file open longer
than they should.

Instead, make close into a vfunc and always close between header and
load.

see https://github.com/libvips/libvips/issues/1370#issuecomment-526829415
This commit is contained in:
John Cupitt 2019-09-01 12:54:47 +01:00
parent 4e2033053e
commit a1ed6c7f6c
1 changed files with 91 additions and 60 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

151
libvips/foreign/gifload.c
View File

@ -32,6 +32,8 @@
* - better feof() handling
* 27/8/19
* - check image and frame bounds, since giflib does not
* 1/9/19
* - improve early close again
*/
/*
@ -207,6 +209,10 @@ typedef struct _VipsForeignLoadGifClass {
/* Close and reopen gif->file.
*/
int (*open)( VipsForeignLoadGif *gif );
/* Close any underlying file resource.
*/
void (*close)( VipsForeignLoadGif *gif );
} VipsForeignLoadGifClass;
G_DEFINE_ABSTRACT_TYPE( VipsForeignLoadGif, vips_foreign_load_gif,
@ -301,32 +307,14 @@ vips_foreign_load_gif_error( VipsForeignLoadGif *gif )
vips_foreign_load_gif_error_vips( gif, error );
}
static void
vips_foreign_load_gif_close( VipsForeignLoadGif *gif )
{
#ifdef HAVE_GIFLIB_5
if( gif->file ) {
int error;
if( DGifCloseFile( gif->file, &error ) == GIF_ERROR )
vips_foreign_load_gif_error_vips( gif, error );
gif->file = NULL;
}
#else
if( gif->file ) {
if( DGifCloseFile( gif->file ) == GIF_ERROR )
vips_foreign_load_gif_error_vips( gif, GifLastError() );
gif->file = NULL;
}
#endif
}
static void
vips_foreign_load_gif_dispose( GObject *gobject )
{
VipsForeignLoadGif *gif = (VipsForeignLoadGif *) gobject;
VipsForeignLoadGifClass *class =
(VipsForeignLoadGifClass *) VIPS_OBJECT_GET_CLASS( gif );
vips_foreign_load_gif_close( gif );
class->close( gif );
VIPS_UNREF( gif->frame );
VIPS_UNREF( gif->previous );
@ -629,18 +617,12 @@ vips_foreign_load_gif_set_header( VipsForeignLoadGif *gif, VipsImage *image )
* Don't flag errors during header scan. Many GIFs do not follow spec.
*/
static int
vips_foreign_load_gif_header( VipsForeignLoad *load )
vips_foreign_load_gif_scan( VipsForeignLoadGif *gif )
{
VipsObjectClass *class = VIPS_OBJECT_GET_CLASS( load );
VipsForeignLoadGifClass *gif_class =
(VipsForeignLoadGifClass *) VIPS_OBJECT_GET_CLASS( load );
VipsForeignLoadGif *gif = (VipsForeignLoadGif *) load;
VipsObjectClass *class = VIPS_OBJECT_GET_CLASS( gif );
GifRecordType record;
if( gif_class->open( gif ) )
return( -1 );
gif->n_pages = 0;
do {
@ -684,11 +666,34 @@ vips_foreign_load_gif_header( VipsForeignLoad *load )
return( -1 );
}
/* And set the output vips header from what we've learned.
*/
if( vips_foreign_load_gif_set_header( gif, load->out ) )
return( 0 );
}
/* Scan the GIF and set the libvips header. We always close after scan, even
* on an error.
*/
static int
vips_foreign_load_gif_header( VipsForeignLoad *load )
{
VipsForeignLoadGifClass *class =
(VipsForeignLoadGifClass *) VIPS_OBJECT_GET_CLASS( load );
VipsForeignLoadGif *gif = (VipsForeignLoadGif *) load;
if( class->open( gif ) )
return( -1 );
if( vips_foreign_load_gif_scan( gif ) ) {
class->close( gif );
return( -1 );
}
if( vips_foreign_load_gif_set_header( gif, load->out ) ) {
class->close( gif );
return( -1 );
}
class->close( gif );
return( 0 );
}
@ -1040,7 +1045,10 @@ vips_foreign_load_gif_generate( VipsRegion *or,
static void
vips_foreign_load_gif_minimise( VipsObject *object, VipsForeignLoadGif *gif )
{
vips_foreign_load_gif_close( gif );
VipsForeignLoadGifClass *class =
(VipsForeignLoadGifClass *) VIPS_OBJECT_GET_CLASS( gif );
class->close( gif );
}
static int
@ -1052,8 +1060,6 @@ vips_foreign_load_gif_load( VipsForeignLoad *load )
VipsImage **t = (VipsImage **)
vips_object_local_array( VIPS_OBJECT( load ), 4 );
/* Rewind.
*/
if( class->open( gif ) )
return( -1 );
@ -1148,6 +1154,26 @@ vips_foreign_load_gif_open( VipsForeignLoadGif *gif )
return( 0 );
}
static void
vips_foreign_load_gif_close( VipsForeignLoadGif *gif )
{
#ifdef HAVE_GIFLIB_5
if( gif->file ) {
int error;
if( DGifCloseFile( gif->file, &error ) == GIF_ERROR )
vips_foreign_load_gif_error_vips( gif, error );
gif->file = NULL;
}
#else
if( gif->file ) {
if( DGifCloseFile( gif->file ) == GIF_ERROR )
vips_foreign_load_gif_error_vips( gif, GifLastError() );
gif->file = NULL;
}
#endif
}
static void
vips_foreign_load_gif_class_init( VipsForeignLoadGifClass *class )
{
@ -1161,6 +1187,7 @@ vips_foreign_load_gif_class_init( VipsForeignLoadGifClass *class )
gobject_class->get_property = vips_object_get_property;
gif_class->open = vips_foreign_load_gif_open;
gif_class->close = vips_foreign_load_gif_close;
load_class->header = vips_foreign_load_gif_header;
load_class->load = vips_foreign_load_gif_load;
@ -1219,17 +1246,6 @@ typedef VipsForeignLoadGifClass VipsForeignLoadGifFileClass;
G_DEFINE_TYPE( VipsForeignLoadGifFile, vips_foreign_load_gif_file,
vips_foreign_load_gif_get_type() );
static void
vips_foreign_load_gif_file_dispose( GObject *gobject )
{
VipsForeignLoadGifFile *file = (VipsForeignLoadGifFile *) gobject;
VIPS_FREEF( fclose, file->fp );
G_OBJECT_CLASS( vips_foreign_load_gif_file_parent_class )->
dispose( gobject );
}
/* Our input function for file open. We can't use DGifOpenFileName(), since
* that just calls open() and won't work with unicode on win32. We can't use
* DGifOpenFileHandle() since that's an fd from open() and you can't pass those
@ -1250,13 +1266,15 @@ vips_giflib_file_read( GifFileType *gfile, GifByteType *buffer, int n )
static int
vips_foreign_load_gif_file_header( VipsForeignLoad *load )
{
VipsForeignLoadGifFile *file = (VipsForeignLoadGifFile *) load;
VipsForeignLoadGif *gif = (VipsForeignLoadGif *) load;
VipsForeignLoadGifClass *class =
(VipsForeignLoadGifClass *) VIPS_OBJECT_GET_CLASS( load );
if( VIPS_FOREIGN_LOAD_CLASS(
vips_foreign_load_gif_file_parent_class )->header( load ) ) {
/* Close early if header read fails in our base class.
*/
VIPS_FREEF( fclose, file->fp );
class->close( gif );
return( -1 );
}
@ -1265,31 +1283,41 @@ vips_foreign_load_gif_file_header( VipsForeignLoad *load )
/* We have to have _open() as a vfunc since our base class needs to be able to
* make two scans of the gif (scan for header, then scan for pixels), so we
* must be able to close and reopen (or rewind).
* must be able to close and reopen.
*/
static int
vips_foreign_load_gif_file_open( VipsForeignLoadGif *gif )
{
VipsForeignLoad *load = (VipsForeignLoad *) gif;
VipsForeignLoadGifFile *file = (VipsForeignLoadGifFile *) gif;
VipsForeignLoadGifClass *class =
(VipsForeignLoadGifClass *) VIPS_OBJECT_GET_CLASS( load );
if( !file->fp ) {
if( !(file->fp =
vips__file_open_read( file->filename, NULL, FALSE )) )
return( -1 );
class->close( gif );
VIPS_SETSTR( load->out->filename, file->filename );
}
else
rewind( file->fp );
if( !(file->fp =
vips__file_open_read( file->filename, NULL, FALSE )) )
return( -1 );
VIPS_SETSTR( load->out->filename, file->filename );
vips_foreign_load_gif_close( gif );
gif->read_func = vips_giflib_file_read;
return( VIPS_FOREIGN_LOAD_GIF_CLASS(
vips_foreign_load_gif_file_parent_class )->open( gif ) );
}
static void
vips_foreign_load_gif_file_close( VipsForeignLoadGif *gif )
{
VipsForeignLoadGifFile *file = (VipsForeignLoadGifFile *) gif;
VIPS_FOREIGN_LOAD_GIF_CLASS(
vips_foreign_load_gif_file_parent_class )->close( gif );
VIPS_FREEF( fclose, file->fp );
}
static const char *vips_foreign_gif_suffs[] = {
".gif",
NULL
@ -1305,7 +1333,6 @@ vips_foreign_load_gif_file_class_init(
VipsForeignLoadClass *load_class = (VipsForeignLoadClass *) class;
VipsForeignLoadGifClass *gif_class = (VipsForeignLoadGifClass *) class;
gobject_class->dispose = vips_foreign_load_gif_file_dispose;
gobject_class->set_property = vips_object_set_property;
gobject_class->get_property = vips_object_get_property;
@ -1318,6 +1345,7 @@ vips_foreign_load_gif_file_class_init(
load_class->header = vips_foreign_load_gif_file_header;
gif_class->open = vips_foreign_load_gif_file_open;
gif_class->close = vips_foreign_load_gif_file_close;
VIPS_ARG_STRING( class, "filename", 1,
_( "Filename" ),
@ -1377,8 +1405,11 @@ static int
vips_foreign_load_gif_buffer_open( VipsForeignLoadGif *gif )
{
VipsForeignLoadGifBuffer *buffer = (VipsForeignLoadGifBuffer *) gif;
VipsForeignLoadGifClass *class =
(VipsForeignLoadGifClass *) VIPS_OBJECT_GET_CLASS( gif );
class->close( gif );
vips_foreign_load_gif_close( gif );
buffer->p = buffer->buf->data;
buffer->bytes_to_go = buffer->buf->length;
gif->read_func = vips_giflib_buffer_read;;