start trying to fix up blob/string mixups

we often set strings as blobs (eg. libtiff uses pointer/length for
XML and does not null-terminate) ... when do we validate stringiness,
and add the missing null?
This commit is contained in:
John Cupitt 2018-11-22 09:15:57 +00:00
parent 572e37a314
commit 8fa3b0aa1c
5 changed files with 131 additions and 57 deletions

9
TODO
View File

@ -1,3 +1,12 @@
- use set_blob_copy everywhere we can
- argh we now have two type sof BLOB, with and without invisible null
termination, with no way to tell them apart
- try
$ vips gaussmat x.mat 0.1 0.1

View File

@ -1352,74 +1352,44 @@ rtiff_set_header( Rtiff *rtiff, VipsImage *out )
/* Read any ICC profile.
*/
if( TIFFGetField( rtiff->tiff,
TIFFTAG_ICCPROFILE, &data_length, &data ) &&
data &&
data_length ) {
void *data_copy;
if( !(data_copy = vips_malloc( NULL, data_length )) )
TIFFTAG_ICCPROFILE, &data_length, &data ) ) {
if( vips_image_set_blob_copy( rtiff, out,
VIPS_META_ICC_NAME, data_length, data ) )
return( -1 );
memcpy( data_copy, data, data_length );
vips_image_set_blob( out, VIPS_META_ICC_NAME,
(VipsCallbackFn) vips_free, data_copy, data_length );
}
/* Read any XMP metadata.
*/
if( TIFFGetField( rtiff->tiff,
TIFFTAG_XMLPACKET, &data_length, &data ) &&
data &&
data_length ) {
void *data_copy;
if( !(data_copy = vips_malloc( NULL, data_length )) )
TIFFTAG_XMLPACKET, &data_length, &data ) ) {
if( vips_image_set_blob_copy( rtiff, out,
VIPS_META_XMP_NAME, data_length, data ) )
return( -1 );
memcpy( data_copy, data, data_length );
vips_image_set_blob( out, VIPS_META_XMP_NAME,
(VipsCallbackFn) vips_free, data_copy, data_length );
}
/* Read any IPTC metadata.
*/
if( TIFFGetField( rtiff->tiff,
TIFFTAG_RICHTIFFIPTC, &data_length, &data ) &&
data &&
data_length ) {
void *data_copy;
/* libtiff stores IPTC as an array of long, not byte.
*/
data_length *= 4;
if( !(data_copy = vips_malloc( NULL, data_length )) )
TIFFTAG_RICHTIFFIPTC, &data_length, &data ) ) {
if( vips_image_set_blob_copy( rtiff, out,
VIPS_META_IPTC_NAME, data_length, data ) )
return( -1 );
memcpy( data_copy, data, data_length );
vips_image_set_blob( out, VIPS_META_IPTC_NAME,
(VipsCallbackFn) vips_free, data_copy, data_length );
/* Older versions of libvips used this misspelt name :-( attach
* under this name too for compatibility.
*/
if( !(data_copy = vips_malloc( NULL, data_length )) )
if( vips_image_set_blob_copy( rtiff, out,
"ipct-data", data_length, data ) )
return( -1 );
memcpy( data_copy, data, data_length );
vips_image_set_blob( out, "ipct-data",
(VipsCallbackFn) vips_free, data_copy, data_length );
}
/* Read any photoshop metadata.
*/
if( TIFFGetField( rtiff->tiff,
TIFFTAG_PHOTOSHOP, &data_length, &data ) &&
data &&
data_length ) {
void *data_copy;
if( !(data_copy = vips_malloc( NULL, data_length )) )
TIFFTAG_PHOTOSHOP, &data_length, &data ) ) {
if( vips_image_set_blob_copy( rtiff, out,
VIPS_META_PHOTOSHOP_NAME, data_length, data ) )
return( -1 );
memcpy( data_copy, data, data_length );
vips_image_set_blob( out, VIPS_META_PHOTOSHOP_NAME,
(VipsCallbackFn) vips_free, data_copy, data_length );
}
/* IMAGEDESCRIPTION often has useful metadata.

View File

@ -71,6 +71,8 @@
* - support png8 palette write with palette, colours, Q, dither
* 25/8/18
* - support xmp read/write
* 21/11/18
* - save xmp as string, not blob
*/
/*
@ -267,7 +269,8 @@ read_new_filename( VipsImage *out, const char *name, gboolean fail )
return( read );
}
/* Set the png text data as metadata on the vips image.
/* Set the png text data as metadata on the vips image. These are always
* null-terminated strings.
*/
static int
vips__set_text( VipsImage *out, int i, const char *key, const char *text )
@ -1104,7 +1107,6 @@ write_vips( Write *write,
if( vips_image_get_string( in, VIPS_META_XMP_NAME, &str ) )
return( -1 );
vips__png_set_text( write->pPng, write->pInfo,
"XML:com.adobe.xmp", str );
}

View File

@ -193,8 +193,10 @@ void vips_image_set_area( VipsImage *image,
const char *name, VipsCallbackFn free_fn, void *data );
int vips_image_get_area( const VipsImage *image,
const char *name, void **data );
void vips_image_set_blob( VipsImage *image, const char *name,
VipsCallbackFn free_fn, void *data, size_t length );
void vips_image_set_blob( VipsImage *image,
const char *name, VipsCallbackFn free_fn, void *data, size_t length );
void vips_image_set_blob_copy( VipsImage *image,
const char *name, void *data, size_t length );
int vips_image_get_blob( const VipsImage *image, const char *name,
void **data, size_t *length );

