Skip to content
Permalink
Browse files
[Bugfix][Server] WFS GetFeature GML: CDATA was url encoded before insert
in GML

The CDATA section was not well formed in the GML build by QGIS Server. We found `&lt;![CDATA[ ]]&gt;`, instead of `<![CDATA[ ]]>`.

For exemple we found
```xml
<qgs:name>&lt;![CDATA[FOO &amp; FOO]]&gt;</qgs:name>
```
instead of
```xml
<qgs:name><![CDATA[FOO &amp; FOO]]></qgs:name>
```

* Funded by Faunalia
  • Loading branch information
Gustry authored and github-actions[bot] committed Mar 18, 2022
1 parent 6118e03 commit f34725d4581261a744e1b50188966779f1c64aec
Show file tree
Hide file tree
Showing 6 changed files with 902 additions and 45 deletions.
@@ -69,6 +69,8 @@ namespace QgsWfs

QString createFeatureGeoJSON( const QgsFeature &feature, const createFeatureParams &params, const QgsAttributeList &pkAttributes );

QDomElement createFieldElement( const QgsField &field, const QVariant &value, QDomDocument &doc );

QString encodeValueToText( const QVariant &value, const QgsEditorWidgetSetup &setup );

QDomElement createFeatureGML2( const QgsFeature &feature, QDomDocument &doc, const createFeatureParams &params, const QgsProject *project, const QgsAttributeList &pkAttributes );
@@ -1412,26 +1414,17 @@ namespace QgsWfs
}

//read all attribute values from the feature
QgsAttributes featureAttributes = feature.attributes();
QgsFields fields = feature.fields();
const QgsAttributes featureAttributes = feature.attributes();
const QgsFields fields = feature.fields();
for ( int i = 0; i < params.attributeIndexes.count(); ++i )
{
int idx = params.attributeIndexes[i];
if ( idx >= fields.count() )
{
continue;
}
const QgsField field = fields.at( idx );
const QgsEditorWidgetSetup setup = field.editorWidgetSetup();
QString attributeName = field.name();

QDomElement fieldElem = doc.createElement( "qgs:" + attributeName.replace( ' ', '_' ).replace( cleanTagNameRegExp, QString() ) );
QDomText fieldText = doc.createTextNode( encodeValueToText( featureAttributes[idx], setup ) );
if ( featureAttributes[idx].isNull() )
{
fieldElem.setAttribute( QStringLiteral( "xsi:nil" ), QStringLiteral( "true" ) );
}
fieldElem.appendChild( fieldText );
const QDomElement fieldElem = createFieldElement( fields.at( idx ), featureAttributes[idx], doc );
typeNameElement.appendChild( fieldElem );
}

@@ -1444,7 +1437,7 @@ namespace QgsWfs
QDomElement featureElement = doc.createElement( QStringLiteral( "gml:featureMember" )/*wfs:FeatureMember*/ );

//qgs:%TYPENAME%
QDomElement typeNameElement = doc.createElement( "qgs:" + params.typeName /*qgs:%TYPENAME%*/ );
QDomElement typeNameElement = doc.createElement( QStringLiteral( "qgs:" ) + params.typeName /*qgs:%TYPENAME%*/ );
QString id = QStringLiteral( "%1.%2" ).arg( params.typeName, QgsServerFeatureId::getServerFid( feature, pkAttributes ) );
typeNameElement.setAttribute( QStringLiteral( "gml:id" ), id );
featureElement.appendChild( typeNameElement );
@@ -1518,8 +1511,8 @@ namespace QgsWfs
}

//read all attribute values from the feature
QgsAttributes featureAttributes = feature.attributes();
QgsFields fields = feature.fields();
const QgsAttributes featureAttributes = feature.attributes();
const QgsFields fields = feature.fields();
for ( int i = 0; i < params.attributeIndexes.count(); ++i )
{
int idx = params.attributeIndexes[i];
@@ -1528,22 +1521,36 @@ namespace QgsWfs
continue;
}

const QgsField field = fields.at( idx );
const QgsEditorWidgetSetup setup = field.editorWidgetSetup();
const QDomElement fieldElem = createFieldElement( fields.at( idx ), featureAttributes[idx], doc );
typeNameElement.appendChild( fieldElem );
}

QString attributeName = field.name();
return featureElement;
}

