Skip to content

Commit

Permalink
Merge pull request #11258 from rouault/fix_11257
Browse files Browse the repository at this point in the history
Raster API: error out on GDT_Unknown/GDT_TypeCount
  • Loading branch information
rouault authored Nov 13, 2024
2 parents d1ab669 + 7ce56cb commit a8642db
Show file tree
Hide file tree
Showing 14 changed files with 183 additions and 37 deletions.
35 changes: 35 additions & 0 deletions autotest/cpp/test_gdal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4454,6 +4454,41 @@ TEST_F(test_gdal, gdal_gcp_class)
}
}

TEST_F(test_gdal, RasterIO_gdt_unknown)
{
GDALDatasetUniquePtr poDS(GDALDriver::FromHandle(GDALGetDriverByName("MEM"))
->Create("", 1, 1, 1, GDT_Float64, nullptr));
CPLErrorHandlerPusher oErrorHandler(CPLQuietErrorHandler);
GByte b = 0;
GDALRasterIOExtraArg sExtraArg;
INIT_RASTERIO_EXTRA_ARG(sExtraArg);
EXPECT_EQ(poDS->RasterIO(GF_Read, 0, 0, 1, 1, &b, 1, 1, GDT_Unknown, 1,
nullptr, 0, 0, 0, &sExtraArg),
CE_Failure);
EXPECT_EQ(poDS->RasterIO(GF_Read, 0, 0, 1, 1, &b, 1, 1, GDT_TypeCount, 1,
nullptr, 0, 0, 0, &sExtraArg),
CE_Failure);
EXPECT_EQ(poDS->GetRasterBand(1)->RasterIO(GF_Read, 0, 0, 1, 1, &b, 1, 1,
GDT_Unknown, 0, 0, &sExtraArg),
CE_Failure);
EXPECT_EQ(poDS->GetRasterBand(1)->RasterIO(GF_Read, 0, 0, 1, 1, &b, 1, 1,
GDT_TypeCount, 0, 0, &sExtraArg),
CE_Failure);
}

TEST_F(test_gdal, CopyWords_gdt_unknown)
{
CPLErrorHandlerPusher oErrorHandler(CPLQuietErrorHandler);
GByte b = 0;
GByte b2 = 0;
CPLErrorReset();
GDALCopyWords(&b, GDT_Byte, 0, &b2, GDT_Unknown, 0, 1);
EXPECT_EQ(CPLGetLastErrorType(), CE_Failure);
CPLErrorReset();
GDALCopyWords(&b, GDT_Unknown, 0, &b2, GDT_Byte, 0, 1);
EXPECT_EQ(CPLGetLastErrorType(), CE_Failure);
}