View File

@ -26,6 +26,8 @@
* - return header enums as enums, not ints
* - vips_image_get_*() all convert everything to target type if they can
* - rename "field" as "name" in docs
* 21/11/18
* - get_string will allow BLOB, G_STRING and REF_STRING
*/
/*
@ -1347,8 +1349,8 @@ vips_image_get_area( const VipsImage *image, const char *name, void **data )
* See also: vips_image_get_blob(), vips_image_set().
*/
void
vips_image_set_blob( VipsImage *image, const char *name,
VipsCallbackFn free_fn, void *data, size_t length )
vips_image_set_blob( VipsImage *image,
const char *name, VipsCallbackFn free_fn, void *data, size_t length )
{
GValue value = { 0 };
@ -1358,6 +1360,70 @@ vips_image_set_blob( VipsImage *image, const char *name,
g_value_unset( &value );
}
/**
* vips_image_set_blob_copy: (method)
* @image: image to attach the metadata to
* @name: metadata name
* @data: pointer to area of memory
* @length: length of memory area
*
* Attaches @blob as a metadata item on @image under the name @name, taking
* a copy of the memory area. A convenience function over vips_image_set_blob().
*
* One more byte is allocated and a secret null added to the end, although
* this extra length is not recorded. This makes reading the value out later
* as a C string safer and more convenient.
*
* See also: vips_image_get_blob(), vips_image_set().
*/
void
vips_image_set_blob_copy( VipsImage *image,
const char *name, void *data, size_t length )
{
void *data_copy;
if( !data ||
length == 0 )
return( 0 );
if( !(data_copy = vips_malloc( NULL, length + 1 )) )
return( -1 );
memcpy( data_copy, data, length );
((unsigned char *) data_copy)[length] = '\0';
vips_image_set_blob( out, field,
(VipsCallbackFn) vips_free, data_copy, length );
return( 0 );
}
/* Set a blob on an image from a libtiff pointer/length. We don't null
* terminate, even though most of these things are strings, because we want to
* be able to write them back unaltered.
*
* Null-termination, checking for utf-8, checking for embedded null characters
* etc. must all be done on readout from the blob.
*/
static int
rtiff_set_blob( Rtiff *rtiff, VipsImage *out, const char *field,
uint32 data_length, unsigned char *data )
{
unsigned char *data_copy;
if( !data ||
data_length == 0 )
return( 0 );
if( !(data_copy = vips_malloc( NULL, data_length )) )
return( -1 );
memcpy( data_copy, data, data_length );
vips_image_set_blob( out, field,
(VipsCallbackFn) vips_free, data_copy, data_length );
return( 0 );
}
/**
* vips_image_get_blob: (method)
* @image: image to get the metadata from
@ -1498,7 +1564,8 @@ vips_image_set_double( VipsImage *image, const char *name, double d )
*
* Gets @out from @im under the name @name.
* The field must be of type
* VIPS_TYPE_REFSTRING.
* G_STRING, VIPS_TYPE_REFSTRING or VIPS_TYPE_BLOB. If it's a BLOB, it must be
* null-terminated.
*
* Do not free @out.
*
@ -1513,21 +1580,45 @@ vips_image_get_string( const VipsImage *image, const char *name,
const char **out )
{
GValue value = { 0 };
VipsArea *area;
if( vips_image_get( image, name, &value ) )
return( -1 );
if( G_VALUE_TYPE( &value ) != VIPS_TYPE_REF_STRING ) {
if( G_VALUE_TYPE( &value ) == VIPS_TYPE_BLOB ) {
char *str;
size_t length;
str = vips_value_get_blob( &value, &length );
if( length <= 0 ||
str[length] != '\0' ) {
g_value_unset( &value );
vips_error( "VipsImage",
_( "field \"%s\" is not null-terminated" ),
name );
return( -1 );
}
*out = str;
}
else if( G_VALUE_TYPE( &value ) == VIPS_TYPE_REF_STRING ) {
VipsArea *area;
area = g_value_get_boxed( &value );
*out = area->data;
}
else if( G_VALUE_TYPE( &value ) == G_TYPE_STRING ) {
*out = g_value_get_string( &value );
}
else {
vips_error( "VipsImage",
_( "field \"%s\" is of type %s, not VipsRefString" ),
name,
g_type_name( G_VALUE_TYPE( &value ) ) );
g_value_unset( &value );
return( -1 );
}
area = g_value_get_boxed( &value );
*out = area->data;
g_value_unset( &value );
return( 0 );
@ -1541,9 +1632,9 @@ vips_image_get_string( const VipsImage *image, const char *name,
*
* Attaches @str as a metadata item on @image as @name.
* A convenience
* function over vips_image_set() using an vips_ref_string.
* function over vips_image_set() using #VIPS_TYPE_REF_STRING.
*
* See also: vips_image_get_double(), vips_image_set(), vips_ref_string
* See also: vips_image_get_double(), vips_image_set().
*/
void
vips_image_set_string( VipsImage *image, const char *name, const char *str )