CreateCopy crashes when creating a virtual copy of a vrt data set

[mauceri]

Hi,

I'm from the ACT developement team.

I have a VRT data set file pointing to a PNG source file. I want to add some Metadata to the VRT file.

In order to do this I'm creating a virtual data set from the VRT data set in the following way:

[...]

GDALDriver *poDriver;
GDALDataset *poSrcDS, *poVirtualDS;

poDriver = GetGDALDriverManager()->GetDriverByName("VRT");
if( poDriver == NULL )
   return;

// open the source data set
poSrcDS = (GDALDataset *) GDALOpenShared( "file.png.vrt", GA_ReadOnly );
if( !poSrcDS )
   return;
    
// create the new virtual data set
poVirtualDS = poDriver->CreateCopy( "", poSrcDS, FALSE, NULL, NULL, NULL );
if( poVirtualDS == NULL ){
   GDALClose(poSrcDS);
   return;
}

[...]

The problem is that GDAL crashes when the createCopy for the virtual data set is executed.

I'm using GDAL 1.4.1, but the problem is happening also with GDAL 1.3

[warmerdam]

Mateusz,

Please try and reproduce this with trunk and/or 1.4.3 and find a fix. Let me know if you have any questions or concerns.

[Mateusz Łoskot]

Frank,

The segmentation fault reported in this ticket is caused by NULL file pointer passed to VSIFWrite function (vrtdriver.cpp:185).

The file pointer is NULL, because empty filename used in client's code:

poVirtualDS = poDriver->CreateCopy( "", poSrcDS, FALSE, NULL, NULL, NULL );

and VSIFOpen called earlier fails back with NULL pointer.

Temporarily, I patched the vrtdataset.cpp file with some tests, but I understand my patch is even wrong because empty filename is (in some cases) accepted. Here is explanation of that part:

Why the user's code crashes when ​VRT Tutorial uses empty filename with CreateCopy() operation?

It fails because VRT driver is used for both source and target datasets. The CreateCopy operation for VRT checks if the source dataset is VRT and if it is, then it attempts to just copy given source VRT dataset (.vrt file) to using new filename. Obviously, the operation fails if empty filename is given.

According to current implementation, CreateCopy() assumes following requirements:

• If VRT driver is used for both source and target datasets, then non-empty and valid filename is required by CreateCopy() operation.

// vrtdataset.cpp:153
if( EQUAL(poSrcDS->GetDriver()->GetDescription(),"VRT")
    && EQUAL(this->GetDescription(),"VRT") )
{
  CPLAssert( strlen(pszFilename) != 0 );
  ...
}

• If target dataset uses VRT driver, but source data set uses non-VRT driver, then empty filename is allowed and in-memory dataset (in VRT format) is created.

// vrtdataset.cpp, else part of block starting in line 153
else if( EQUAL(this->GetDescription(),"VRT") )
{
  CPLAssert( ! EQUAL(poSrcDS->GetDriver()->GetDescription(),"VRT") );

  // 0 == strlen(pszFilename) is allowed,
  // so in-memory dataset is created
  ...
}

This is my understanding of how VRT stuff works.

BTW, if I'm correct, I believe the VRT tutorial should be updated accordingly.

[warmerdam]

Mateusz,

The case where the source driver is VRT in CreateCopy() should have a special check added for a destination filename of "", and in that case no effort should be made to write the serialized XML to a file. Instead the serialized XML should be passed into GDALOpen() as the filename.

Make sure a test of this case makes it into the test suite.

[Mateusz Łoskot]

Simple test presenting the VRTCreateCopy fix in action.

[Mateusz Łoskot]

I've applied fix for this bug (r11909).

I assume the bug has been fixed correctly and closing it, but if it hasn't, please reopen the ticket.

[Mateusz Łoskot]

I've ported this fix to branches/1.4 (r12515)