// Test GDALRasterBand::ReadRaster
TEST_F(test_gdal, ReadRaster)
{
Expand Down
31 changes: 23 additions & 8 deletions autotest/gcore/rasterio.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,6 @@

from osgeo import gdal


###############################################################################
@pytest.fixture(autouse=True, scope="module")
def module_disable_exceptions():
with gdaltest.disable_exceptions():
yield


###############################################################################
# Test writing a 1x1 buffer to a 10x6 raster and read it back

Expand Down Expand Up @@ -217,6 +209,7 @@ def test_rasterio_4():
# Test error cases of ReadRaster()


@gdaltest.disable_exceptions()
def test_rasterio_5():

ds = gdal.Open("data/byte.tif")
Expand Down Expand Up @@ -284,6 +277,7 @@ def test_rasterio_5():
# Test error cases of WriteRaster()


@gdaltest.disable_exceptions()
def test_rasterio_6():

ds = gdal.GetDriverByName("MEM").Create("", 2, 2)
Expand Down Expand Up @@ -757,6 +751,7 @@ def test_rasterio_overview_subpixel_resampling():
# Test error when getting a block


@gdaltest.disable_exceptions()
def test_rasterio_10():
ds = gdal.Open("data/byte_truncated.tif")

Expand Down Expand Up @@ -1431,6 +1426,7 @@ def test_rasterio_dataset_readarray_cint16():
assert got[1] == numpy.array([[3 + 4j]])


@gdaltest.disable_exceptions()
def test_rasterio_rasterband_write_on_readonly():

ds = gdal.Open("data/byte.tif")
Expand All @@ -1440,6 +1436,7 @@ def test_rasterio_rasterband_write_on_readonly():
assert err != 0


@gdaltest.disable_exceptions()
def test_rasterio_dataset_write_on_readonly():

ds = gdal.Open("data/byte.tif")
Expand Down Expand Up @@ -3012,6 +3009,7 @@ def test_rasterio_writeraster_from_memoryview():
# Test ReadRaster() in an existing buffer


@gdaltest.disable_exceptions()
def test_rasterio_readraster_in_existing_buffer():

ds = gdal.GetDriverByName("MEM").Create("", 2, 1)
Expand Down Expand Up @@ -3048,6 +3046,7 @@ def test_rasterio_readraster_in_existing_buffer():
# Test ReadBlock() in an existing buffer


@gdaltest.disable_exceptions()
def test_rasterio_readblock_in_existing_buffer():

ds = gdal.GetDriverByName("MEM").Create("", 2, 1)
Expand All @@ -3074,6 +3073,7 @@ def test_rasterio_readblock_in_existing_buffer():
# Test ReadRaster() in an existing buffer and alignment issues


@gdaltest.disable_exceptions()
@pytest.mark.parametrize(
"datatype",
[
Expand Down Expand Up @@ -3390,3 +3390,18 @@ def test_rasterio_overview_selection():
assert ds.GetRasterBand(1).ReadRaster(0, 0, 100, 100, 29, 29)[0] == 2
assert ds.GetRasterBand(1).ReadRaster(0, 0, 100, 100, 25, 25)[0] == 3
assert ds.GetRasterBand(1).ReadRaster(0, 0, 100, 100, 24, 24)[0] == 3


###############################################################################
# Check robustness to GDT_Unknown


def test_rasterio_gdt_unknown():

with gdal.GetDriverByName("MEM").Create("", 1, 1) as ds:
# Caught at the SWIG level
with pytest.raises(Exception, match="Illegal value for data type"):
ds.ReadRaster(buf_type=gdal.GDT_Unknown)
# Caught at the SWIG level
with pytest.raises(Exception, match="Illegal value for data type"):
ds.GetRasterBand(1).ReadRaster(buf_type=gdal.GDT_Unknown)
10 changes: 10 additions & 0 deletions autotest/gcore/vrtmisc.py
Original file line number Diff line number Diff line change
Expand Up @@ -1088,3 +1088,13 @@ def test_vrt_read_netcdf():
xml_data = xml.read()
# print(xml_data)
assert 'NETCDF:"alldatatypes.nc":ubyte_var' in xml_data


###############################################################################


def test_vrtmisc_add_band_gdt_unknown():
vrt_ds = gdal.GetDriverByName("VRT").Create("", 1, 1, 0)
options = ["subClass=VRTSourcedRasterBand", "blockXSize=32", "blockYSize=48"]
with pytest.raises(Exception, match="Illegal GDT_Unknown/GDT_TypeCount argument"):
vrt_ds.AddBand(gdal.GDT_Unknown, options)
17 changes: 12 additions & 5 deletions autotest/gdrivers/mem.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,7 @@ def test_mem_1():
#######################################################
# Setup dataset
drv = gdal.GetDriverByName("MEM")
gdaltest.mem_ds = drv.Create("mem_1.mem", 50, 3)
ds = gdaltest.mem_ds
ds = drv.Create("mem_1.mem", 50, 3)

assert ds.GetProjection() == "", "projection wrong"

Expand Down Expand Up @@ -777,8 +776,16 @@ def test_mem_alpha_ismaskband():


###############################################################################
# cleanup
# Check robustness to GDT_Unknown


def test_mem_cleanup():
gdaltest.mem_ds = None
def test_mem_gdt_unknown():

with pytest.raises(Exception, match="Illegal GDT_Unknown/GDT_TypeCount argument"):
gdal.GetDriverByName("MEM").Create("", 1, 1, 1, gdal.GDT_Unknown)

with gdal.GetDriverByName("MEM").Create("", 1, 1, 0, gdal.GDT_Unknown) as ds:
with pytest.raises(
Exception, match="Illegal GDT_Unknown/GDT_TypeCount argument"
):
ds.AddBand(gdal.GDT_Unknown)
12 changes: 12 additions & 0 deletions autotest/gdrivers/vrtwarp.py
Original file line number Diff line number Diff line change
Expand Up @@ -771,3 +771,15 @@ def test_vrtwarp_autocreatewarpedvrt_invalid_nodata():
ds.GetRasterBand(1).SetNoDataValue(-9999)
vrt_ds = gdal.AutoCreateWarpedVRT(ds)
assert vrt_ds.GetRasterBand(1).DataType == gdal.GDT_Byte


###############################################################################


def test_vrtwarp_add_band_gdt_unknown():

ds = gdal.GetDriverByName("MEM").Create("", 1, 1, 1, gdal.GDT_Byte)
ds.SetGeoTransform([0, 1, 0, 0, 0, -1])
vrt_ds = gdal.AutoCreateWarpedVRT(ds)
with pytest.raises(Exception, match="Illegal GDT_Unknown/GDT_TypeCount argument"):
vrt_ds.AddBand(gdal.GDT_Unknown)
8 changes: 7 additions & 1 deletion frmts/mem/memdataset.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -710,6 +710,12 @@ CPLErr MEMDataset::AddBand(GDALDataType eType, char **papszOptions)
{
const int nBandId = GetRasterCount() + 1;
const GSpacing nPixelSize = GDALGetDataTypeSizeBytes(eType);
if (nPixelSize == 0)
{
ReportError(CE_Failure, CPLE_IllegalArg,
"Illegal GDT_Unknown/GDT_TypeCount argument");
return CE_Failure;
}

/* -------------------------------------------------------------------- */
/* Do we need to allocate the memory ourselves? This is the */
Expand Down Expand Up @@ -1333,7 +1339,7 @@ MEMDataset *MEMDataset::Create(const char * /* pszFilename */, int nXSize,
/* First allocate band data, verifying that we can get enough */
/* memory. */
/* -------------------------------------------------------------------- */
const int nWordSize = GDALGetDataTypeSize(eType) / 8;
const int nWordSize = GDALGetDataTypeSizeBytes(eType);
if (nBandsIn > 0 && nWordSize > 0 &&
(nBandsIn > INT_MAX / nWordSize ||
(GIntBig)nXSize * nYSize > GINTBIG_MAX / (nWordSize * nBandsIn)))
Expand Down
7 changes: 7 additions & 0 deletions frmts/vrt/vrtdataset.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1601,6 +1601,13 @@ VRTDataset *VRTDataset::OpenXML(const char *pszXML, const char *pszVRTPath,
CPLErr VRTDataset::AddBand(GDALDataType eType, char **papszOptions)

{
if (eType == GDT_Unknown || eType == GDT_TypeCount)
{
ReportError(CE_Failure, CPLE_IllegalArg,
"Illegal GDT_Unknown/GDT_TypeCount argument");
return CE_Failure;
}

SetNeedsFlush();

/* ==================================================================== */
Expand Down
7 changes: 7 additions & 0 deletions frmts/vrt/vrtwarped.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2165,6 +2165,13 @@ CPLErr VRTWarpedDataset::IRasterIO(
CPLErr VRTWarpedDataset::AddBand(GDALDataType eType, char ** /* papszOptions */)

{
if (eType == GDT_Unknown || eType == GDT_TypeCount)
{
ReportError(CE_Failure, CPLE_IllegalArg,
"Illegal GDT_Unknown/GDT_TypeCount argument");
return CE_Failure;
}

SetBand(GetRasterCount() + 1,
new VRTWarpedRasterBand(this, GetRasterCount() + 1, eType));

Expand Down
15 changes: 11 additions & 4 deletions gcore/gdaldataset.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2724,7 +2724,8 @@ CPLErr GDALDataset::RasterIO(GDALRWFlag eRWFlag, int nXOff, int nYOff,

psExtraArg = &sExtraArg;
}
else if (psExtraArg->nVersion != RASTERIO_EXTRA_ARG_CURRENT_VERSION)
else if (CPL_UNLIKELY(psExtraArg->nVersion !=
RASTERIO_EXTRA_ARG_CURRENT_VERSION))
{
ReportError(CE_Failure, CPLE_AppDefined,
"Unhandled version of GDALRasterIOExtraArg");
Expand All @@ -2734,7 +2735,7 @@ CPLErr GDALDataset::RasterIO(GDALRWFlag eRWFlag, int nXOff, int nYOff,
GDALRasterIOExtraArgSetResampleAlg(psExtraArg, nXSize, nYSize, nBufXSize,
nBufYSize);

if (nullptr == pData)
if (CPL_UNLIKELY(nullptr == pData))
{
ReportError(CE_Failure, CPLE_AppDefined,
"The buffer into which the data should be read is null");
Expand All @@ -2745,7 +2746,7 @@ CPLErr GDALDataset::RasterIO(GDALRWFlag eRWFlag, int nXOff, int nYOff,
/* Do some validation of parameters. */
/* -------------------------------------------------------------------- */

if (eRWFlag != GF_Read && eRWFlag != GF_Write)
if (CPL_UNLIKELY(eRWFlag != GF_Read && eRWFlag != GF_Write))
{
ReportError(
CE_Failure, CPLE_IllegalArg,
Expand All @@ -2756,7 +2757,7 @@ CPLErr GDALDataset::RasterIO(GDALRWFlag eRWFlag, int nXOff, int nYOff,

if (eRWFlag == GF_Write)
{
if (eAccess != GA_Update)
if (CPL_UNLIKELY(eAccess != GA_Update))
{
ReportError(CE_Failure, CPLE_AppDefined,
"Write operation not permitted on dataset opened "
Expand All @@ -2771,6 +2772,12 @@ CPLErr GDALDataset::RasterIO(GDALRWFlag eRWFlag, int nXOff, int nYOff,
nBufYSize, nBandCount, panBandMap);
if (eErr != CE_None || bStopProcessing)
return eErr;
if (CPL_UNLIKELY(eBufType == GDT_Unknown || eBufType == GDT_TypeCount))
{
ReportError(CE_Failure, CPLE_AppDefined,
"Illegal GDT_Unknown/GDT_TypeCount argument");
return CE_Failure;
}

/* -------------------------------------------------------------------- */
/* If pixel and line spacing are defaulted assign reasonable */
Expand Down
20 changes: 14 additions & 6 deletions gcore/gdaldriver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -195,8 +195,8 @@ GDALDataset *GDALDriver::Create(const char *pszFilename, int nXSize, int nYSize,
/* Does this format support creation. */
/* -------------------------------------------------------------------- */
pfnCreate = GetCreateCallback();
if (pfnCreate == nullptr && pfnCreateEx == nullptr &&
pfnCreateVectorOnly == nullptr)
if (CPL_UNLIKELY(pfnCreate == nullptr && pfnCreateEx == nullptr &&
pfnCreateVectorOnly == nullptr))
{
CPLError(CE_Failure, CPLE_NotSupported,
"GDALDriver::Create() ... no create method implemented"
Expand All @@ -207,7 +207,7 @@ GDALDataset *GDALDriver::Create(const char *pszFilename, int nXSize, int nYSize,
/* -------------------------------------------------------------------- */
/* Do some rudimentary argument checking. */
/* -------------------------------------------------------------------- */
if (nBands < 0)
if (CPL_UNLIKELY(nBands < 0))
{
CPLError(CE_Failure, CPLE_AppDefined,
"Attempt to create dataset with %d bands is illegal,"
Expand All @@ -216,9 +216,9 @@ GDALDataset *GDALDriver::Create(const char *pszFilename, int nXSize, int nYSize,
return nullptr;
}

if (GetMetadataItem(GDAL_DCAP_RASTER) != nullptr &&
GetMetadataItem(GDAL_DCAP_VECTOR) == nullptr &&
(nXSize < 1 || nYSize < 1))
if (CPL_UNLIKELY(GetMetadataItem(GDAL_DCAP_RASTER) != nullptr &&
GetMetadataItem(GDAL_DCAP_VECTOR) == nullptr &&
(nXSize < 1 || nYSize < 1)))
{
CPLError(CE_Failure, CPLE_AppDefined,
"Attempt to create %dx%d dataset is illegal,"
Expand All @@ -227,6 +227,14 @@ GDALDataset *GDALDriver::Create(const char *pszFilename, int nXSize, int nYSize,
return nullptr;
}

if (CPL_UNLIKELY(nBands != 0 &&
(eType == GDT_Unknown || eType == GDT_TypeCount)))
{
CPLError(CE_Failure, CPLE_IllegalArg,
"Illegal GDT_Unknown/GDT_TypeCount argument");
return nullptr;
}

/* -------------------------------------------------------------------- */
/* Make sure we cleanup if there is an existing dataset of this */
/* name. But even if that seems to fail we will continue since */
Expand Down
12 changes: 10 additions & 2 deletions gcore/gdalmultidim.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10043,7 +10043,8 @@ GDALExtendedDataType::operator=(GDALExtendedDataType &&other)
*
* This is the same as the C function GDALExtendedDataTypeCreate()
*
* @param eType Numeric data type.
* @param eType Numeric data type. Must be different from GDT_Unknown and
* GDT_TypeCount
*/
GDALExtendedDataType GDALExtendedDataType::Create(GDALDataType eType)
{
Expand Down Expand Up @@ -10471,12 +10472,19 @@ void GDALDimension::ParentDeleted()
*
* The returned handle should be freed with GDALExtendedDataTypeRelease().
*
* @param eType Numeric data type.
* @param eType Numeric data type. Must be different from GDT_Unknown and
* GDT_TypeCount
*
* @return a new GDALExtendedDataTypeH handle, or nullptr.
*/
GDALExtendedDataTypeH GDALExtendedDataTypeCreate(GDALDataType eType)
{
if (CPL_UNLIKELY(eType == GDT_Unknown || eType == GDT_TypeCount))
{
CPLError(CE_Failure, CPLE_IllegalArg,
"Illegal GDT_Unknown/GDT_TypeCount argument");
return nullptr;
}
return new GDALExtendedDataTypeHS(
new GDALExtendedDataType(GDALExtendedDataType::Create(eType)));
}
Expand Down
Loading

0 comments on commit a8642db

Please sign in to comment.