Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Raster API: error out on GDT_Unknown/GDT_TypeCount #11258

Merged
merged 1 commit into from
Nov 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading