From 116cf67f4c59196605abdb244657c3070c4310af Mon Sep 17 00:00:00 2001 From: Even Rouault Date: Thu, 9 May 2019 22:21:29 +0200 Subject: [PATCH 1/2] BigTIFF creation: write TileByteCounts/StripByteCounts tag with LONG when possible In most situations of BigTIFF file, the tile/strip sizes are of reasonable size, that is they fit on a 4-byte LONG. So in that case, use LONG instead of LONG8 to save some space. For uncompressed file, it is easy to detect such situations by checking at the TIFFTileSize64()/TIFFStripSize64() return. For compressed file, we must take into account the fact that compression may sometimes result in larger compressed data. So we allow this optimization only for a few select compression times, and take a huge security margin (10x factor). We also only apply this optimization on multi-strip files, so as to allow easy on-the-fly growing of single-strip files whose strip size could grow above the 4GB threshold. This change is compatible with the BigTIFF specification. According to https://www.awaresystems.be/imaging/tiff/bigtiff.html: "The StripOffsets, StripByteCounts, TileOffsets, and TileByteCounts tags are allowed to have the datatype TIFF_LONG8 in BigTIFF. Old datatypes TIFF_LONG, and TIFF_SHORT where allowed in the TIFF 6.0 specification, are still valid in BigTIFF, too. " On a practical point of view, this is also compatible on reading/writing of older libtiff 4.X versions. The only glitch I found, which is rather minor, is when using such a BigTIFF file with TileByteCounts/StripByteCounts written with TIFF_LONG, and updating it with an older libtiff 4.X version with a change in the [Tile/Strip][ByteCounts/Offsets] array. In that case the _TIFFRewriteField() function will rewrite the directory and array with TIFF_LONG8, instead of updating the existing array (this is an issue fixed by this commit). The file will still be valid however, hence the minor severity of this. --- libtiff/tif_dirwrite.c | 69 +++++++++++++++++++++++++++++++++++++++--- 1 file changed, 64 insertions(+), 5 deletions(-) diff --git a/libtiff/tif_dirwrite.c b/libtiff/tif_dirwrite.c index 83c01b24..5b454c97 100644 --- a/libtiff/tif_dirwrite.c +++ b/libtiff/tif_dirwrite.c @@ -1651,6 +1651,29 @@ TIFFWriteDirectoryTagShortLong(TIFF* tif, uint32* ndir, TIFFDirEntry* dir, uint1 return(TIFFWriteDirectoryTagCheckedLong(tif,ndir,dir,tag,value)); } +static int WriteAsLong8(TIFF* tif, uint64 strile_size) +{ + const uint16 compression = tif->tif_dir.td_compression; + if ( compression == COMPRESSION_NONE ) + { + return strile_size >= 0xFFFFFFFFU; + } + else if ( compression == COMPRESSION_JPEG || + compression == COMPRESSION_LZW || + compression == COMPRESSION_ADOBE_DEFLATE || + compression == COMPRESSION_LZMA || + compression == COMPRESSION_LERC || + compression == COMPRESSION_ZSTD || + compression == COMPRESSION_WEBP ) + { + /* For a few select compression types, we assume that in the worst */ + /* case the compressed size will be 10 times the uncompressed size */ + /* This is overly pessismistic ! */ + return strile_size >= 0xFFFFFFFFU / 10; + } + return 1; +} + /************************************************************************/ /* TIFFWriteDirectoryTagLongLong8Array() */ /* */ @@ -1675,10 +1698,28 @@ TIFFWriteDirectoryTagLongLong8Array(TIFF* tif, uint32* ndir, TIFFDirEntry* dir, return(1); } - /* We always write LONG8 for BigTIFF, no checking needed. */ if( tif->tif_flags&TIFF_BIGTIFF ) - return TIFFWriteDirectoryTagCheckedLong8Array(tif,ndir,dir, - tag,count,value); + { + int write_aslong8 = 1; + /* In the case of ByteCounts array, we may be able to write them on */ + /* LONG if the strip/tilesize is not too big. */ + /* Also do that for count > 1 in the case someone would want to create */ + /* a single-strip file with a growing height, in which case using */ + /* LONG8 will be safer. */ + if( count > 1 && tag == TIFFTAG_STRIPBYTECOUNTS ) + { + write_aslong8 = WriteAsLong8(tif, TIFFStripSize64(tif)); + } + else if( count > 1 && tag == TIFFTAG_TILEBYTECOUNTS ) + { + write_aslong8 = WriteAsLong8(tif, TIFFTileSize64(tif)); + } + if( write_aslong8 ) + { + return TIFFWriteDirectoryTagCheckedLong8Array(tif,ndir,dir, + tag,count,value); + } + } /* ** For classic tiff we want to verify everything is in range for LONG @@ -2826,8 +2867,20 @@ _TIFFRewriteField(TIFF* tif, uint16 tag, TIFFDataType in_datatype, else datatype = in_datatype; } - else - datatype = in_datatype; + else + { + if( in_datatype == TIFF_LONG8 && + (entry_type == TIFF_LONG || entry_type == TIFF_LONG8 ) ) + datatype = entry_type; + else if( in_datatype == TIFF_SLONG8 && + (entry_type == TIFF_SLONG || entry_type == TIFF_SLONG8 ) ) + datatype = entry_type; + if( in_datatype == TIFF_IFD8 && + (entry_type == TIFF_IFD || entry_type == TIFF_IFD8 ) ) + datatype = entry_type; + else + datatype = in_datatype; + } /* -------------------------------------------------------------------- */ /* Prepare buffer of actual data to write. This includes */ @@ -2876,6 +2929,12 @@ _TIFFRewriteField(TIFF* tif, uint16 tag, TIFFDataType in_datatype, } } } + else + { + TIFFErrorExt( tif->tif_clientdata, module, + "Unhandled type conversion." ); + return 0; + } if( TIFFDataWidth(datatype) > 1 && (tif->tif_flags&TIFF_SWAB) ) { From 458c211ae2d2f9ba4a9982394594a023a3c2275c Mon Sep 17 00:00:00 2001 From: Even Rouault Date: Fri, 10 May 2019 11:08:01 +0200 Subject: [PATCH 2/2] Creation: use SHORT type when possible for StripByteCounts/TileByteCounts This follows the same logic as previous commit. --- libtiff/tif_dirwrite.c | 130 +++++++++++++++++++++++++++++++---------- test/rewrite_tag.c | 39 ++++++++----- 2 files changed, 126 insertions(+), 43 deletions(-) diff --git a/libtiff/tif_dirwrite.c b/libtiff/tif_dirwrite.c index 5b454c97..8171ba84 100644 --- a/libtiff/tif_dirwrite.c +++ b/libtiff/tif_dirwrite.c @@ -1651,12 +1651,12 @@ TIFFWriteDirectoryTagShortLong(TIFF* tif, uint32* ndir, TIFFDirEntry* dir, uint1 return(TIFFWriteDirectoryTagCheckedLong(tif,ndir,dir,tag,value)); } -static int WriteAsLong8(TIFF* tif, uint64 strile_size) +static int _WriteAsType(TIFF* tif, uint64 strile_size, uint64 uncompressed_threshold) { const uint16 compression = tif->tif_dir.td_compression; if ( compression == COMPRESSION_NONE ) { - return strile_size >= 0xFFFFFFFFU; + return strile_size > uncompressed_threshold; } else if ( compression == COMPRESSION_JPEG || compression == COMPRESSION_LZW || @@ -1669,27 +1669,34 @@ static int WriteAsLong8(TIFF* tif, uint64 strile_size) /* For a few select compression types, we assume that in the worst */ /* case the compressed size will be 10 times the uncompressed size */ /* This is overly pessismistic ! */ - return strile_size >= 0xFFFFFFFFU / 10; + return strile_size >= uncompressed_threshold / 10; } return 1; } +static int WriteAsLong8(TIFF* tif, uint64 strile_size) +{ + return _WriteAsType(tif, strile_size, 0xFFFFFFFFU); +} + +static int WriteAsLong4(TIFF* tif, uint64 strile_size) +{ + return _WriteAsType(tif, strile_size, 0xFFFFU); +} + /************************************************************************/ /* TIFFWriteDirectoryTagLongLong8Array() */ /* */ -/* Write out LONG8 array as LONG8 for BigTIFF or LONG for */ -/* Classic TIFF with some checking. */ +/* Write out LONG8 array and write a SHORT/LONG/LONG8 depending */ +/* on strile size and Classic/BigTIFF mode. */ /************************************************************************/ static int TIFFWriteDirectoryTagLongLong8Array(TIFF* tif, uint32* ndir, TIFFDirEntry* dir, uint16 tag, uint32 count, uint64* value) { static const char module[] = "TIFFWriteDirectoryTagLongLong8Array"; - uint64* ma; - uint32 mb; - uint32* p; - uint32* q; int o; + int write_aslong4; /* is this just a counting pass? */ if (dir==NULL) @@ -1721,32 +1728,77 @@ TIFFWriteDirectoryTagLongLong8Array(TIFF* tif, uint32* ndir, TIFFDirEntry* dir, } } - /* - ** For classic tiff we want to verify everything is in range for LONG - ** and convert to long format. - */ - - p = _TIFFmalloc(count*sizeof(uint32)); - if (p==NULL) + write_aslong4 = 1; + if( count > 1 && tag == TIFFTAG_STRIPBYTECOUNTS ) { - TIFFErrorExt(tif->tif_clientdata,module,"Out of memory"); - return(0); + write_aslong4 = WriteAsLong4(tif, TIFFStripSize64(tif)); } - - for (q=p, ma=value, mb=0; mb 1 && tag == TIFFTAG_TILEBYTECOUNTS ) { - if (*ma>0xFFFFFFFF) + write_aslong4 = WriteAsLong4(tif, TIFFTileSize64(tif)); + } + if( write_aslong4 ) + { + /* + ** For classic tiff we want to verify everything is in range for LONG + ** and convert to long format. + */ + + uint32* p = _TIFFmalloc(count*sizeof(uint32)); + uint32* q; + uint64* ma; + uint32 mb; + + if (p==NULL) { - TIFFErrorExt(tif->tif_clientdata,module, - "Attempt to write value larger than 0xFFFFFFFF in Classic TIFF file."); - _TIFFfree(p); + TIFFErrorExt(tif->tif_clientdata,module,"Out of memory"); return(0); } - *q= (uint32)(*ma); - } - o=TIFFWriteDirectoryTagCheckedLongArray(tif,ndir,dir,tag,count,p); - _TIFFfree(p); + for (q=p, ma=value, mb=0; mb0xFFFFFFFF) + { + TIFFErrorExt(tif->tif_clientdata,module, + "Attempt to write value larger than 0xFFFFFFFF in LONG array."); + _TIFFfree(p); + return(0); + } + *q= (uint32)(*ma); + } + + o=TIFFWriteDirectoryTagCheckedLongArray(tif,ndir,dir,tag,count,p); + _TIFFfree(p); + } + else + { + uint16* p = _TIFFmalloc(count*sizeof(uint16)); + uint16* q; + uint64* ma; + uint32 mb; + + if (p==NULL) + { + TIFFErrorExt(tif->tif_clientdata,module,"Out of memory"); + return(0); + } + + for (q=p, ma=value, mb=0; mb0xFFFF) + { + /* Should not happen normally given the check we did before */ + TIFFErrorExt(tif->tif_clientdata,module, + "Attempt to write value larger than 0xFFFF in SHORT array."); + _TIFFfree(p); + return(0); + } + *q= (uint16)(*ma); + } + + o=TIFFWriteDirectoryTagCheckedShortArray(tif,ndir,dir,tag,count,p); + _TIFFfree(p); + } return(o); } @@ -2859,7 +2911,7 @@ _TIFFRewriteField(TIFF* tif, uint16 tag, TIFFDataType in_datatype, if( TIFFDataWidth(in_datatype) == 8 && !(tif->tif_flags&TIFF_BIGTIFF) ) { if( in_datatype == TIFF_LONG8 ) - datatype = TIFF_LONG; + datatype = entry_type == TIFF_SHORT ? TIFF_SHORT : TIFF_LONG; else if( in_datatype == TIFF_SLONG8 ) datatype = TIFF_SLONG; else if( in_datatype == TIFF_IFD8 ) @@ -2870,7 +2922,8 @@ _TIFFRewriteField(TIFF* tif, uint16 tag, TIFFDataType in_datatype, else { if( in_datatype == TIFF_LONG8 && - (entry_type == TIFF_LONG || entry_type == TIFF_LONG8 ) ) + (entry_type == TIFF_SHORT || entry_type == TIFF_LONG || + entry_type == TIFF_LONG8 ) ) datatype = entry_type; else if( in_datatype == TIFF_SLONG8 && (entry_type == TIFF_SLONG || entry_type == TIFF_SLONG8 ) ) @@ -2929,6 +2982,23 @@ _TIFFRewriteField(TIFF* tif, uint16 tag, TIFFDataType in_datatype, } } } + else if( datatype == TIFF_SHORT && in_datatype == TIFF_LONG8 ) + { + tmsize_t i; + + for( i = 0; i < count; i++ ) + { + ((uint16 *) buf_to_write)[i] = + (uint16) ((uint64 *) data)[i]; + if( (uint64) ((uint16 *) buf_to_write)[i] != ((uint64 *) data)[i] ) + { + _TIFFfree( buf_to_write ); + TIFFErrorExt( tif->tif_clientdata, module, + "Value exceeds 16bit range of output type." ); + return 0; + } + } + } else { TIFFErrorExt( tif->tif_clientdata, module, diff --git a/test/rewrite_tag.c b/test/rewrite_tag.c index 2a366f78..1708b024 100644 --- a/test/rewrite_tag.c +++ b/test/rewrite_tag.c @@ -30,6 +30,7 @@ #include "tif_config.h" #include +#include #ifdef HAVE_UNISTD_H # include @@ -38,7 +39,6 @@ #include "tiffio.h" #include "tiffiop.h" -const uint32 width = 10; const uint32 length = 40; const uint32 rows_per_strip = 1; @@ -49,6 +49,7 @@ int test_packbits() int i; unsigned char buf[10] = {0,0,0,0,0,0,0,0,0,0}; + uint32 width = 10; int length = 20; const char *filename = "test_packbits.tif"; @@ -136,17 +137,20 @@ int test_packbits() /************************************************************************/ /* rewrite_test() */ /************************************************************************/ -int rewrite_test( const char *filename, int length, int bigtiff, +int rewrite_test( const char *filename, uint32 width, int length, int bigtiff, uint64 base_value ) { TIFF *tif; int i; - unsigned char buf[10] = {5,6,7,8,9,10,11,12,13,14}; + unsigned char *buf; uint64 *rowoffset, *rowbytes; uint64 *upd_rowoffset; uint64 *upd_bytecount; + buf = calloc(1, width); + assert(buf); + /* Test whether we can write tags. */ if( bigtiff ) tif = TIFFOpen(filename, "w8"); @@ -155,6 +159,7 @@ int rewrite_test( const char *filename, int length, int bigtiff, if (!tif) { fprintf (stderr, "Can't create test TIFF file %s.\n", filename); + free(buf); return 1; } @@ -202,6 +207,7 @@ int rewrite_test( const char *filename, int length, int bigtiff, tif = TIFFOpen(filename, "r+"); if (!tif) { fprintf (stderr, "Can't open test TIFF file %s.\n", filename); + free(buf); return 1; } @@ -219,7 +225,7 @@ int rewrite_test( const char *filename, int length, int bigtiff, upd_rowoffset = (uint64 *) _TIFFmalloc(sizeof(uint64) * length); for( i = 0; i < length; i++ ) - upd_rowoffset[i] = base_value + i*10; + upd_rowoffset[i] = base_value + i*width; if( !_TIFFRewriteField( tif, TIFFTAG_STRIPOFFSETS, TIFF_LONG8, length, upd_rowoffset ) ) @@ -232,7 +238,7 @@ int rewrite_test( const char *filename, int length, int bigtiff, upd_bytecount = (uint64 *) _TIFFmalloc(sizeof(uint64) * length); for( i = 0; i < length; i++ ) - upd_bytecount[i] = 100 + i*10; + upd_bytecount[i] = 100 + i*width; if( !_TIFFRewriteField( tif, TIFFTAG_STRIPBYTECOUNTS, TIFF_LONG8, length, upd_bytecount ) ) @@ -250,6 +256,7 @@ int rewrite_test( const char *filename, int length, int bigtiff, tif = TIFFOpen(filename, "r"); if (!tif) { fprintf (stderr, "Can't open test TIFF file %s.\n", filename); + free(buf); return 1; } @@ -261,7 +268,7 @@ int rewrite_test( const char *filename, int length, int bigtiff, for( i = 0; i < length; i++ ) { - uint64 expect = base_value + i*10; + uint64 expect = base_value + i*width; if( rowoffset[i] != expect ) { @@ -284,7 +291,7 @@ int rewrite_test( const char *filename, int length, int bigtiff, for( i = 0; i < length; i++ ) { - uint64 expect = 100 + i*10; + uint64 expect = 100 + i*width; if( rowbytes[i] != expect ) { @@ -300,6 +307,7 @@ int rewrite_test( const char *filename, int length, int bigtiff, } TIFFClose( tif ); + free(buf); /* All tests passed; delete file and exit with success status. */ unlink(filename); @@ -308,6 +316,7 @@ int rewrite_test( const char *filename, int length, int bigtiff, failure: /* Something goes wrong; close file and return unsuccessful status. */ TIFFClose(tif); + free(buf); /* unlink(filename); */ return 1; @@ -325,16 +334,20 @@ main(void) failure |= test_packbits(); /* test fairly normal use */ - failure |= rewrite_test( "rewrite1.tif", 10, 0, 100 ); - failure |= rewrite_test( "rewrite2.tif", 10, 1, 100 ); + failure |= rewrite_test( "rewrite1.tif", 10, 10, 0, 100 ); + failure |= rewrite_test( "rewrite2.tif", 10, 10, 1, 100 ); /* test case of fitting all in directory entry */ - failure |= rewrite_test( "rewrite3.tif", 1, 0, 100 ); - failure |= rewrite_test( "rewrite4.tif", 1, 1, 100 ); + failure |= rewrite_test( "rewrite3.tif", 10, 1, 0, 100 ); + failure |= rewrite_test( "rewrite4.tif", 10, 1, 1, 100 ); /* test with very large values that don't fit in 4bytes (bigtiff only) */ - failure |= rewrite_test( "rewrite5.tif", 1000, 1, 0x6000000000ULL ); - failure |= rewrite_test( "rewrite6.tif", 1, 1, 0x6000000000ULL ); + failure |= rewrite_test( "rewrite5.tif", 10, 1000, 1, 0x6000000000ULL ); + failure |= rewrite_test( "rewrite6.tif", 10, 1, 1, 0x6000000000ULL ); + + /* StripByteCounts on LONG */ + failure |= rewrite_test( "rewrite7.tif", 65536, 1, 0, 100 ); + failure |= rewrite_test( "rewrite8.tif", 65536, 2, 0, 100 ); return failure; }