Skip to content

Commit 0fe882e

Browse files
authored
[cli] Fix some error messages (#13896)
Fix #13662
1 parent 0ae0176 commit 0fe882e

File tree

5 files changed

+83
-19
lines changed

5 files changed

+83
-19
lines changed

apps/gdalalg_materialize.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -244,7 +244,7 @@ bool GDALMaterializeVectorAlgorithm::RunStep(GDALPipelineStepRunContext &ctxt)
244244
}
245245

246246
CPLStringList aosOptions;
247-
aosOptions.AddString("--invoked-from-gdal-vector-convert");
247+
aosOptions.AddString("--invoked-from-gdal-algorithm");
248248
if (!m_overwrite)
249249
{
250250
aosOptions.AddString("--no-overwrite");

apps/gdalalg_vector_write.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
****************************************************************************/
1212

1313
#include "gdalalg_vector_write.h"
14-
1514
#include "cpl_string.h"
1615
#include "gdal_utils.h"
1716
#include "gdal_priv.h"
@@ -52,7 +51,7 @@ bool GDALVectorWriteAlgorithm::RunStep(GDALPipelineStepRunContext &ctxt)
5251
}
5352

5453
CPLStringList aosOptions;
55-
aosOptions.AddString("--invoked-from-gdal-vector-convert");
54+
aosOptions.AddString("--invoked-from-gdal-algorithm");
5655
if (!m_overwrite)
5756
{
5857
aosOptions.AddString("--no-overwrite");
@@ -114,8 +113,11 @@ bool GDALVectorWriteAlgorithm::RunStep(GDALPipelineStepRunContext &ctxt)
114113
&hSrcDS, psOptions, nullptr));
115114
GDALVectorTranslateOptionsFree(psOptions);
116115
}
116+
117117
if (!poRetDS)
118+
{
118119
return false;
120+
}
119121

120122
if (!hOutDS)
121123
{

apps/ogr2ogr_lib.cpp

Lines changed: 27 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -422,8 +422,8 @@ struct GDALVectorTranslateOptions
422422
/*! set to true to prevent overwriting existing dataset */
423423
bool bNoOverwrite = false;
424424

425-
/*! set to true to prevent if called from "gdal vector convert" */
426-
bool bInvokedFromGdalVectorConvert = false;
425+
/*! set to true to customize error messages when called from "new" (GDAL 3.11) CLI or Algorithm API */
426+
bool bInvokedFromGdalAlgorithm = false;
427427
};
428428

