Skip to content

Commit 2b1c4d1

Browse files
rouaultrouault
authored
Merge pull request OSGeo#13113 from rouault/fix_13112
GDALAlgorithm: re-arrange argument validation so that 'gdal raster create --bbox=' doesn't crash
2 parents aff7e2d + c0f4742 commit 2b1c4d1

File tree

6 files changed

+112
-63
lines changed

6 files changed

+112
-63
lines changed

‎autotest/cpp/test_gdal_algorithm.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2953,8 +2953,8 @@ TEST_F(test_gdal_algorithm, min_max_count_equal)
29532953

29542954
{
29552955
MyAlgorithm alg;
2956-
alg.GetArg("arg")->Set(std::vector<std::string>{"foo"});
29572956
CPLErrorStateBackuper oBackuper(CPLQuietErrorHandler);
2957+
EXPECT_FALSE(alg.GetArg("arg")->Set(std::vector<std::string>{"foo"}));
29582958
EXPECT_FALSE(alg.ValidateArguments());
29592959
EXPECT_STREQ(CPLGetLastErrorMsg(),
29602960
"test: 1 value has been specified for argument 'arg', "

‎autotest/gcore/algorithm.py‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -458,7 +458,10 @@ def test_algorithm_arg_set_dataset_list(tmp_path):
458458
alg["input"] = [tmp_path]
459459
alg["input"] = ["foo"]
460460
alg["input"] = [gdal.GetDriverByName("MEM").Create("", 1, 1)]
461-
alg["input"] = []
461+
with pytest.raises(
462+
RuntimeError, match="Only 0 value has been specified for argument 'input'"
463+
):
464+
alg["input"] = []
462465
alg["input"] = ["foo", "bar"]
463466

464467
with pytest.raises(TypeError):

‎autotest/utilities/test_gdalalg_raster_create.py‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -454,7 +454,7 @@ def test_gdalalg_raster_create_driver_not_available():
454454
drv = gdal.GetDriverByName("MEM")
455455
drv.Deregister()
456456
try:
457-
with pytest.raises(Exception, match="Cannot find driver MEM"):
457+
with pytest.raises(Exception, match="Driver 'MEM' does not exist"):
458458
alg.Run()
459459
finally:
460460
drv.Register()
@@ -498,3 +498,21 @@ def test_gdalalg_raster_set_bbox_failed(tmp_vsimem):
498498

499499
with pytest.raises(Exception, match="Setting extent failed"):
500500
alg.Run()
501+
502+
503+
def test_gdalalg_raster_create_empty_bbox():
504+
505+
alg = get_alg()
506+
alg["output-format"] = "MEM"
507+
with pytest.raises(
508+
Exception,
509+
match="0 value has been specified for argument 'bbox', whereas exactly 4 were expected",
510+
):
511+
alg["bbox"] = []
512+
alg["size"] = [1, 1]
513+
514+
with pytest.raises(
515+
Exception,
516+
match="0 value has been specified for argument 'bbox', whereas exactly 4 were expected",
517+
):
518+
alg.Run()

‎autotest/utilities/test_gdalalg_vector_layer_algebra.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -387,7 +387,7 @@ def test_gdal_vector_layer_algebra_output_driver_not_existing(tmp_vsimem):
387387
drv = gdal.GetDriverByName("ESRI Shapefile")
388388
drv.Deregister()
389389
try:
390-
with pytest.raises(Exception, match="Driver ESRI Shapefile does not exist"):
390+
with pytest.raises(Exception, match="Driver 'ESRI Shapefile' does not exist"):
391391
alg.Run()
392392
finally:
393393
drv.Register()

‎gcore/gdalalgorithm.cpp‎

Lines changed: 84 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -1028,15 +1028,94 @@ bool GDALAlgorithmArg::RunValidationActions()
10281028
ret = ValidateRealRange(v) && ret;
10291029
}
10301030

1031-
for (const auto &f : m_validationActions)
1031+
if (GDALAlgorithmArgTypeIsList(GetType()))
10321032
{
1033-
if (!f())
1033+
int valueCount = 0;
1034+
if (GetType() == GAAT_STRING_LIST)
1035+
{
1036+
valueCount =
1037+
static_cast<int>(Get<std::vector<std::string>>().size());
1038+
}
1039+
else if (GetType() == GAAT_INTEGER_LIST)
1040+
{
1041+
valueCount = static_cast<int>(Get<std::vector<int>>().size());
1042+
}
1043+
else if (GetType() == GAAT_REAL_LIST)
1044+
{
1045+
valueCount = static_cast<int>(Get<std::vector<double>>().size());
1046+
}
1047+
else if (GetType() == GAAT_DATASET_LIST)
1048+
{
1049+
valueCount = static_cast<int>(
1050+
Get<std::vector<GDALArgDatasetValue>>().size());
1051+
}
1052+
1053+
if (valueCount != GetMinCount() && GetMinCount() == GetMaxCount())
1054+
{
1055+
ReportError(CE_Failure, CPLE_AppDefined,
1056+
"%d value%s been specified for argument '%s', "
1057+
"whereas exactly %d %s expected.",
1058+
valueCount, valueCount > 1 ? "s have" : " has",
1059+
GetName().c_str(), GetMinCount(),
1060+
GetMinCount() > 1 ? "were" : "was");
10341061
ret = false;
1062+
}
1063+
else if (valueCount < GetMinCount())
1064+
{
1065+
ReportError(CE_Failure, CPLE_AppDefined,
1066+
"Only %d value%s been specified for argument '%s', "
1067+
"whereas at least %d %s expected.",
1068+
valueCount, valueCount > 1 ? "s have" : " has",
1069+
GetName().c_str(), GetMinCount(),
1070+
GetMinCount() > 1 ? "were" : "was");
1071+
ret = false;
1072+
}
1073+
else if (valueCount > GetMaxCount())
1074+
{
1075+
ReportError(CE_Failure, CPLE_AppDefined,
1076+
"%d value%s been specified for argument '%s', "
1077+
"whereas at most %d %s expected.",
1078+
valueCount, valueCount > 1 ? "s have" : " has",
1079+
GetName().c_str(), GetMaxCount(),
1080+
GetMaxCount() > 1 ? "were" : "was");
1081+
ret = false;
1082+
}
1083+
}
1084+
1085+
if (ret)
1086+
{
1087+
for (const auto &f : m_validationActions)
1088+
{
1089+
if (!f())
1090+
ret = false;
1091+
}
10351092
}
10361093

10371094
return ret;
10381095
}
10391096

