Skip to content

Commit

Permalink
Fix segfault on WFS filters with empty literals (PostGIS + Spatialite…
Browse files Browse the repository at this point in the history
…). Implement PropertyIsNull for those layers (MapServer#5347)

The PropertyIsNull is handled through a hack that consists in emulating it as a
PropertyIsEqualTo "_MAPSERVER_NULL_" comparison. This is only done for PostGIS and
Spatialite layers.

CREDITS: Funded by:
Regione Toscana - Settore Sistema Informativo Territoriale ed
Ambientale (CIG: 644544015A)
  • Loading branch information
rouault committed Nov 26, 2016
1 parent ac253a7 commit a0e8ee0
Show file tree
Hide file tree
Showing 14 changed files with 251 additions and 2 deletions.
51 changes: 51 additions & 0 deletions mapogcfilter.c
Original file line number Diff line number Diff line change
Expand Up @@ -3667,6 +3667,57 @@ int FLTCheckInvalidOperand(FilterEncodingNode *psFilterNode)
return status;
}

/************************************************************************/
/* FLTProcessPropertyIsNull */
/* */
/* HACK for PropertyIsNull processing. PostGIS & Spatialite only */
/* for now. */
/************************************************************************/
int FLTProcessPropertyIsNull(FilterEncodingNode *psFilterNode,
mapObj *map, int i)
{
int status = MS_SUCCESS;

if (psFilterNode->eType == FILTER_NODE_TYPE_COMPARISON &&
psFilterNode->psLeftNode != NULL &&
psFilterNode->psLeftNode->eType == FILTER_NODE_TYPE_PROPERTYNAME &&
strcmp(psFilterNode->pszValue, "PropertyIsNull") == 0 &&
!FLTIsGMLDefaultProperty(psFilterNode->psLeftNode->pszValue) )
{
layerObj* lp;
int layerWasOpened;

lp = GET_LAYER(map, i);
layerWasOpened = msLayerIsOpen(lp);

/* Horrible HACK to compensate for the lack of null testing in MapServer */
if( (lp->connectiontype == MS_POSTGIS ||
(lp->connectiontype == MS_OGR && msOGRIsSpatialite(lp))) &&
strcmp(psFilterNode->pszValue, "PropertyIsNull") == 0 )
{
msFree(psFilterNode->pszValue);
psFilterNode->pszValue = msStrdup("PropertyIsEqualTo");
psFilterNode->psRightNode = FLTCreateBinaryCompFilterEncodingNode();
psFilterNode->psRightNode->eType = FILTER_NODE_TYPE_LITERAL;
psFilterNode->psRightNode->pszValue = msStrdup("_MAPSERVER_NULL_");
}

if (!layerWasOpened) /* do not close the layer if it has been opened somewhere else (paging?) */
msLayerClose(lp);
}

if (psFilterNode->psLeftNode)
{
status = FLTProcessPropertyIsNull(psFilterNode->psLeftNode, map, i);
if( status == MS_SUCCESS )
{
if (psFilterNode->psRightNode)
status = FLTProcessPropertyIsNull(psFilterNode->psRightNode, map, i);
}
}
return status;
}

