Merge pull request #6212 from rouault/5520_refreshed

Fix bug in msQueryByFilter

mappostgis.cpp

@@ -3687,19 +3687,29 @@ static int msPostGISLayerTranslateFilter(layerObj *layer, expressionObj *filter,
         case MS_TOKEN_COMPARISON_CONTAINS:
         case MS_TOKEN_COMPARISON_EQUALS:
         case MS_TOKEN_COMPARISON_DWITHIN:
+        {
           if(node->next->token != '(') goto cleanup;
           native_string += "st_";
-          native_string += msExpressionTokenToString(node->token);
+          const char* str = msExpressionTokenToString(node->token);
+          if( str == nullptr )
+              goto cleanup;
+          native_string += str;
           break;
+        }
 
 	/* functions */
         case MS_TOKEN_FUNCTION_LENGTH:
         case MS_TOKEN_FUNCTION_AREA:
         case MS_TOKEN_FUNCTION_BUFFER:
         case MS_TOKEN_FUNCTION_DIFFERENCE:
+        {
           native_string += "st_";
-          native_string += msExpressionTokenToString(node->token);
+          const char* str = msExpressionTokenToString(node->token);
+          if( str == nullptr )
+              goto cleanup;
+          native_string += str;
           break;
+        }
 
 	case MS_TOKEN_COMPARISON_IEQ:
             if( ieq_expected )
@@ -3724,6 +3734,7 @@ static int msPostGISLayerTranslateFilter(layerObj *layer, expressionObj *filter,
           break;
 
         default:
+        {
           /* by default accept the general token to string conversion */
 
           if(node->token == MS_TOKEN_COMPARISON_EQ && node->next != nullptr && node->next->token == MS_TOKEN_LITERAL_TIME) break; /* skip, handled with the next token */
@@ -3736,9 +3747,13 @@ static int msPostGISLayerTranslateFilter(layerObj *layer, expressionObj *filter,
               break;
           }
 
-          native_string += msExpressionTokenToString(node->token);
+          const char* str = msExpressionTokenToString(node->token);
+          if( str == nullptr )
+              goto cleanup;
+          native_string += str;
           break;
         }
+        }
 
       node = node->next;
     }

mapquery.c

@@ -806,7 +806,7 @@ int msQueryByFilter(mapObj *map)
     paging = msLayerGetPaging(lp);
     msLayerClose(lp); /* reset */
     status = msLayerOpen(lp);
-    if(status != MS_SUCCESS) goto query_error;
+    if(status != MS_SUCCESS) return MS_FAILURE;
     msLayerEnablePaging(lp, paging);
 
     /* disable driver paging */
@@ -825,15 +825,15 @@ int msQueryByFilter(mapObj *map)
       lp->filter = mergeFilters(&map->query.filter, map->query.filteritem, &old_filter, old_filteritem);      
       if(!lp->filter.string) {
 	msSetError(MS_MISCERR, "Filter merge failed, able to process query.", "msQueryByFilter()");
-        goto query_error;
+        goto restore_old_filter;
       }      
     } else {
       msCopyExpression(&lp->filter, &map->query.filter); /* apply new filter */
     }
 
     /* build item list, we want *all* items, note this *also* build tokens for the layer filter */
     status = msLayerWhichItems(lp, MS_TRUE, NULL);
-    if(status != MS_SUCCESS) goto query_error;
+    if(status != MS_SUCCESS) goto restore_old_filter;
 
     search_rect = map->query.rect;
 
@@ -904,7 +904,7 @@ int msQueryByFilter(mapObj *map)
     if(status == MS_DONE) { /* no overlap */
       msLayerClose(lp);
       continue;
-    } else if(status != MS_SUCCESS) goto query_error;
+    } else if(status != MS_SUCCESS) goto restore_old_filter;
 
     lp->resultcache = (resultCacheObj *)malloc(sizeof(resultCacheObj)); /* allocate and initialize the result cache */
     initResultCache( lp->resultcache);
@@ -985,7 +985,7 @@ int msQueryByFilter(mapObj *map)
 
     msProjectDestroyReprojector(reprojector);
 
-    if(status != MS_DONE) goto query_error;
+    if(status != MS_DONE) return MS_FAILURE;
     if(!map->query.only_cache_result_count && lp->resultcache->numresults == 0) 
       msLayerClose(lp); /* no need to keep the layer open */
   } /* next layer */
@@ -999,12 +999,11 @@ int msQueryByFilter(mapObj *map)
   msSetError(MS_NOTFOUND, "No matching record(s) found.", "msQueryByFilter()");
   return MS_FAILURE;
 
-query_error:
-  // msFree(lp->filteritem);
-  // lp->filteritem = old_filteritem;
-  // msCopyExpression(&lp->filter, &old_filter); /* restore old filter */
-  // msFreeExpression(&old_filter);
-  // msLayerClose(lp);
+restore_old_filter:
+  lp->filteritem = old_filteritem;
+  msCopyExpression(&lp->filter, &old_filter); /* restore old filter */
+  msFreeExpression(&old_filter);
+  msLayerClose(lp);
   return MS_FAILURE;
 }
 

msautotest/mspython/test_postgis.py

@@ -316,3 +316,57 @@ def test_postgis_box_bind_values_queryByRect():
             break
         count += 1
     assert count == 15
+
+###############################################################################
+# Test fix for https://github.com/MapServer/MapServer/pull/5520
+
+
+def test_postgis_queryByFilter_bad_filteritem():
+
+    map = mapscript.mapObj()
+    layer = mapscript.layerObj(map)
+    layer.updateFromString("""
+        LAYER
+            CONNECTIONTYPE postgis
+            CONNECTION "dbname=msautotest user=postgres"
+            DATA "the_geom from (select * from province order by gid) as foo using unique gid"
+            NAME mylayer
+            TYPE POLYGON
+            TEMPLATE "junk.tmpl"
+            FILTER 'Cape Breton Island'
+            FILTERITEM 'bad_filter_item'
+        END
+        """)
+
+    layer.open()
+    try:
+        layer.queryByFilter( map, "1 = 1" )
+        assert False
+    except mapscript.MapServerError:
+        pass
+
+    # Check that original filter and filteritem are properly restored
+    assert layer.getFilterString() == '"Cape Breton Island"'
+    assert layer.filteritem == "bad_filter_item"
+
+###############################################################################
+#
+
+
+def test_postgis_queryByFilter_bad_expression():
+
+    map = mapscript.mapObj()
+    layer = mapscript.layerObj(map)
+    layer.updateFromString("""
+        LAYER
+            CONNECTIONTYPE postgis
+            CONNECTION "dbname=msautotest user=postgres"
+            DATA "the_geom from (select * from province order by gid) as foo using unique gid"
+            NAME mylayer
+            TYPE POLYGON
+            TEMPLATE "junk.tmpl"
+        END
+        """)
+
+    layer.open()
+    layer.queryByFilter( map, "ERROR" )