Merge pull request #4302 from rouault/fix_pyproj_705

Fail consistently on network error and grid transformations
OSGeo · Nov 1, 2024 · 4d08385 · 4d08385
2 parents ddfb7e7 + 17b2c2b
commit 4d08385
Show file tree

Hide file tree

Showing 6 changed files with 261 additions and 26 deletions.
diff --git a/src/iso19111/c_api.cpp b/src/iso19111/c_api.cpp
@@ -210,11 +210,13 @@ PJ *pj_obj_create(PJ_CONTEXT *ctx, const BaseObjectNNPtr &objIn) {
                     PROJStringFormatter::Convention::PROJ_5,
                     std::move(dbContext));
                 auto projString = coordop->exportToPROJString(formatter.get());
-                if (proj_context_is_network_enabled(ctx)) {
+                const bool defer_grid_opening_backup = ctx->defer_grid_opening;
+                if (!defer_grid_opening_backup &&
+                    proj_context_is_network_enabled(ctx)) {
                     ctx->defer_grid_opening = true;
                 }
                 auto pj = pj_create_internal(ctx, projString.c_str());
-                ctx->defer_grid_opening = false;
+                ctx->defer_grid_opening = defer_grid_opening_backup;
                 if (pj) {
                     pj->iso_obj = objIn;
                     pj->iso_obj_is_coordinate_operation = true;

diff --git a/src/transformations/gridshift.cpp b/src/transformations/gridshift.cpp
@@ -80,6 +80,7 @@ struct GridInfo {
 struct gridshiftData {
     ListOfGenericGrids m_grids{};
     bool m_defer_grid_opening = false;
+    int m_error_code_in_defer_grid_opening = 0;
     bool m_bHasHorizontalOffset = false;
     bool m_bHasGeographic3DOffset = false;
     bool m_bHasEllipsoidalHeightOffset = false;
@@ -724,10 +725,14 @@ PJ_XYZ gridshiftData::grid_apply_internal(
 // ---------------------------------------------------------------------------
 
 bool gridshiftData::loadGridsIfNeeded(PJ *P) {
-    if (m_defer_grid_opening) {
+    if (m_error_code_in_defer_grid_opening) {
+        proj_errno_set(P, m_error_code_in_defer_grid_opening);
+        return false;
+    } else if (m_defer_grid_opening) {
         m_defer_grid_opening = false;
         m_grids = pj_generic_grid_init(P, "grids");
-        if (proj_errno(P)) {
+        m_error_code_in_defer_grid_opening = proj_errno(P);
+        if (m_error_code_in_defer_grid_opening) {
             return false;
         }
         bool isProjectedCoord;

diff --git a/src/transformations/hgridshift.cpp b/src/transformations/hgridshift.cpp
@@ -22,6 +22,7 @@ struct hgridshiftData {
     double t_epoch = 0;
     ListOfHGrids grids{};
     bool defer_grid_opening = false;
+    int error_code_in_defer_grid_opening = 0;
 };
 } // anonymous namespace
 
@@ -33,9 +34,11 @@ static PJ_XYZ pj_hgridshift_forward_3d(PJ_LPZ lpz, PJ *P) {
     if (Q->defer_grid_opening) {
         Q->defer_grid_opening = false;
         Q->grids = pj_hgrid_init(P, "grids");
-        if (proj_errno(P)) {
-            return proj_coord_error().xyz;
-        }
+        Q->error_code_in_defer_grid_opening = proj_errno(P);
+    }
+    if (Q->error_code_in_defer_grid_opening) {
+        proj_errno_set(P, Q->error_code_in_defer_grid_opening);
+        return proj_coord_error().xyz;
     }
 
     if (!Q->grids.empty()) {
@@ -55,9 +58,11 @@ static PJ_LPZ pj_hgridshift_reverse_3d(PJ_XYZ xyz, PJ *P) {
     if (Q->defer_grid_opening) {
         Q->defer_grid_opening = false;
         Q->grids = pj_hgrid_init(P, "grids");
-        if (proj_errno(P)) {
-            return proj_coord_error().lpz;
-        }
+        Q->error_code_in_defer_grid_opening = proj_errno(P);
+    }
+    if (Q->error_code_in_defer_grid_opening) {
+        proj_errno_set(P, Q->error_code_in_defer_grid_opening);
+        return proj_coord_error().lpz;
     }
 
     if (!Q->grids.empty()) {

diff --git a/src/transformations/vgridshift.cpp b/src/transformations/vgridshift.cpp
@@ -23,6 +23,7 @@ struct vgridshiftData {
     double forward_multiplier = 0;
     ListOfVGrids grids{};
     bool defer_grid_opening = false;
+    int error_code_in_defer_grid_opening = 0;
 };
 } // anonymous namespace
 
@@ -60,9 +61,11 @@ static PJ_XYZ pj_vgridshift_forward_3d(PJ_LPZ lpz, PJ *P) {
         Q->defer_grid_opening = false;
         Q->grids = pj_vgrid_init(P, "grids");
         deal_with_vertcon_gtx_hack(P);
-        if (proj_errno(P)) {
-            return proj_coord_error().xyz;
-        }
+        Q->error_code_in_defer_grid_opening = proj_errno(P);
+    }
+    if (Q->error_code_in_defer_grid_opening) {
+        proj_errno_set(P, Q->error_code_in_defer_grid_opening);
+        return proj_coord_error().xyz;
     }
 
     if (!Q->grids.empty()) {
@@ -84,9 +87,11 @@ static PJ_LPZ pj_vgridshift_reverse_3d(PJ_XYZ xyz, PJ *P) {
         Q->defer_grid_opening = false;
         Q->grids = pj_vgrid_init(P, "grids");
         deal_with_vertcon_gtx_hack(P);
-        if (proj_errno(P)) {
-            return proj_coord_error().lpz;
-        }
+        Q->error_code_in_defer_grid_opening = proj_errno(P);
+    }
+    if (Q->error_code_in_defer_grid_opening) {
+        proj_errno_set(P, Q->error_code_in_defer_grid_opening);
+        return proj_coord_error().lpz;
     }
 
     if (!Q->grids.empty()) {

diff --git a/src/transformations/xyzgridshift.cpp b/src/transformations/xyzgridshift.cpp
@@ -45,6 +45,7 @@ struct xyzgridshiftData {
     bool grid_ref_is_input = true;
     ListOfGenericGrids grids{};
     bool defer_grid_opening = false;
+    int error_code_in_defer_grid_opening = 0;
     double multiplier = 1.0;
 };
 } // anonymous namespace
@@ -56,9 +57,11 @@ static bool get_grid_values(PJ *P, xyzgridshiftData *Q, const PJ_LP &lp,
     if (Q->defer_grid_opening) {
         Q->defer_grid_opening = false;
         Q->grids = pj_generic_grid_init(P, "grids");
-        if (proj_errno(P)) {
-            return false;
-        }
+        Q->error_code_in_defer_grid_opening = proj_errno(P);
+    }
+    if (Q->error_code_in_defer_grid_opening) {
+        proj_errno_set(P, Q->error_code_in_defer_grid_opening);
+        return false;
     }
 
     GenericShiftGridSet *gridset = nullptr;

diff --git a/test/unit/test_network.cpp b/test/unit/test_network.cpp
@@ -1234,26 +1234,241 @@ TEST(networking, network_endpoint_env_variable) {
 
 #ifdef CURL_ENABLED
 
-TEST(networking, network_endpoint_api) {
+TEST(networking, network_endpoint_api_and_not_reachable_gridshift) {
     auto ctx = proj_context_create();
     proj_grid_cache_set_enable(ctx, false);
     proj_context_set_enable_network(ctx, true);
     proj_context_set_url_endpoint(ctx, "http://0.0.0.0");
 
-    // NAD83 to NAD83(HARN) in West-Virginia. Using wvhpgn.tif
+    // NAD83 to NAD83(HARN) using
+    // us_noaa_nadcon5_nad83_1986_nad83_harn_conus.tif
     auto P = proj_create_crs_to_crs(ctx, "EPSG:4269", "EPSG:4152", nullptr);
     ASSERT_NE(P, nullptr);
 
     PJ_COORD c;
-    c.xyz.x = 40;  // lat
-    c.xyz.y = -80; // long
-    c.xyz.z = 0;
-    c = proj_trans(P, PJ_FWD, c);
+    c.xyzt.x = 40;  // lat
+    c.xyzt.y = -80; // long
+    c.xyzt.z = 0;
+    c.xyzt.t = HUGE_VAL;
+
+    {
+        PJ_COORD c2 = proj_trans(P, PJ_FWD, c);
+        EXPECT_EQ(c2.xyz.x, HUGE_VAL);
+        EXPECT_EQ(proj_errno(P), PROJ_ERR_OTHER_NETWORK_ERROR);
+        PJ *last_op = proj_trans_get_last_used_operation(P);
+        EXPECT_STREQ(proj_pj_info(last_op).description,
+                     "NAD83 to NAD83(HARN) (47)");
+        proj_destroy(last_op);
+    }
+
+    proj_errno_reset(P);
+
+    // Check again. Cf https://github.com/pyproj4/pyproj/issues/705
+    {
+        PJ_COORD c2 = proj_trans(P, PJ_FWD, c);
+        EXPECT_EQ(c2.xyz.x, HUGE_VAL);
+        EXPECT_EQ(proj_errno(P), PROJ_ERR_OTHER_NETWORK_ERROR);
+        PJ *last_op = proj_trans_get_last_used_operation(P);
+        EXPECT_STREQ(proj_pj_info(last_op).description,
+                     "NAD83 to NAD83(HARN) (47)");
+        proj_destroy(last_op);
+    }
+
+    proj_errno_reset(P);
+
+    // Check also reverse direction
+    {
+        PJ_COORD c2 = proj_trans(P, PJ_INV, c);
+        EXPECT_EQ(c2.xyz.x, HUGE_VAL);
+        EXPECT_EQ(proj_errno(P), PROJ_ERR_OTHER_NETWORK_ERROR);
+        PJ *last_op = proj_trans_get_last_used_operation(P);
+        EXPECT_STREQ(proj_pj_info(last_op).description,
+                     "NAD83 to NAD83(HARN) (47)");
+        proj_destroy(last_op);
+    }
 
     proj_destroy(P);
     proj_context_destroy(ctx);
+}
 
-    EXPECT_EQ(c.xyz.x, HUGE_VAL);
+#endif
+
+// ---------------------------------------------------------------------------
+
+#ifdef CURL_ENABLED
+
+TEST(networking, network_endpoint_api_and_not_reachable_xyzgridshift) {
+    auto ctx = proj_context_create();
+    proj_grid_cache_set_enable(ctx, false);
+    proj_context_set_enable_network(ctx, true);
+    proj_context_set_url_endpoint(ctx, "http://0.0.0.0");
+
+    // NTF to RGF93 using fr_ign_gr3df97a.tif
+    auto P = proj_create_crs_to_crs(ctx, "EPSG:4275", "EPSG:4171", nullptr);
+    ASSERT_NE(P, nullptr);
+
+    PJ_COORD c;
+    c.xyzt.x = 49; // lat
+    c.xyzt.y = 2;  // long
+    c.xyzt.z = 0;
+    c.xyzt.t = HUGE_VAL;
+
+    {
+        PJ_COORD c2 = proj_trans(P, PJ_FWD, c);
+        EXPECT_EQ(c2.xyz.x, HUGE_VAL);
+        EXPECT_EQ(proj_errno(P), PROJ_ERR_OTHER_NETWORK_ERROR);
+        PJ *last_op = proj_trans_get_last_used_operation(P);
+        EXPECT_STREQ(proj_pj_info(last_op).description, "NTF to RGF93 v1 (1)");
+        proj_destroy(last_op);
+    }
+
+    proj_errno_reset(P);
+
+    // Check again. Cf https://github.com/pyproj4/pyproj/issues/705
+    {
+        PJ_COORD c2 = proj_trans(P, PJ_FWD, c);
+        EXPECT_EQ(c2.xyz.x, HUGE_VAL);
+        EXPECT_EQ(proj_errno(P), PROJ_ERR_OTHER_NETWORK_ERROR);
+        PJ *last_op = proj_trans_get_last_used_operation(P);
+        EXPECT_STREQ(proj_pj_info(last_op).description, "NTF to RGF93 v1 (1)");
+        proj_destroy(last_op);
+    }
+
+    proj_errno_reset(P);
+
+    // Check also reverse direction
+    {
+        PJ_COORD c2 = proj_trans(P, PJ_INV, c);
+        EXPECT_EQ(c2.xyz.x, HUGE_VAL);
+        EXPECT_EQ(proj_errno(P), PROJ_ERR_OTHER_NETWORK_ERROR);
+        PJ *last_op = proj_trans_get_last_used_operation(P);
+        EXPECT_STREQ(proj_pj_info(last_op).description, "NTF to RGF93 v1 (1)");
+        proj_destroy(last_op);
+    }
+
+    proj_destroy(P);
+    proj_context_destroy(ctx);
+}
+
+#endif
+
+// ---------------------------------------------------------------------------
+
+#ifdef CURL_ENABLED
+
+TEST(networking, network_endpoint_api_and_not_reachable_hgridshift) {
+    auto ctx = proj_context_create();
+    proj_grid_cache_set_enable(ctx, false);
+    proj_context_set_enable_network(ctx, true);
+    proj_context_set_url_endpoint(ctx, "http://0.0.0.0");
+
+    // MGI to ETRS89 using at_bev_AT_GIS_GRID_2021_09_28.tif
+    auto P = proj_create_crs_to_crs(ctx, "EPSG:4312", "EPSG:4258", nullptr);
+    ASSERT_NE(P, nullptr);
+
+    PJ_COORD c;
+    c.xyzt.x = 48; // lat
+    c.xyzt.y = 15; // long
+    c.xyzt.z = 0;
+    c.xyzt.t = HUGE_VAL;
+
+    {
+        PJ_COORD c2 = proj_trans(P, PJ_FWD, c);
+        EXPECT_EQ(c2.xyz.x, HUGE_VAL);
+        EXPECT_EQ(proj_errno(P), PROJ_ERR_OTHER_NETWORK_ERROR);
+        PJ *last_op = proj_trans_get_last_used_operation(P);
+        EXPECT_STREQ(proj_pj_info(last_op).description, "MGI to ETRS89 (8)");
+        proj_destroy(last_op);
+    }
+
+    proj_errno_reset(P);
+
+    // Check again. Cf https://github.com/pyproj4/pyproj/issues/705
+    {
+        PJ_COORD c2 = proj_trans(P, PJ_FWD, c);
+        EXPECT_EQ(c2.xyz.x, HUGE_VAL);
+        EXPECT_EQ(proj_errno(P), PROJ_ERR_OTHER_NETWORK_ERROR);
+        PJ *last_op = proj_trans_get_last_used_operation(P);
+        EXPECT_STREQ(proj_pj_info(last_op).description, "MGI to ETRS89 (8)");
+        proj_destroy(last_op);
+    }
+
+    proj_errno_reset(P);
+
+    // Check also reverse direction
+    {
+        PJ_COORD c2 = proj_trans(P, PJ_INV, c);
+        EXPECT_EQ(c2.xyz.x, HUGE_VAL);
+        EXPECT_EQ(proj_errno(P), PROJ_ERR_OTHER_NETWORK_ERROR);
+        PJ *last_op = proj_trans_get_last_used_operation(P);
+        EXPECT_STREQ(proj_pj_info(last_op).description, "MGI to ETRS89 (8)");
+        proj_destroy(last_op);
+    }
+
+    proj_destroy(P);
+    proj_context_destroy(ctx);
+}
+
+#endif
+
+// ---------------------------------------------------------------------------
+
+#ifdef CURL_ENABLED
+
+TEST(networking, network_endpoint_api_and_not_reachable_vgridshift) {
+    auto ctx = proj_context_create();
+    proj_grid_cache_set_enable(ctx, false);
+    proj_context_set_enable_network(ctx, true);
+    proj_context_set_url_endpoint(ctx, "http://0.0.0.0");
+
+    // "POSGAR 2007 to SRVN16 height (1)" using ar_ign_GEOIDE-Ar16.tif
+    auto P = proj_create_crs_to_crs(ctx, "EPSG:5342", "EPSG:9521", nullptr);
+    ASSERT_NE(P, nullptr);
+
+    PJ_COORD c;
+    c.xyzt.x = -40; // lat
+    c.xyzt.y = -60; // long
+    c.xyzt.z = 0;
+    c.xyzt.t = HUGE_VAL;
+
+    {
+        PJ_COORD c2 = proj_trans(P, PJ_FWD, c);
+        EXPECT_EQ(c2.xyz.x, HUGE_VAL);
+        EXPECT_EQ(proj_errno(P), PROJ_ERR_OTHER_NETWORK_ERROR);
+        PJ *last_op = proj_trans_get_last_used_operation(P);
+        EXPECT_STREQ(proj_pj_info(last_op).description,
+                     "POSGAR 2007 to SRVN16 height (1)");
+        proj_destroy(last_op);
+    }
+
+    proj_errno_reset(P);
+
+    // Check again. Cf https://github.com/pyproj4/pyproj/issues/705
+    {
+        PJ_COORD c2 = proj_trans(P, PJ_FWD, c);
+        EXPECT_EQ(c2.xyz.x, HUGE_VAL);
+        EXPECT_EQ(proj_errno(P), PROJ_ERR_OTHER_NETWORK_ERROR);
+        PJ *last_op = proj_trans_get_last_used_operation(P);
+        EXPECT_STREQ(proj_pj_info(last_op).description,
+                     "POSGAR 2007 to SRVN16 height (1)");
+        proj_destroy(last_op);
+    }
+
+    proj_errno_reset(P);
+
+    // Check also reverse direction
+    {
+        PJ_COORD c2 = proj_trans(P, PJ_INV, c);
+        EXPECT_EQ(c2.xyz.x, HUGE_VAL);
+        EXPECT_EQ(proj_errno(P), PROJ_ERR_OTHER_NETWORK_ERROR);
+        PJ *last_op = proj_trans_get_last_used_operation(P);
+        EXPECT_STREQ(proj_pj_info(last_op).description,
+                     "POSGAR 2007 to SRVN16 height (1)");
+        proj_destroy(last_op);
+    }
+
+    proj_destroy(P);
+    proj_context_destroy(ctx);
 }
 
 #endif