/************************************************************************/
/* FLTCheckInvalidProperty */
/* */
Expand Down
2 changes: 2 additions & 0 deletions mapogcfilter.h
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,8 @@ int FLTCheckInvalidProperty(FilterEncodingNode *psFilterNode,
FilterEncodingNode* FLTSimplify(FilterEncodingNode *psFilterNode,
int* pnEvaluation);
int FLTApplyFilterToLayerCommonExpressionWithRect(mapObj *map, int iLayerIndex, const char *pszExpression, rectObj rect);
int FLTProcessPropertyIsNull(FilterEncodingNode *psFilterNode,
mapObj *map, int i);

#endif

Expand Down
39 changes: 38 additions & 1 deletion mapogr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1531,7 +1531,7 @@ static char *msOGREscapeSQLParam(layerObj *layer, const char *pszString)
{
#ifdef USE_OGR
char* pszEscapedStr =NULL;
if(layer && pszString && strlen(pszString) > 0) {
if(layer && pszString) {
char* pszEscapedOGRStr = CPLEscapeString(pszString, strlen(pszString),
CPLES_SQL );
pszEscapedStr = msStrdup(pszEscapedOGRStr);
Expand Down Expand Up @@ -1617,6 +1617,14 @@ char *msOGRGetToken(layerObj* layer, tokenListNodeObjPtr *node) {
out = msStrdup(n->tokenval.dblval == 0 ? "FALSE" : "TRUE");
break;
case MS_TOKEN_COMPARISON_EQ:
if(n->next != NULL && n->next->token == MS_TOKEN_LITERAL_STRING &&
strcmp(n->next->tokenval.strval, "_MAPSERVER_NULL_") == 0 )
{
out = msStrdup(" IS NULL");
n = n->next;
break;
}

out = msStrdup(" = ");
break;
case MS_TOKEN_COMPARISON_NE:
Expand Down Expand Up @@ -3035,6 +3043,35 @@ static int msOGRLayerIsOpen(layerObj *layer)
#endif /* USE_OGR */
}

int msOGRIsSpatialite(layerObj* layer)
{
#ifdef USE_OGR
msOGRFileInfo *psInfo =(msOGRFileInfo*)layer->layerinfo;
if (psInfo && psInfo->dialect &&
EQUAL(psInfo->dialect, "Spatialite") )
{
// reasons to not produce native string: not simple layer, or an explicit deny
char *do_this = msLayerGetProcessingKey(layer, "NATIVE_SQL"); // default is YES
if (do_this && strcmp(do_this, "NO") == 0) {
return MS_FALSE;
}
return MS_TRUE;
}

return MS_FALSE;

#else
/* ------------------------------------------------------------------
* OGR Support not included...
* ------------------------------------------------------------------ */

msSetError(MS_MISCERR, "OGR support is not available.", "msOGRIsSpatialite()");
return(MS_FALSE);

#endif /* USE_OGR */
}


/**********************************************************************
* msOGRLayerWhichShapes()
*
Expand Down
9 changes: 8 additions & 1 deletion mappostgis.c
Original file line number Diff line number Diff line change
Expand Up @@ -3454,7 +3454,7 @@ char *msPostGISEscapeSQLParam(layerObj *layer, const char *pszString)
size_t nSrcLen;
char* pszEscapedStr =NULL;

if (layer && pszString && strlen(pszString) > 0) {
if (layer && pszString) {
if(!msPostGISLayerIsOpen(layer))
msPostGISLayerOpen(layer);

Expand Down Expand Up @@ -3752,6 +3752,13 @@ int msPostGISLayerTranslateFilter(layerObj *layer, expressionObj *filter, char *

if(node->token == MS_TOKEN_COMPARISON_EQ && node->next != NULL && node->next->token == MS_TOKEN_LITERAL_TIME) break; /* skip, handled with the next token */
if(bindingToken == MS_TOKEN_BINDING_TIME && (node->token == MS_TOKEN_COMPARISON_EQ || node->token == MS_TOKEN_COMPARISON_NE)) break; /* skip, handled elsewhere */
if(node->token == MS_TOKEN_COMPARISON_EQ && node->next != NULL && node->next->token == MS_TOKEN_LITERAL_STRING &&
strcmp(node->next->tokenval.strval, "_MAPSERVER_NULL_") == 0 )
{
native_string = msStringConcatenate(native_string, " IS NULL");
node = node->next;
break;
}

native_string = msStringConcatenate(native_string, msExpressionTokenToString(node->token));
break;
Expand Down
2 changes: 2 additions & 0 deletions mapserver.h
Original file line number Diff line number Diff line change
Expand Up @@ -3130,6 +3130,8 @@ shapeObj *msOffsetCurve(shapeObj *p, double offset);
shapeObj *msGEOSOffsetCurve(shapeObj *p, double offset);
#endif

int msOGRIsSpatialite(layerObj* layer);

#endif /* SWIG */

#ifdef __cplusplus
Expand Down
2 changes: 2 additions & 0 deletions mapwfs.c
Original file line number Diff line number Diff line change
Expand Up @@ -2065,6 +2065,8 @@ static int msWFSRunFilter(mapObj* map,
return msWFSException(map, "mapserv", MS_OWS_ERROR_NO_APPLICABLE_CODE, paramsObj->pszVersion);
}

FLTProcessPropertyIsNull(psNode, map, lp->index);

/*preparse the filter for gml aliases*/
FLTPreParseFilterForAliasAndGroup(psNode, map, lp->index, "G");

Expand Down
Binary file modified msautotest/wxs/data/db.sqlite
Binary file not shown.
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
Content-Type: text/xml; charset=UTF-8

<?xml version='1.0' encoding="UTF-8" ?>
<wfs:FeatureCollection
xmlns:ms="http://mapserver.gis.umn.edu/mapserver"
xmlns:wfs="http://www.opengis.net/wfs"
xmlns:gml="http://www.opengis.net/gml"
xmlns:ogc="http://www.opengis.net/ogc"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://www.opengis.net/wfs http://ogc.dmsolutions.ca/wfs/1.0.0/WFS-basic.xsd
http://mapserver.gis.umn.edu/mapserver http://localhost/path/to/wfs_simple?SERVICE=WFS&amp;VERSION=1.0.0&amp;REQUEST=DescribeFeatureType&amp;TYPENAME=popplace&amp;OUTPUTFORMAT=XMLSCHEMA">
<gml:boundedBy>
<gml:null>missing</gml:null>
</gml:boundedBy>
</wfs:FeatureCollection>

Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
Content-Type: text/xml; subtype=gml/3.1.1; charset=UTF-8

<?xml version='1.0' encoding="UTF-8" ?>
<wfs:FeatureCollection
xmlns:ms="http://mapserver.gis.umn.edu/mapserver"
xmlns:gml="http://www.opengis.net/gml"
xmlns:wfs="http://www.opengis.net/wfs"
xmlns:ogc="http://www.opengis.net/ogc"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://mapserver.gis.umn.edu/mapserver http://localhost/path/to/wfs_simple?SERVICE=WFS&amp;VERSION=1.1.0&amp;REQUEST=DescribeFeatureType&amp;TYPENAME=popplace&amp;OUTPUTFORMAT=text/xml;%20subtype=gml/3.1.1 http://www.opengis.net/wfs http://ogc.dmsolutions.ca/wfs/1.1.0/wfs.xsd">
<gml:boundedBy>
<gml:Envelope srsName="EPSG:4326">
<gml:lowerCorner>46.980843 -64.052355</gml:lowerCorner>
<gml:upperCorner>46.980843 -64.052355</gml:upperCorner>
</gml:Envelope>
</gml:boundedBy>
<gml:featureMember>
<ms:popplace>
<gml:boundedBy>
<gml:Envelope srsName="EPSG:4326">
<gml:lowerCorner>46.980843 -64.052355</gml:lowerCorner>
<gml:upperCorner>46.980843 -64.052355</gml:upperCorner>
</gml:Envelope>
</gml:boundedBy>
<ms:msGeometry>
<gml:Point srsName="EPSG:4326">
<gml:pos>46.980843 -64.052355</gml:pos>
</gml:Point>
</ms:msGeometry>
<ms:gid>1</ms:gid>
<ms:area>0</ms:area>
<ms:perimeter>0</ms:perimeter>
<ms:popplace_>8</ms:popplace_>
<ms:popplace_i>1</ms:popplace_i>
<ms:unique_key>BACMK</ms:unique_key>
<ms:name>&quot;Tignish</ms:name>
<ms:name_e></ms:name_e>
<ms:name_f></ms:name_f>
<ms:unique_k_1></ms:unique_k_1>
<ms:unique_k_2></ms:unique_k_2>
<ms:reg_code>11</ms:reg_code>
<ms:nts50>021I16</ms:nts50>
<ms:lat>465700</ms:lat>
<ms:long>640200</ms:long>
<ms:sgc_code>0</ms:sgc_code>
<ms:capital>0</ms:capital>
<ms:pop_range>1</ms:pop_range>
</ms:popplace>
</gml:featureMember>
</wfs:FeatureCollection>

Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
Content-Type: text/xml; subtype=gml/3.1.1; charset=UTF-8

<?xml version='1.0' encoding="UTF-8" ?>
<wfs:FeatureCollection
xmlns:ms="http://mapserver.gis.umn.edu/mapserver"
xmlns:gml="http://www.opengis.net/gml"
xmlns:wfs="http://www.opengis.net/wfs"
xmlns:ogc="http://www.opengis.net/ogc"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://mapserver.gis.umn.edu/mapserver http://localhost/path/to/wfs_simple?SERVICE=WFS&amp;VERSION=1.1.0&amp;REQUEST=DescribeFeatureType&amp;TYPENAME=popplace&amp;OUTPUTFORMAT=text/xml;%20subtype=gml/3.1.1 http://www.opengis.net/wfs http://ogc.dmsolutions.ca/wfs/1.1.0/wfs.xsd">
<gml:boundedBy>
<gml:Null>missing</gml:Null>
</gml:boundedBy>
</wfs:FeatureCollection>

19 changes: 19 additions & 0 deletions msautotest/wxs/expected/wfs_ogr_native_sql_24.xml
Original file line number Diff line number Diff line change
Expand Up @@ -775,5 +775,24 @@ Content-Type: text/xml; charset=UTF-8
<ms:region>0</ms:region>
</ms:towns>
</gml:featureMember>
<gml:featureMember>
<ms:towns>
<gml:boundedBy>
<gml:Box srsName="EPSG:32632">
<gml:coordinates>653901.530000,4866278.050000 653901.530000,4866278.050000</gml:coordinates>
</gml:Box>
</gml:boundedBy>
<ms:msGeometry>
<gml:Point srsName="EPSG:32632">
<gml:coordinates>653901.530000,4866278.050000</gml:coordinates>
</gml:Point>
</ms:msGeometry>
<ms:name></ms:name>
<ms:peoples></ms:peoples>
<ms:localcounc></ms:localcounc>
<ms:county></ms:county>
<ms:region></ms:region>
</ms:towns>
</gml:featureMember>
</wfs:FeatureCollection>

39 changes: 39 additions & 0 deletions msautotest/wxs/expected/wfs_ogr_native_sql_40.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
Content-Type: text/xml; subtype=gml/3.1.1; charset=UTF-8

<?xml version='1.0' encoding="UTF-8" ?>
<wfs:FeatureCollection
xmlns:ms="http://mapserver.gis.umn.edu/mapserver"
xmlns:gml="http://www.opengis.net/gml"
xmlns:wfs="http://www.opengis.net/wfs"
xmlns:ogc="http://www.opengis.net/ogc"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://mapserver.gis.umn.edu/mapserver http://localhost/path/to/wfs_simple?SERVICE=WFS&amp;VERSION=1.1.0&amp;REQUEST=DescribeFeatureType&amp;TYPENAME=towns&amp;OUTPUTFORMAT=text/xml;%20subtype=gml/3.1.1 http://www.opengis.net/wfs http://schemas.opengis.net/wfs/1.1.0/wfs.xsd">
<gml:boundedBy>
<gml:Envelope srsName="EPSG:32632">
<gml:lowerCorner>653901.530000 4866278.050000</gml:lowerCorner>
<gml:upperCorner>653901.530000 4866278.050000</gml:upperCorner>
</gml:Envelope>
</gml:boundedBy>
<!-- WARNING: FeatureId item 'ID' not found in typename 'towns'. -->
<gml:featureMember>
<ms:towns>
<gml:boundedBy>
<gml:Envelope srsName="EPSG:32632">
<gml:lowerCorner>653901.530000 4866278.050000</gml:lowerCorner>
<gml:upperCorner>653901.530000 4866278.050000</gml:upperCorner>
</gml:Envelope>
</gml:boundedBy>
<ms:msGeometry>
<gml:Point srsName="EPSG:32632">
<gml:pos>653901.530000 4866278.050000</gml:pos>
</gml:Point>
</ms:msGeometry>
<ms:name></ms:name>
<ms:peoples></ms:peoples>
<ms:localcounc></ms:localcounc>
<ms:county></ms:county>
<ms:region></ms:region>
</ms:towns>
</gml:featureMember>
</wfs:FeatureCollection>

5 changes: 5 additions & 0 deletions msautotest/wxs/wfs_filter_postgis.map
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@
# Verify PropertyIsEqualTo with no matching (due to case sensitivity)
# RUN_PARMS: wfs_filter_postgis_property_is_equal_no_matching.xml [MAPSERV] QUERY_STRING="map=[MAPFILE]&SERVICE=WFS&VERSION=1.0.0&REQUEST=GetFeature&TYPENAME=popplace&FILTER=<Filter><PropertyIsEqualTo><PropertyName>name</PropertyName><Literal>DIGBY</Literal></PropertyIsEqualTo></Filter>" > [RESULT]
# RUN_PARMS: wfs_filter_postgis_property_is_equal_case_insensitive.xml [MAPSERV] QUERY_STRING="map=[MAPFILE]&SERVICE=WFS&VERSION=1.0.0&REQUEST=GetFeature&TYPENAME=popplace&FILTER=<Filter><PropertyIsEqualTo matchCase=\"false\"><PropertyName>name</PropertyName><Literal>DIGBY</Literal></PropertyIsEqualTo></Filter>" > [RESULT]
# RUN_PARMS: wfs_filter_postgis_property_is_equal_empty.xml [MAPSERV] QUERY_STRING="map=[MAPFILE]&SERVICE=WFS&VERSION=1.0.0&REQUEST=GetFeature&TYPENAME=popplace&FILTER=<Filter><PropertyIsEqualTo><PropertyName>name</PropertyName><Literal></Literal></PropertyIsEqualTo></Filter>" > [RESULT]

# RUN_PARMS: wfs_filter_postgis_property_is_null_zero_result.xml [MAPSERV] QUERY_STRING="map=[MAPFILE]&SERVICE=WFS&VERSION=1.1.0&REQUEST=GetFeature&TYPENAME=popplace&FILTER=<Filter><PropertyIsNull><PropertyName>name</PropertyName></PropertyIsNull></Filter>&MAXFEATURES=1" > [RESULT]

# RUN_PARMS: wfs_filter_postgis_property_is_null_result.xml [MAPSERV] QUERY_STRING="map=[MAPFILE]&SERVICE=WFS&VERSION=1.1.0&REQUEST=GetFeature&TYPENAME=popplace&FILTER=<Filter><PropertyIsNull><PropertyName>name_e</PropertyName></PropertyIsNull></Filter>&MAXFEATURES=1" > [RESULT]

# Test quotes in filter
# RUN_PARMS: wfs_filter_postgis_property_is_equal_quote1.xml [MAPSERV] QUERY_STRING="map=[MAPFILE]&SERVICE=WFS&VERSION=1.0.0&REQUEST=GetFeature&TYPENAME=popplace&FILTER=<Filter><PropertyIsEqualTo><PropertyName>name</PropertyName><Literal>\"Tignish</Literal></PropertyIsEqualTo></Filter>" > [RESULT]
Expand Down
3 changes: 3 additions & 0 deletions msautotest/wxs/wfs_ogr_native_sql.map
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,9 @@
#
# Spatial query on a layer (unoptimized case with a 'SELECT ... FROM ... WHERE' DATA statement) that returns zero features
# RUN_PARMS: wfs_ogr_native_sql_39.xml [MAPSERV] QUERY_STRING="map=[MAPFILE]&SERVICE=WFS&VERSION=1.0.0&REQUEST=GetFeature&TYPENAME=select_zero_feature&OUTPUTFORMAT=GML2&FILTER=<Filter><BBOX><PropertyName>Geometry</PropertyName><Box srsName='EPSG:32632'><coordinates>659159,4877386 696879,4898059</coordinates></Box></BBOX></Filter>" > [RESULT]
#
# PropertyIsNull
# RUN_PARMS: wfs_ogr_native_sql_40.xml [MAPSERV] QUERY_STRING="map=[MAPFILE]&SERVICE=WFS&VERSION=1.1.0&REQUEST=GetFeature&TYPENAME=towns&FILTER=<Filter><PropertyIsNull><PropertyName>name</PropertyName></PropertyIsNull></Filter>" > [RESULT]

MAP

Expand Down

0 comments on commit a0e8ee0

Please sign in to comment.