429429
struct TargetLayerInfo
@@ -2636,10 +2636,21 @@ GDALDatasetH GDALVectorTranslate(const char *pszDest, GDALDatasetH hDstDS,
26362636
}
26372637
if (bError)
26382638
{
2639-
CPLError(CE_Failure, CPLE_IllegalArg,
2640-
"-nln name must be specified combined with "
2641-
"a single source layer name,\nor a -sql statement, and "
2642-
"name must be different from an existing layer.");
2639+
if (psOptions->bInvokedFromGdalAlgorithm)
2640+
{
2641+
CPLError(CE_Failure, CPLE_IllegalArg,
2642+
"--output-layer name must be specified combined with "
2643+
"a single source layer name and it "
2644+
"must be different from an existing layer.");
2645+
}
2646+
else
2647+
{
2648+
CPLError(
2649+
CE_Failure, CPLE_IllegalArg,
2650+
"-nln name must be specified combined with "
2651+
"a single source layer name,\nor a -sql statement, and "
2652+
"name must be different from an existing layer.");
2653+
}
26432654
return nullptr;
26442655
}
26452656
}
@@ -2753,7 +2764,7 @@ GDALDatasetH GDALVectorTranslate(const char *pszDest, GDALDatasetH hDstDS,
27532764
if (aoDrivers.empty())
27542765
{
27552766
if (CPLGetExtensionSafe(pszDest).empty() &&
2756-
!psOptions->bInvokedFromGdalVectorConvert)
2767+
!psOptions->bInvokedFromGdalAlgorithm)
27572768
{
27582769
psOptions->osFormat = "ESRI Shapefile";
27592770
}
@@ -2902,7 +2913,7 @@ GDALDatasetH GDALVectorTranslate(const char *pszDest, GDALDatasetH hDstDS,
29022913
}
29032914
bNewDataSource = true;
29042915

2905-
if (psOptions->bInvokedFromGdalVectorConvert && !bSingleLayer &&
2916+
if (psOptions->bInvokedFromGdalAlgorithm && !bSingleLayer &&
29062917
!bOutputDirectory &&
29072918
(!poODS->TestCapability(ODsCCreateLayer) ||
29082919
!poDriver->GetMetadataItem(GDAL_DCAP_MULTIPLE_VECTOR_LAYERS)))
@@ -3198,7 +3209,7 @@ GDALDatasetH GDALVectorTranslate(const char *pszDest, GDALDatasetH hDstDS,
31983209
}
31993210
else if (!poResultSet->TestCapability(OLCFastFeatureCount))
32003211
{
3201-
if (!psOptions->bInvokedFromGdalVectorConvert)
3212+
if (!psOptions->bInvokedFromGdalAlgorithm)
32023213
{
32033214
CPLError(
32043215
CE_Warning, CPLE_AppDefined,
@@ -3488,7 +3499,7 @@ GDALDatasetH GDALVectorTranslate(const char *pszDest, GDALDatasetH hDstDS,
34883499
nullptr, psOptions.get())) &&
34893500
!psOptions->bSkipFailures)
34903501
{
3491-
if (psOptions->bInvokedFromGdalVectorConvert)
3502+
if (psOptions->bInvokedFromGdalAlgorithm)
34923503
{
34933504
CPLError(
34943505
CE_Failure, CPLE_AppDefined,
@@ -3664,7 +3675,7 @@ GDALDatasetH GDALVectorTranslate(const char *pszDest, GDALDatasetH hDstDS,
36643675
{
36653676
if (!poLayer->TestCapability(OLCFastFeatureCount))
36663677
{
3667-
if (!psOptions->bInvokedFromGdalVectorConvert)
3678+
if (!psOptions->bInvokedFromGdalAlgorithm)
36683679
{
36693680
CPLError(
36703681
CE_Warning, CPLE_NotSupported,
@@ -3773,7 +3784,7 @@ GDALDatasetH GDALVectorTranslate(const char *pszDest, GDALDatasetH hDstDS,
37733784
pProgressArg.get(), psOptions.get())) &&
37743785
!psOptions->bSkipFailures)
37753786
{
3776-
if (psOptions->bInvokedFromGdalVectorConvert)
3787+
if (psOptions->bInvokedFromGdalAlgorithm)
37773788
{
37783789
CPLError(CE_Failure, CPLE_AppDefined,
37793790
"Failed to write layer '%s'. Use --skip-errors to "
@@ -5228,7 +5239,7 @@ SetupTargetLayer::Setup(OGRLayer *poSrcLayer, const char *pszNewLayerName,
52285239
/* -------------------------------------------------------------------- */
52295240
else if (!bAppend && !m_bNewDataSource)
52305241
{
5231-
if (psOptions->bInvokedFromGdalVectorConvert)
5242+
if (psOptions->bInvokedFromGdalAlgorithm)
52325243
{
52335244
CPLError(CE_Failure, CPLE_AppDefined,
52345245
"Layer %s already exists, and --append not specified. "
@@ -8460,9 +8471,9 @@ static std::unique_ptr<GDALArgumentParser> GDALVectorTranslateOptionsGetParser(
84608471
.store_into(psOptions->bNoOverwrite)
84618472
.hidden();
84628473

8463-
// Undocumented option used by gdal vector convert
8464-
argParser->add_argument("--invoked-from-gdal-vector-convert")
8465-
.store_into(psOptions->bInvokedFromGdalVectorConvert)
8474+
// Undocumented option used by gdal vector * algorithms
8475+
argParser->add_argument("--invoked-from-gdal-algorithm")
8476+
.store_into(psOptions->bInvokedFromGdalAlgorithm)
84668477
.hidden();
84678478

84688479
if (psOptionsForBinary)

autotest/utilities/test_gdalalg_vector_convert.py

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -438,3 +438,33 @@ def create_src_file():
438438
assert f["other"] == "foo"
439439
assert f.GetGeometryRef().ExportToWkt() == "POINT (10 10)"
440440
ds = None
441+
442+
443+
@pytest.mark.require_driver("GeoJSON")
444+
def test_error_message_leak(tmp_vsimem):
445+
"""Test issue GH #13662"""
446+
447+
json_path = tmp_vsimem / "test_error_message_leak_in.json"
448+
out_path = tmp_vsimem / "test_error_message_leak_out.shp"
449+
450+
src_ds = ogr.GetDriverByName("GeoJSON").CreateDataSource(json_path)
451+
lyr = src_ds.CreateLayer("test")
452+
f = ogr.Feature(lyr.GetLayerDefn())
453+
f.SetGeometry(ogr.CreateGeometryFromWkt("POINT(1 2)"))
454+
lyr.CreateFeature(f)
455+
f = ogr.Feature(lyr.GetLayerDefn())
456+
f.SetGeometry(ogr.CreateGeometryFromWkt("LINESTRING(1 2, 3 4)"))
457+
lyr.CreateFeature(f)
458+
src_ds = None
459+
460+
alg = get_convert_alg()
461+
with pytest.raises(
462+
Exception,
463+
match="Failed to write layer 'test'. Use --skip-errors to ignore errors",
464+
):
465+
alg.ParseRunAndFinalize(
466+
[
467+
json_path,
468+
out_path,
469+
],
470+
)

autotest/utilities/test_gdalalg_vector_edit.py

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,3 +199,24 @@ def test_gdalalg_vector_edit_unset_fid():
199199
assert lyr.GetFIDColumn() == ""
200200
f = lyr.GetNextFeature()
201201
assert f.GetFID() == 0
202+
203+
204+
@pytest.mark.require_driver("GPKG")
205+
def test_error_message_leak():
206+
"""Test issue GH #13662"""
207+
208+
alg = get_edit_alg()
209+
in_filename = "../gdrivers/data/gpkg/byte.gpkg"
210+
out_filename = in_filename
211+
with pytest.raises(
212+
Exception,
213+
match="--output-layer name must be specified combined with a single source layer name and it must be different from an existing layer.",
214+
):
215+
alg.ParseRunAndFinalize(
216+
[
217+
in_filename,
218+
"--crs=EPSG:4326",
219+
"--overwrite-layer",
220+
out_filename,
221+
],
222+
)

0 commit comments

Comments
 (0)