fix VipsSource and named pipes

We used to assume (in several places) that any source with a filename was
seekable. This patch adds a is_file test, and makes all the loaders use it.

see https://github.com/libvips/libvips/issues/2467
This commit is contained in:
John Cupitt 2021-10-08 12:20:24 +01:00
parent 60bae63644
commit f6281284a1
8 changed files with 143 additions and 83 deletions

View File

@ -10,6 +10,7 @@
- flatten handles out of range alpha and max_alpha correctly
- don't use atexit for cleanup, it's too unreliable
- tiff writer loops for the whole image rather than per page [LionelArn2]
- fix VipsSource with named pipes [vibbix]
16/8/21 started 8.11.4
- fix off-by-one error in new rank fast path

View File

@ -88,17 +88,22 @@ vips_foreign_load_fits_build( VipsObject *object )
VipsForeignLoadFits *fits =
(VipsForeignLoadFits *) object;
/* We can only open source which have an associated filename, since
/* We can only open sources which have an associated filename, since
* the fits library works in terms of filenames.
*/
if( fits->source ) {
fits->filename = vips_connection_filename( VIPS_CONNECTION(
fits->source ) );
if( !fits->filename ) {
VipsConnection *connection = VIPS_CONNECTION( fits->source );
const char *filename;
if( !vips_source_is_file( fits->source ) ||
!(filename = vips_connection_filename( connection )) ) {
vips_error( class->nickname, "%s",
_( "no filename available" ) );
return( -1 );
}
fits->filename = filename;
}
if( VIPS_OBJECT_CLASS( vips_foreign_load_fits_parent_class )->
@ -296,10 +301,12 @@ vips_foreign_load_fits_source_build( VipsObject *object )
static gboolean
vips_foreign_load_fits_source_is_a_source( VipsSource *source )
{
VipsConnection *connection = VIPS_CONNECTION( source );
const char *filename;
return( (filename =
vips_connection_filename( VIPS_CONNECTION( source ) )) &&
return( vips_source_is_file( source ) &&
(filename = vips_connection_filename( connection )) &&
vips__fits_isfits( filename ) );
}

View File

@ -121,13 +121,18 @@ vips_foreign_load_nifti_build( VipsObject *object )
* the nifti library works in terms of filenames.
*/
if( nifti->source ) {
nifti->filename = vips_connection_filename( VIPS_CONNECTION(
nifti->source ) );
if( !nifti->filename ) {
vips_error( class->nickname, "%s",
_( "no filename available" ) );
VipsConnection *connection = VIPS_CONNECTION( nifti->source );
const char *filename;
if( !vips_source_is_file( nifti->source ) ||
!(filename = vips_connection_filename( connection )) ) {
vips_error( class->nickname,
"%s", _( "no filename available" ) );
return( -1 );
}
nifti->filename = filename;
}
if( VIPS_OBJECT_CLASS( vips_foreign_load_nifti_parent_class )->
@ -747,10 +752,12 @@ vips_foreign_load_nifti_source_build( VipsObject *object )
static gboolean
vips_foreign_load_nifti_source_is_a_source( VipsSource *source )
{
VipsConnection *connection = VIPS_CONNECTION( source );
const char *filename;
return( (filename =
vips_connection_filename( VIPS_CONNECTION( source ) )) &&
return( vips_source_is_file( source ) &&
(filename = vips_connection_filename( connection )) &&
vips_foreign_load_nifti_is_a( filename ) );
}

View File

@ -761,14 +761,19 @@ vips_foreign_load_openslide_build( VipsObject *object )
* the openslide library works in terms of filenames.
*/
if( openslide->source ) {
openslide->filename =
vips_connection_filename( VIPS_CONNECTION(
openslide->source ) );
if( !openslide->filename ) {
VipsConnection *connection =
VIPS_CONNECTION( openslide->source );
const char *filename;
if( !vips_source_is_file( openslide->source ) ||
!(filename = vips_connection_filename( connection )) ) {
vips_error( class->nickname, "%s",
_( "no filename available" ) );
return( -1 );
}
openslide->filename = filename;
}
if( VIPS_OBJECT_CLASS( vips_foreign_load_openslide_parent_class )->
@ -1033,10 +1038,12 @@ vips_foreign_load_openslide_source_build( VipsObject *object )
static gboolean
vips_foreign_load_openslide_source_is_a_source( VipsSource *source )
{
VipsConnection *connection = VIPS_CONNECTION( source );
const char *filename;
return( (filename =
vips_connection_filename( VIPS_CONNECTION( source ) )) &&
return( vips_source_is_file( source ) &&
(filename = vips_connection_filename( connection )) &&
vips__openslide_isslide( filename ) );
}

View File

@ -873,7 +873,7 @@ rad2vips_get_header( Read *read, VipsImage *out )
interpretation,
1, read->aspect );
VIPS_SETSTR( out->filename,
VIPS_SETSTR( out->filename,
vips_connection_filename(
VIPS_CONNECTION( read->sbuf->source ) ) );

View File

@ -110,31 +110,24 @@ vips_foreign_load_vips_get_flags_filename( const char *filename )
static int
vips_foreign_load_vips_header( VipsForeignLoad *load )
{
VipsObjectClass *class = VIPS_OBJECT_GET_CLASS( load );
VipsForeignLoadVips *vips = (VipsForeignLoadVips *) load;
VipsConnection *connection = VIPS_CONNECTION( vips->source );
const char *filename;
VipsImage *image;
VipsImage *x;
if( (filename =
vips_connection_filename( VIPS_CONNECTION( vips->source ) )) ) {
if( !(image = vips_image_new_mode( filename, "r" )) )
return( -1 );
}
else {
VipsObjectClass *class = VIPS_OBJECT_GET_CLASS( load );
/* We could add load vips from memory, fd, via mmap etc. here.
* We should perhaps move iofuncs/vips.c into this file.
*
* For now, just fail unless there's a filename associated
* with this source.
*/
if( !vips_source_is_file( vips->source ) ||
!(filename = vips_connection_filename( connection )) ) {
vips_error( class->nickname,
"%s", _( "no filename associated with source" ) );
return( -1 );
}
if( !(image = vips_image_new_mode( filename, "r" )) )
return( -1 );
/* What a hack. Remove the @out that's there now and replace it with
* our image.
*/
@ -280,10 +273,12 @@ vips_foreign_load_vips_source_build( VipsObject *object )
static gboolean
vips_foreign_load_vips_source_is_a_source( VipsSource *source )
{
VipsConnection *connection = VIPS_CONNECTION( source );
const char *filename;
return( (filename =
vips_connection_filename( VIPS_CONNECTION( source ) )) &&
return( vips_source_is_file( source ) &&
(filename = vips_connection_filename( connection )) &&
vips__file_magic( filename ) );
}

View File

@ -218,6 +218,7 @@ int vips_source_unminimise( VipsSource *source );
int vips_source_decode( VipsSource *source );
gint64 vips_source_read( VipsSource *source, void *data, size_t length );
gboolean vips_source_is_mappable( VipsSource *source );
gboolean vips_source_is_file( VipsSource *source );
const void *vips_source_map( VipsSource *source, size_t *length );
VipsBlob *vips_source_map_blob( VipsSource *source );
gint64 vips_source_seek( VipsSource *source, gint64 offset, int whence );

View File

@ -10,6 +10,8 @@
* 26/11/20
* - use _setmode() on win to force binary read for previously opened
* descriptors
* 8/10/21
* - fix named pipes
*/
/*
@ -129,6 +131,52 @@ vips_pipe_read_limit_set( gint64 limit )
G_DEFINE_TYPE( VipsSource, vips_source, VIPS_TYPE_CONNECTION );
/* Does this source support seek. You must unminimise before calling this.
*/
static int
vips_source_test_seek( VipsSource *source )
{
if( !source->have_tested_seek ) {
VipsSourceClass *class = VIPS_SOURCE_GET_CLASS( source );
source->have_tested_seek = TRUE;
VIPS_DEBUG_MSG( "vips_source_can_seek: testing seek ..\n" );
/* Can we seek this input?
*
* We need to call the method directly rather than via
* vips_source_seek() etc. or we might trigger seek emulation.
*/
if( source->data ||
class->seek( source, 0, SEEK_CUR ) != -1 ) {
gint64 length;
VIPS_DEBUG_MSG( " seekable source\n" );
/* We should be able to get the length of seekable
* objects.
*/
if( (length = vips_source_length( source )) == -1 )
return( -1 );
source->length = length;
/* If we can seek, we won't need to save header bytes.
*/
VIPS_FREEF( g_byte_array_unref, source->header_bytes );
}
else {
/* Not seekable. This must be some kind of pipe.
*/
VIPS_DEBUG_MSG( " not seekable\n" );
source->is_pipe = TRUE;
}
}
return( 0 );
}
/* We can't test for seekability or length during _build, since the read and
* seek signal handlers might not have been connected yet. Instead, we test
* when we first need to know.
@ -136,49 +184,10 @@ G_DEFINE_TYPE( VipsSource, vips_source, VIPS_TYPE_CONNECTION );
static int
vips_source_test_features( VipsSource *source )
{
VipsSourceClass *class = VIPS_SOURCE_GET_CLASS( source );
if( source->have_tested_seek )
return( 0 );
source->have_tested_seek = TRUE;
VIPS_DEBUG_MSG( "vips_source_test_features: testing seek ..\n" );
/* We'll need a descriptor to test.
*/
if( vips_source_unminimise( source ) )
if( vips_source_unminimise( source ) ||
vips_source_test_seek( source ) )
return( -1 );
/* Can we seek this input?
*
* We need to call the method directly rather than via
* vips_source_seek() etc. or we might trigger seek emulation.
*/
if( source->data ||
class->seek( source, 0, SEEK_CUR ) != -1 ) {
gint64 length;
VIPS_DEBUG_MSG( " seekable source\n" );
/* We should be able to get the length of seekable
* objects.
*/
if( (length = vips_source_length( source )) == -1 )
return( -1 );
source->length = length;
/* If we can seek, we won't need to save header bytes.
*/
VIPS_FREEF( g_byte_array_unref, source->header_bytes );
}
else {
/* Not seekable. This must be some kind of pipe.
*/
VIPS_DEBUG_MSG( " not seekable\n" );
source->is_pipe = TRUE;
}
return( 0 );
}
@ -634,12 +643,19 @@ vips_source_unminimise( VipsSource *source )
connection->tracked_descriptor = fd;
connection->descriptor = fd;
VIPS_DEBUG_MSG( "vips_source_unminimise: "
"restoring read position %" G_GINT64_FORMAT "\n",
source->read_position );
if( vips__seek( connection->descriptor,
source->read_position, SEEK_SET ) == -1 )
return( -1 );
if( vips_source_test_seek( source ) )
return( -1 );
/* It might be a named pipe.
*/
if( !source->is_pipe ) {
VIPS_DEBUG_MSG( "vips_source_unminimise: restoring "
"read position %" G_GINT64_FORMAT "\n",
source->read_position );
if( vips__seek( connection->descriptor,
source->read_position, SEEK_SET ) == -1 )
return( -1 );
}
}
return( 0 );
@ -976,6 +992,32 @@ vips_source_is_mappable( VipsSource *source )
VIPS_CONNECTION( source )->descriptor != -1) );
}
/**
* vips_source_is_file:
* @source: source to operate on
*
* Test if this source is a simple file with support for seek. Named pipes,
* for example, will fail this test. If TRUE, you can use
* vips_connection_filename() to find the filename.
*
* Use this to add basic source support for older loaders which can only work
* on files.
*
* Returns: %TRUE if the source is a simple file.
*/
gboolean
vips_source_is_file( VipsSource *source )
{
if( vips_source_unminimise( source ) ||
vips_source_test_features( source ) )
return( -1 );
/* There's a filename, and it supports seek.
*/
return( VIPS_CONNECTION( source )->filename &&
!source->is_pipe );
}
/**
* vips_source_map:
* @source: source to operate on