revise seek() behaviour

fuzz tests fail though ... we need to split StreamInput up
This commit is contained in:
John Cupitt 2019-10-17 17:18:45 +01:00
parent 696ff2b24a
commit f99f3d3f57
9 changed files with 101 additions and 70 deletions

View File

@ -1988,6 +1988,7 @@ vips_foreign_operation_init( void )
extern GType vips_foreign_load_png_stream_get_type( void );
extern GType vips_foreign_save_png_file_get_type( void );
extern GType vips_foreign_save_png_buffer_get_type( void );
extern GType vips_foreign_save_png_stream_get_type( void );
extern GType vips_foreign_load_csv_get_type( void );
extern GType vips_foreign_save_csv_get_type( void );
extern GType vips_foreign_load_matrix_get_type( void );
@ -2023,12 +2024,12 @@ vips_foreign_operation_init( void )
extern GType vips_foreign_save_magick_buffer_get_type( void );
extern GType vips_foreign_save_dz_file_get_type( void );
extern GType vips_foreign_save_dz_buffer_get_type( void );
extern GType vips_foreign_load_webp_stream_get_type( void );
extern GType vips_foreign_load_webp_file_get_type( void );
extern GType vips_foreign_load_webp_buffer_get_type( void );
extern GType vips_foreign_save_webp_stream_get_type( void );
extern GType vips_foreign_load_webp_stream_get_type( void );
extern GType vips_foreign_save_webp_file_get_type( void );
extern GType vips_foreign_save_webp_buffer_get_type( void );
extern GType vips_foreign_save_webp_stream_get_type( void );
extern GType vips_foreign_load_pdf_get_type( void );
extern GType vips_foreign_load_pdf_file_get_type( void );
extern GType vips_foreign_load_pdf_buffer_get_type( void );
@ -2103,11 +2104,12 @@ vips_foreign_operation_init( void )
#endif /*HAVE_GSF*/
#ifdef HAVE_PNG
vips_foreign_load_png_stream_get_type();
vips_foreign_load_png_get_type();
vips_foreign_load_png_buffer_get_type();
vips_foreign_load_png_stream_get_type();
vips_foreign_save_png_file_get_type();
vips_foreign_save_png_buffer_get_type();
vips_foreign_save_png_stream_get_type();
#endif /*HAVE_PNG*/
#ifdef HAVE_MATIO
@ -2115,22 +2117,22 @@ vips_foreign_operation_init( void )
#endif /*HAVE_MATIO*/
#ifdef HAVE_JPEG
vips_foreign_load_jpeg_stream_get_type();
vips_foreign_load_jpeg_file_get_type();
vips_foreign_load_jpeg_buffer_get_type();
vips_foreign_save_jpeg_stream_get_type();
vips_foreign_load_jpeg_stream_get_type();
vips_foreign_save_jpeg_file_get_type();
vips_foreign_save_jpeg_buffer_get_type();
vips_foreign_save_jpeg_stream_get_type();
vips_foreign_save_jpeg_mime_get_type();
#endif /*HAVE_JPEG*/
#ifdef HAVE_LIBWEBP
vips_foreign_load_webp_stream_get_type();
vips_foreign_load_webp_file_get_type();
vips_foreign_load_webp_buffer_get_type();
vips_foreign_save_webp_stream_get_type();
vips_foreign_load_webp_stream_get_type();
vips_foreign_save_webp_file_get_type();
vips_foreign_save_webp_buffer_get_type();
vips_foreign_save_webp_stream_get_type();
#endif /*HAVE_LIBWEBP*/
#ifdef HAVE_TIFF

View File

