From: Tony Cook Date: Sun, 13 May 2001 07:20:37 +0000 (+0000) Subject: added some tests to check how we handle variously damaged GIFs X-Git-Tag: Imager-0.48^2~653 X-Git-Url: http://git.imager.perl.org/imager.git/commitdiff_plain/e6172a17b4d60bb9cf69578ebccb8afd78704056 added some tests to check how we handle variously damaged GIFs added code to release the memory allocated for the returned colour map correctly --- diff --git a/gif.c b/gif.c index 27bf1b8a..cc0149cd 100644 --- a/gif.c +++ b/gif.c @@ -163,6 +163,13 @@ i_readgif_low(GifFileType *GifFile, int **colour_table, int *colours) { mm_log((1,"i_readgif_low(GifFile %p, colour_table %p, colours %p)\n", GifFile, colour_table, colours)); + /* it's possible that the caller has called us with *colour_table being + non-NULL, but we check that to see if we need to free an allocated + colour table on error. + */ + if (colour_table) + *colour_table = NULL; + BackGround = GifFile->SBackGroundColor; ColorMap = (GifFile->Image.ColorMap ? GifFile->Image.ColorMap : GifFile->SColorMap); @@ -187,7 +194,10 @@ i_readgif_low(GifFileType *GifFile, int **colour_table, int *colours) { if (DGifGetRecordType(GifFile, &RecordType) == GIF_ERROR) { gif_push_error(); i_push_error(0, "Unable to get record type"); - if (colour_table) free(colour_table); /* FIXME: Isn't this an error? */ + if (colour_table && *colour_table) { + free(*colour_table); + *colour_table = NULL; + } i_img_destroy(im); DGifCloseFile(GifFile); return NULL; @@ -198,7 +208,10 @@ i_readgif_low(GifFileType *GifFile, int **colour_table, int *colours) { if (DGifGetImageDesc(GifFile) == GIF_ERROR) { gif_push_error(); i_push_error(0, "Unable to get image descriptor"); - if (colour_table) free(colour_table); /* FIXME: Isn't this an error? */ + if (colour_table && *colour_table) { + free(*colour_table); + *colour_table = NULL; + } i_img_destroy(im); DGifCloseFile(GifFile); return NULL; @@ -214,7 +227,7 @@ i_readgif_low(GifFileType *GifFile, int **colour_table, int *colours) { /* No colormap and we are about to read in the image - abandon for now */ mm_log((1, "Going in with no colormap\n")); i_push_error(0, "Image does not have a local or a global color map"); - if (colour_table) free(colour_table); /* FIXME: Isn't this an error? */ + /* we can't have allocated a colour table here */ i_img_destroy(im); DGifCloseFile(GifFile); return NULL; @@ -230,7 +243,10 @@ i_readgif_low(GifFileType *GifFile, int **colour_table, int *colours) { if (GifFile->Image.Left + GifFile->Image.Width > GifFile->SWidth || GifFile->Image.Top + GifFile->Image.Height > GifFile->SHeight) { i_push_errorf(0, "Image %d is not confined to screen dimension, aborted.\n",ImageNum); - if (colour_table) free(colour_table); /* FIXME: Yet again */ + if (colour_table && *colour_table) { + free(*colour_table); + *colour_table = NULL; + } i_img_destroy(im); DGifCloseFile(GifFile); return(0); @@ -242,8 +258,10 @@ i_readgif_low(GifFileType *GifFile, int **colour_table, int *colours) { if (DGifGetLine(GifFile, &GifRow[Col], Width) == GIF_ERROR) { gif_push_error(); i_push_error(0, "Reading GIF line"); - if (colour_table) - free(colour_table); + if (colour_table && *colour_table) { + free(*colour_table); + *colour_table = NULL; + } i_img_destroy(im); DGifCloseFile(GifFile); return NULL; @@ -264,8 +282,10 @@ i_readgif_low(GifFileType *GifFile, int **colour_table, int *colours) { if (DGifGetLine(GifFile, &GifRow[Col], Width) == GIF_ERROR) { gif_push_error(); i_push_error(0, "Reading GIF line"); - if (colour_table) - free(colour_table); + if (colour_table && *colour_table) { + free(*colour_table); + *colour_table = NULL; + } i_img_destroy(im); DGifCloseFile(GifFile); return NULL; @@ -287,8 +307,10 @@ i_readgif_low(GifFileType *GifFile, int **colour_table, int *colours) { if (DGifGetExtension(GifFile, &ExtCode, &Extension) == GIF_ERROR) { gif_push_error(); i_push_error(0, "Reading extension record"); - if (colour_table) - free(colour_table); + if (colour_table && *colour_table) { + free(*colour_table); + *colour_table = NULL; + } i_img_destroy(im); DGifCloseFile(GifFile); return NULL; @@ -297,8 +319,10 @@ i_readgif_low(GifFileType *GifFile, int **colour_table, int *colours) { if (DGifGetExtensionNext(GifFile, &Extension) == GIF_ERROR) { gif_push_error(); i_push_error(0, "reading next block of extension"); - if (colour_table) - free(colour_table); + if (colour_table && *colour_table) { + free(*colour_table); + *colour_table = NULL; + } i_img_destroy(im); DGifCloseFile(GifFile); return NULL; @@ -317,8 +341,10 @@ i_readgif_low(GifFileType *GifFile, int **colour_table, int *colours) { if (DGifCloseFile(GifFile) == GIF_ERROR) { gif_push_error(); i_push_error(0, "Closing GIF file object"); - if (colour_table) - free(colour_table); + if (colour_table && *colour_table) { + free(*colour_table); + *colour_table = NULL; + } i_img_destroy(im); return NULL; } diff --git a/t/t105gif.t b/t/t105gif.t index 5fcfaa8f..50931001 100644 --- a/t/t105gif.t +++ b/t/t105gif.t @@ -1,5 +1,5 @@ $|=1; -print "1..19\n"; +print "1..22\n"; use Imager qw(:all); init_log("testout/t105gif.log",1); @@ -293,9 +293,25 @@ EOS else { print "ok 19 # ",Imager::_error_as_msg(),"\n"; } -} + # try to read a truncated gif (no image descriptors) + read_failure('testimg/trimgdesc.gif', 20); + # file truncated just after the image descriptor tag + read_failure('testimg/trmiddesc.gif', 21); + # image has no colour map + read_failure('testimg/nocmap.gif', 22); + # image has a local colour map + open FH, "< testimg/loccmap.gif" + or die "Cannot open testimg/loccmap.gif: $!"; + binmode FH; + if (i_readgif(fileno(FH))) { + print "ok 23\n"; + } + else { + print "not ok 23 # failed to read image with only a local colour map"; + } +} sub test_readgif_cb { my ($size) = @_; @@ -306,3 +322,21 @@ sub test_readgif_cb { close FH; return $img; } + +# tests for reading bad gif files +sub read_failure { + my ($filename, $testnum) = @_; + + open FH, "< $filename" + or die "Cannot open $filename: $!"; + binmode FH; + my ($result, $map) = i_readgif(fileno(FH)); + if ($result) { + print "not ok $testnum # this is an invalid file, we succeeded\n"; + } + else { + print "ok $testnum # ",Imager::_error_as_msg(),"\n"; + } + close FH; +} + diff --git a/testimg/loccmap.gif b/testimg/loccmap.gif new file mode 100644 index 00000000..3a595bd6 Binary files /dev/null and b/testimg/loccmap.gif differ diff --git a/testimg/nocmap.gif b/testimg/nocmap.gif new file mode 100644 index 00000000..83941109 Binary files /dev/null and b/testimg/nocmap.gif differ diff --git a/testimg/trimgdesc.gif b/testimg/trimgdesc.gif new file mode 100644 index 00000000..f352b0ed Binary files /dev/null and b/testimg/trimgdesc.gif differ diff --git a/testimg/trmiddesc.gif b/testimg/trmiddesc.gif new file mode 100644 index 00000000..386e3c12 Binary files /dev/null and b/testimg/trmiddesc.gif differ