[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 `<![CDATA[ ]]>`, instead of `<![CDATA[ ]]>`. For exemple we found ```xml <qgs:name><![CDATA[FOO & FOO]]></qgs:name> ``` instead of ```xml <qgs:name><![CDATA[FOO & FOO]]></qgs:name> ``` * Funded by Faunalia
qgis · Mar 18, 2022 · f34725d · f34725d
1 parent 6118e03
commit f34725d
Show file tree

Hide file tree

Showing 6 changed files with 902 additions and 45 deletions.
diff --git a/src/server/services/wfs/qgswfsgetfeature.cpp b/src/server/services/wfs/qgswfsgetfeature.cpp
@@ -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();
       }
     }
 

diff --git a/tests/src/python/test_qgsserver.py b/tests/src/python/test_qgsserver.py
@@ -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()

diff --git a/tests/src/python/test_qgsserver_wfs.py b/tests/src/python/test_qgsserver_wfs.py
@@ -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"""
 

diff --git a/tests/testdata/qgis_server/test_layer_wfs_cdata.geojson b/tests/testdata/qgis_server/test_layer_wfs_cdata.geojson
@@ -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 ] ] } }
+]
+}