QDomElement fieldElem = doc.createElement( "qgs:" + attributeName.replace( ' ', '_' ).replace( cleanTagNameRegExp, QString() ) );
QDomText fieldText = doc.createTextNode( encodeValueToText( featureAttributes[idx], setup ) );
if ( featureAttributes[idx].isNull() )
QDomElement createFieldElement( const QgsField &field, const QVariant &value, QDomDocument &doc )
{
const QgsEditorWidgetSetup setup = field.editorWidgetSetup();
const QString attributeName = field.name().replace( ' ', '_' ).replace( cleanTagNameRegExp, QString() );
QDomElement fieldElem = doc.createElement( QStringLiteral( "qgs:" ) + attributeName );
if ( value.isNull() )
{
fieldElem.setAttribute( QStringLiteral( "xsi:nil" ), QStringLiteral( "true" ) );
}
else
{
const QString fieldText = encodeValueToText( value, setup );
//do we need CDATA
if ( fieldText.indexOf( '<' ) != -1 || fieldText.indexOf( '&' ) != -1 )
{
fieldElem.setAttribute( QStringLiteral( "xsi:nil" ), QStringLiteral( "true" ) );
fieldElem.appendChild( doc.createCDATASection( fieldText ) );
}
else
{
fieldElem.appendChild( doc.createTextNode( fieldText ) );
}
fieldElem.appendChild( fieldText );
typeNameElement.appendChild( fieldElem );
}

return featureElement;
return fieldElem;
}

QString encodeValueToText( const QVariant &value, const QgsEditorWidgetSetup &setup )
@@ -1590,27 +1597,11 @@ namespace QgsWfs
case QVariant::StringList:
case QVariant::List:
case QVariant::Map:
{
QString v = QgsJsonUtils::encodeValue( value );

//do we need CDATA
if ( v.indexOf( '<' ) != -1 || v.indexOf( '&' ) != -1 )
v.prepend( QStringLiteral( "<![CDATA[" ) ).append( QStringLiteral( "]]>" ) );

return v;
}
return QgsJsonUtils::encodeValue( value );

default:
case QVariant::String:
{
QString v = value.toString();

//do we need CDATA
if ( v.indexOf( '<' ) != -1 || v.indexOf( '&' ) != -1 )
v.prepend( QStringLiteral( "<![CDATA[" ) ).append( QStringLiteral( "]]>" ) );

return v;
}
return value.toString();
}
}

@@ -77,7 +77,10 @@ def assertXMLEqual(self, response, expected, msg='', raw=False):
for diff in difflib.unified_diff([l.decode('utf8') for l in expected_lines], [l.decode('utf8') for l in response_lines]):
diffs.append(diff)

self.assertEqual(len(expected_lines), len(response_lines), "Expected and response have different number of lines!\n{}\n{}".format(msg, '\n'.join(diffs)))
self.assertEqual(
len(expected_lines),
len(response_lines),
"Expected and response have different number of lines!\n{}\n{}\nWe got :\n{}".format(msg, '\n'.join(diffs), '\n'.join([i.decode("utf-8") for i in response_lines])))
for expected_line in expected_lines:
expected_line = expected_line.strip()
response_line = response_lines[line_no - 1].strip()
@@ -599,6 +599,15 @@ def test_describeFeatureType(self):
self.wfs_request_compare("DescribeFeatureType", '1.1.0', "TYPENAME=does_not_exist&",
'wfs_describeFeatureType_1_1_0_typename_wrong', project_file=project_file)

def test_GetFeature_with_cdata(self):
""" Test GetFeature with CDATA."""
self.wfs_request_compare(
"GetFeature",
"1.0.0",
"TYPENAME=test_layer_wfs_cdata_lines&",
'wfs_getfeature_cdata',
project_file="test_layer_wfs_cdata.qgs")

def test_describeFeatureTypeVirtualFields(self):
"""Test DescribeFeatureType with virtual fields: bug GH-29767"""

@@ -0,0 +1,11 @@
{
"type": "FeatureCollection",
"name": "test_lines",
"features": [
{ "type": "Feature", "properties": { "id": 1, "name": "éù%@ > 1", "comment": "Accents, sup" }, "geometry": { "type": "LineString", "coordinates": [ [ 3.8, 43.5 ], [ 3.8, 43.6 ] ] } },
{ "type": "Feature", "properties": { "id": 2, "name": "Line > 2", "comment": "Normal, sup" }, "geometry": { "type": "LineString", "coordinates": [ [ 3.8, 43.6 ], [ 3.9, 43.6 ] ] } },
{ "type": "Feature", "properties": { "id": 3, "name": "Line < 3", "comment": "Normal, inf" }, "geometry": { "type": "LineString", "coordinates": [ [ 3.9, 43.6 ], [ 3.9, 43.5 ] ] } },
{ "type": "Feature", "properties": { "id": 4, "name": "05200", "comment": "Trailing 0" }, "geometry": { "type": "LineString", "coordinates": [ [ 3.9, 43.5 ], [ 3.8, 43.5 ] ] } },
{ "type": "Feature", "properties": { "id": 5, "name": "Line & 2", "comment": "And sign" }, "geometry": { "type": "LineString", "coordinates": [ [ 3.8, 43.5 ], [ 3.9, 43.6 ] ] } }
]
}

0 comments on commit f34725d

Please sign in to comment.