1097+
/************************************************************************/
1098+
/* GDALAlgorithmArg::ReportError() */
1099+
/************************************************************************/
1100+
1101+
void GDALAlgorithmArg::ReportError(CPLErr eErrClass, CPLErrorNum err_no,
1102+
const char *fmt, ...) const
1103+
{
1104+
va_list args;
1105+
va_start(args, fmt);
1106+
if (m_owner)
1107+
{
1108+
m_owner->ReportError(eErrClass, err_no, "%s",
1109+
CPLString().vPrintf(fmt, args).c_str());
1110+
}
1111+
else
1112+
{
1113+
CPLError(eErrClass, err_no, "%s",
1114+
CPLString().vPrintf(fmt, args).c_str());
1115+
}
1116+
va_end(args);
1117+
}
1118+
10401119
/************************************************************************/
10411120
/* GDALAlgorithmArg::GetEscapedString() */
10421121
/************************************************************************/
@@ -2849,63 +2928,6 @@ bool GDALAlgorithm::ValidateArguments()
28492928
if (!ProcessDatasetArg(arg.get(), this))
28502929
ret = false;
28512930
}
2852-
else if (arg->IsExplicitlySet() &&
2853-
GDALAlgorithmArgTypeIsList(arg->GetType()))
2854-
{
2855-
int valueCount = 0;
2856-
if (arg->GetType() == GAAT_STRING_LIST)
2857-
{
2858-
valueCount = static_cast<int>(
2859-
arg->Get<std::vector<std::string>>().size());
2860-
}
2861-
else if (arg->GetType() == GAAT_INTEGER_LIST)
2862-
{
2863-
valueCount =
2864-
static_cast<int>(arg->Get<std::vector<int>>().size());
2865-
}
2866-
else if (arg->GetType() == GAAT_REAL_LIST)
2867-
{
2868-
valueCount =
2869-
static_cast<int>(arg->Get<std::vector<double>>().size());
2870-
}
2871-
else if (arg->GetType() == GAAT_DATASET_LIST)
2872-
{
2873-
valueCount = static_cast<int>(
2874-
arg->Get<std::vector<GDALArgDatasetValue>>().size());
2875-
}
2876-
2877-
if (valueCount != arg->GetMinCount() &&
2878-
arg->GetMinCount() == arg->GetMaxCount())
2879-
{
2880-
ReportError(CE_Failure, CPLE_AppDefined,
2881-
"%d value%s been specified for argument '%s', "
2882-
"whereas exactly %d %s expected.",
2883-
valueCount, valueCount > 1 ? "s have" : " has",
2884-
arg->GetName().c_str(), arg->GetMinCount(),
2885-
arg->GetMinCount() > 1 ? "were" : "was");
2886-
ret = false;
2887-
}
2888-
else if (valueCount < arg->GetMinCount())
2889-
{
2890-
ReportError(CE_Failure, CPLE_AppDefined,
2891-
"Only %d value%s been specified for argument '%s', "
2892-
"whereas at least %d %s expected.",
2893-
valueCount, valueCount > 1 ? "s have" : " has",
2894-
arg->GetName().c_str(), arg->GetMinCount(),
2895-
arg->GetMinCount() > 1 ? "were" : "was");
2896-
ret = false;
2897-
}
2898-
else if (valueCount > arg->GetMaxCount())
2899-
{
2900-
ReportError(CE_Failure, CPLE_AppDefined,
2901-
"%d value%s been specified for argument '%s', "
2902-
"whereas at most %d %s expected.",
2903-
valueCount, valueCount > 1 ? "s have" : " has",
2904-
arg->GetName().c_str(), arg->GetMaxCount(),
2905-
arg->GetMaxCount() > 1 ? "were" : "was");
2906-
ret = false;
2907-
}
2908-
}
29092931

29102932
if (arg->IsExplicitlySet() && arg->GetType() == GAAT_DATASET_LIST &&
29112933
arg->AutoOpenDataset())
@@ -2978,6 +3000,9 @@ bool GDALAlgorithm::ValidateArguments()
29783000
}
29793001
}
29803002
}
3003+
3004+
if (arg->IsExplicitlySet() && !arg->RunValidationActions())
3005+
ret = false;
29813006
}
29823007

29833008
for (const auto &f : m_validationActions)

‎gcore/gdalalgorithm.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2003,6 +2003,9 @@ class CPL_DLL GDALAlgorithmArg /* non-final */
20032003
bool ValidateIntRange(int val) const;
20042004
bool ValidateRealRange(double val) const;
20052005

2006+
void ReportError(CPLErr eErrClass, CPLErrorNum err_no, const char *fmt,
2007+
...) const CPL_PRINT_FUNC_FORMAT(4, 5);
2008+
20062009
CPL_DISALLOW_COPY_ASSIGN(GDALAlgorithmArg)
20072010
};
20082011

0 commit comments

Comments
 (0)