@ -138,7 +138,7 @@ vips__tiff_openout( const char *path, gboolean bigtiff )
static tsize_t
openin_stream_read( thandle_t st, tdata_t data, tsize_t size )
{
VipsStreamInput *input = (VipsStreamInput *) st;
VipsStreamInput *input = VIPS_STREAM_INPUT( st );
return( vips_stream_input_read( input, data, size ) );
}
@ -154,7 +154,7 @@ openin_stream_write( thandle_t st, tdata_t buffer, tsize_t size )
static toff_t
openin_stream_seek( thandle_t st, toff_t position, int whence )
{
VipsStreamInput *input = (VipsStreamInput *) st;
VipsStreamInput *input = VIPS_STREAM_INPUT( st );
return( vips_stream_input_seek( input, position, whence ) );
}
@ -162,7 +162,7 @@ openin_stream_seek( thandle_t st, toff_t position, int whence )
static int
openin_stream_close( thandle_t st )
{
VipsStreamInput *input = (VipsStreamInput *) st;
VipsStreamInput *input = VIPS_STREAM_INPUT( st );
VIPS_UNREF( input );
@ -172,12 +172,12 @@ openin_stream_close( thandle_t st )
static toff_t
openin_stream_size( thandle_t st )
{
/* Do we need this?
*/
printf( "aaaaargh!!\n" );
g_assert( FALSE );
VipsStreamInput *input = VIPS_STREAM_INPUT( st );
return( 0 );
/* libtiff will use this to get file size if tags like StripByteCounts
* are missing.
*/
return( vips_stream_input_size( input ) );
}
static int

View File

@ -158,8 +158,8 @@ vips__new_output_message( j_common_ptr cinfo )
vips_error( "VipsJpeg", _( "%s" ), buffer );
#ifdef DEBUG
#endif /*DEBUG*/
printf( "vips__new_output_message: \"%s\"\n", buffer );
#endif /*DEBUG*/
/* This is run for things like file truncated. Signal invalidate to
* force this op out of cache.

View File

@ -1188,7 +1188,7 @@ vips__png_write_stream( VipsImage *in, VipsStreamOutput *output,
colours, Q, dither ) ) {
write_finish( write );
vips_error( "vips2png",
"%s", _( "unable to write to buffer" ) );
"%s", _( "unable to write to stream" ) );
return( -1 );
}

View File

@ -132,13 +132,14 @@ typedef struct _VipsStreamInput {
/* TRUE if this descriptor supports mmap(). If not, then we have to
* read() the whole stream if the loader needs the entire image.
*/
gboolean mapable;
gboolean mappable;
/*< private >*/
/* The current read point.
/* The current read point. off_t can be 32 bits on some platforms, so
* make sure we have a full 64.
*/
off_t read_position;
gint64 read_position;
/* Save data read during header phase here. If we rewind and try
* again, serve data from this until it runs out.
@ -185,7 +186,7 @@ typedef struct _VipsStreamInputClass {
/* Seek to a certain position, args exactly as lseek(2).
*/
off_t (*seek)( VipsStreamInput *, off_t offset, int );
gint64 (*seek)( VipsStreamInput *, gint64 offset, int );
/* Shut down anything that can safely restarted. For example, if
* there's a fd that supports lseek(), it can be closed, since later
@ -210,12 +211,13 @@ VipsStreamInput *vips_stream_input_new_from_options( const char *options );
ssize_t vips_stream_input_read( VipsStreamInput *input,
unsigned char *data, size_t length );
const void *vips_stream_input_map( VipsStreamInput *input, size_t *length );
off_t vips_stream_input_seek( VipsStreamInput *input,
off_t offset, int whence );
gint64 vips_stream_input_seek( VipsStreamInput *input,
gint64 offset, int whence );
int vips_stream_input_rewind( VipsStreamInput *input );
void vips_stream_input_minimise( VipsStreamInput *input );
int vips_stream_input_decode( VipsStreamInput *input );
unsigned char *vips_stream_input_sniff( VipsStreamInput *input, size_t length );
gint64 vips_stream_input_size( VipsStreamInput *input );
#define VIPS_TYPE_STREAM_OUTPUT (vips_stream_output_get_type())
#define VIPS_STREAM_OUTPUT( obj ) \

View File

@ -276,7 +276,7 @@ GSList *vips__gslist_gvalue_copy( const GSList *list );
GSList *vips__gslist_gvalue_merge( GSList *a, const GSList *b );
char *vips__gslist_gvalue_get( const GSList *list );
int vips__seek( int fd, gint64 pos );
gint64 vips__seek( int fd, gint64 pos, int whence );
int vips__ftruncate( int fd, gint64 pos );
int vips_existsf( const char *name, ... )
__attribute__((format(printf, 1, 2)));

View File

@ -33,16 +33,27 @@
/* TODO
*
* - catch out of range seeks
* - some sanity thing to prevent endless streams filling memory?
* - filename encoding
* - add seekable input sources
* - need to be able to set seekable and mapable via constructor
* - seek END and _size need thinking about with pipes ... perhaps we should
* force-read the whole thing in
* - _size() needs to be a vfunc too (libtiff needs it)
* - only fetch size once
* - revise logic once we have all the use cases nailed down ... split to:
* + base class, just contain interface and perhaps sniff support
* + memory input subclass
* + mappable input subclass,
* + seekable file input subclass
* + pipe input subclass (uses header cache)
* - need to be able to set seekable and mappable via constructor
* - test we can really change all behaviour in the subclass ... add callbacks
* as well to make it simpler for language bindings
*/
/*
#define VIPS_DEBUG
*/
#define VIPS_DEBUG
#ifdef HAVE_CONFIG_H
#include <config.h>
@ -213,7 +224,6 @@ vips_stream_input_open( VipsStreamInput *input )
stream->tracked_descriptor == -1 &&
stream->filename ) {
int fd;
off_t new_pos;
if( (fd = vips_tracked_open( stream->filename,
MODE_READ )) == -1 ) {
@ -227,13 +237,9 @@ vips_stream_input_open( VipsStreamInput *input )
VIPS_DEBUG_MSG( "vips_stream_input_open: "
"restoring read position %zd\n", input->read_position );
new_pos = lseek( stream->descriptor,
input->read_position, SEEK_SET );
if( new_pos == -1 ) {
vips_error_system( errno, STREAM_NAME( stream ),
"%s", _( "unable to lseek()" ) );
return( 0 );
}
if( vips__seek( stream->descriptor,
input->read_position, SEEK_SET ) == -1 )
return( -1 );
}
return( 0 );
@ -273,11 +279,11 @@ vips_stream_input_build( VipsObject *object )
/* Do +=0 on the current position. This fails for pipes, at
* least on linux.
*/
if( lseek( stream->descriptor, 0, SEEK_CUR ) != -1 )
if( vips__seek( stream->descriptor, 0, SEEK_CUR ) != -1 )
input->seekable = TRUE;
if( vips__mmap_supported( stream->descriptor ) )
input->mapable = TRUE;
input->mappable = TRUE;
}
if( vips_object_argument_isset( object, "blob" ) )
@ -304,7 +310,7 @@ vips_stream_input_read_real( VipsStreamInput *input,
VIPS_DEBUG_MSG( "vips_stream_input_read_real:\n" );
if( input->blob ) {
VipsArea *area = (VipsArea *) input->blob;
VipsArea *area = VIPS_AREA( input->blob );
ssize_t available = VIPS_MIN( length,
area->length - input->read_position );
@ -345,13 +351,11 @@ vips_stream_input_map_real( VipsStreamInput *input, size_t *length )
return( file_baseaddr );
}
static off_t
vips_stream_input_seek_real( VipsStreamInput *input, off_t offset, int whence )
static gint64
vips_stream_input_seek_real( VipsStreamInput *input, gint64 offset, int whence )
{
VipsStream *stream = VIPS_STREAM( input );
off_t new_pos;
VIPS_DEBUG_MSG( "vips_stream_input_seek_real:\n" );
if( !input->seekable ||
@ -360,14 +364,7 @@ vips_stream_input_seek_real( VipsStreamInput *input, off_t offset, int whence )
return( -1 );
}
new_pos = lseek( stream->descriptor, offset, whence );
if( new_pos == -1 ) {
vips_error_system( errno, STREAM_NAME( stream ),
"%s", _( "seek error" ) );
return( -1 );
}
return( new_pos );
return( vips__seek( stream->descriptor, offset, whence ) );
}
static void
@ -605,11 +602,11 @@ vips_stream_input_read( VipsStreamInput *input,
return( -1 );
}
/* If we're not seekable, we need to save header bytes for
* reuse.
/* We need to save bytes if we're in header mode and we can't
* seek or map.
*/
if( input->header_bytes &&
!input->seekable &&
(!input->seekable || !input->mappable) &&
!input->decode &&
n > 0 )
g_byte_array_append( input->header_bytes,
@ -644,7 +641,7 @@ vips_stream_input_map( VipsStreamInput *input, size_t *length )
/* An input that supports mmap.
*/
if( input->mapable ) {
if( input->mappable ) {
VIPS_DEBUG_MSG( " mmaping source\n" );
if( !input->baseaddr ) {
input->baseaddr = class->map( input, &input->length );
@ -673,12 +670,12 @@ vips_stream_input_map( VipsStreamInput *input, size_t *length )
return( input->header_bytes->data );
}
off_t
vips_stream_input_seek( VipsStreamInput *input, off_t offset, int whence )
gint64
vips_stream_input_seek( VipsStreamInput *input, gint64 offset, int whence )
{
VipsStreamInputClass *class = VIPS_STREAM_INPUT_GET_CLASS( input );
off_t new_pos;
gint64 new_pos;
VIPS_DEBUG_MSG( "vips_stream_input_seek:\n" );
@ -692,7 +689,9 @@ vips_stream_input_seek( VipsStreamInput *input, off_t offset, int whence )
break;
case SEEK_END:
/* TODO .. I don't think any loader needs SEEK_END.
/* TODO .. I don't think any loader needs SEEK_END, and
* implementing it on pipes would force us to read the whole
* thing in.
*/
default:
vips_error( STREAM_NAME( input ), "%s", _( "bad 'whence'" ) );
@ -715,9 +714,13 @@ vips_stream_input_seek( VipsStreamInput *input, off_t offset, int whence )
*/
while( input->read_position < new_pos ) {
unsigned char buffer[4096];
ssize_t read;
if( vips_stream_input_read( input, buffer, 4096 ) )
read = vips_stream_input_read( input, buffer, 4096 );
if( read < 0 )
return( -1 );
if( read == 0 ) {
}
}
}
else if( input->blob ||
@ -727,8 +730,7 @@ vips_stream_input_seek( VipsStreamInput *input, off_t offset, int whence )
;
}
else {
new_pos = class->seek( input, offset, whence );
if( new_pos == -1 )
if( (new_pos = class->seek( input, offset, whence )) == -1 )
return( -1 );
}
@ -806,6 +808,24 @@ vips_stream_input_sniff( VipsStreamInput *input, size_t length )
return( input->sniff->data );
}
gint64
vips_stream_input_size( VipsStreamInput *input )
{
VipsStream *stream = VIPS_STREAM( input );
gint64 size;
if( stream->descriptor >= 0 &&
(size = vips_file_length( stream->descriptor )) >= 0 )
return( size );
else if( input->blob )
return( VIPS_AREA( input->blob )->length );
else if( input->header_bytes )
return( input->header_bytes->len );
else
return( -1 );
}
G_DEFINE_TYPE( VipsStreamOutput, vips_stream_output, VIPS_TYPE_STREAM );
static void

View File

@ -1030,29 +1030,36 @@ vips__gslist_gvalue_get( const GSList *list )
/* Need our own seek(), since lseek() on win32 can't do long files.
*/
int
vips__seek( int fd, gint64 pos )
gint64
vips__seek( int fd, gint64 pos, int whence )
{
gint64 new_pos;
#ifdef OS_WIN32
{
HANDLE hFile = (HANDLE) _get_osfhandle( fd );
LARGE_INTEGER p;
LARGE_INTEGER q;
/* Whence uses the same numbering on win32 and posix.
*/
p.QuadPart = pos;
if( !SetFilePointerEx( hFile, p, NULL, FILE_BEGIN ) ) {
if( !SetFilePointerEx( hFile, p, &q, whence ) ) {
vips_error_system( GetLastError(), "vips__seek",
"%s", _( "unable to seek" ) );
return( -1 );
}
new_pos = q.QuadPart;
}
#else /*!OS_WIN32*/
if( lseek( fd, pos, SEEK_SET ) == (off_t) -1 ) {
if( (new_pos = lseek( fd, pos, whence )) == (off_t) -1 ) {
vips_error( "vips__seek", "%s", _( "unable to seek" ) );
return( -1 );
}
#endif /*OS_WIN32*/
return( 0 );
return( new_pos );
}
/* Need our own ftruncate(), since ftruncate() on win32 can't do long files.

View File

@ -485,7 +485,7 @@ read_chunk( int fd, gint64 offset, size_t length )
{
char *buf;
if( vips__seek( fd, offset ) )
if( vips__seek( fd, offset, SEEK_SET ) == -1 )
return( NULL );
if( !(buf = vips_malloc( NULL, length + 1 )) )
return( NULL );
@ -755,7 +755,7 @@ readhist( VipsImage *im )
XML_Parser parser;
VipsExpatParse vep;
if( vips__seek( im->fd, image_pixel_length( im ) ) )
if( vips__seek( im->fd, image_pixel_length( im ), SEEK_SET ) == -1 )
return( -1 );
parser = XML_ParserCreate( "UTF-8" );
@ -797,7 +797,7 @@ vips__write_extension_block( VipsImage *im, void *buf, int size )
}
if( vips__ftruncate( im->fd, psize ) ||
vips__seek( im->fd, psize ) )
vips__seek( im->fd, psize, SEEK_SET ) == -1 )
return( -1 );
if( vips__write( im->fd, buf, size ) )
return( -1 );
@ -1032,7 +1032,7 @@ vips_image_open_input( VipsImage *image )
return( -1 );
}
vips__seek( image->fd, 0 );
vips__seek( image->fd, 0, SEEK_SET );
if( read( image->fd, header, VIPS_SIZEOF_HEADER ) !=
VIPS_SIZEOF_HEADER ||
vips__read_header_bytes( image, header ) ) {