Merge pull request #41211 from elpaso/bugfix-gh41116-wms-urlencode

WMS provider: urlencode layer names in GetMap URL requests
qgis · Feb 3, 2021 · 846bd8c · 846bd8c
2 parents 489401f + 55573ed
commit 846bd8c
Show file tree

Hide file tree

Showing 5 changed files with 88 additions and 7 deletions.
diff --git a/src/providers/wms/qgswmsprovider.cpp b/src/providers/wms/qgswmsprovider.cpp
@@ -1041,8 +1041,8 @@ QUrl QgsWmsProvider::createRequestUrlWMS( const QgsRectangle &viewExtent, int pi
   {
     if ( mActiveSubLayerVisibility.constFind( *it ).value() )
     {
-      visibleLayers += *it;
-      visibleStyles += *it2;
+      visibleLayers += QUrl::toPercentEncoding( *it );
+      visibleStyles += QUrl::toPercentEncoding( *it2 );
     }
 
     ++it2;

diff --git a/src/server/qgsrequesthandler.cpp b/src/server/qgsrequesthandler.cpp
@@ -220,10 +220,7 @@ void QgsRequestHandler::parseInput()
         const QList<pair_t> items = query.queryItems();
         for ( const pair_t &pair : items )
         {
-          // QUrl::fromPercentEncoding doesn't replace '+' with space
-          const QString key = QUrl::fromPercentEncoding( QString( pair.first ).replace( '+', ' ' ).toUtf8() );
-          const QString value = QUrl::fromPercentEncoding( QString( pair.second ).replace( '+', ' ' ).toUtf8() );
-          mRequest.setParameter( key.toUpper(), value );
+          mRequest.setParameter( pair.first, pair.second );
         }
         setupParameters();
       }

diff --git a/tests/src/providers/testqgswmsprovider.cpp b/tests/src/providers/testqgswmsprovider.cpp
@@ -101,6 +101,24 @@ class TestQgsWmsProvider: public QObject
                                          "STYLES=&FORMAT=&TRANSPARENT=TRUE" ) );
     }
 
+    // regression #41116
+    void queryItemsWithPlusSign()
+    {
+      const QString failingAddress( "layers=plus+sign&styles=&url=http://localhost:8380/mapserv" );
+      const QgsWmsParserSettings config;
+      QgsWmsCapabilities cap;
+      QFile file( QStringLiteral( TEST_DATA_DIR ) + "/provider/GetCapabilities.xml" );
+      QVERIFY( file.open( QIODevice::ReadOnly | QIODevice::Text ) );
+      const QByteArray content = file.readAll().replace( "<Name>test</Name>",  "<Name>plus+sign</Name>" );
+      QVERIFY( cap.parseResponse( content, config ) );
+      QgsWmsProvider provider( failingAddress, QgsDataProvider::ProviderOptions(), &cap );
+      QUrl url( provider.createRequestUrlWMS( QgsRectangle( 0, 0, 90, 90 ), 100, 100 ) );
+      QCOMPARE( url.toString(), QString( "http://localhost:8380/mapserv?SERVICE=WMS&VERSION=1.3.0&REQUEST=GetMap&BBOX=0,0,90,90&"
+                                         "CRS=EPSG:2056&WIDTH=100&HEIGHT=100&"
+                                         "LAYERS=plus%2Bsign&STYLES=&FORMAT=&TRANSPARENT=TRUE" ) );
+    }
+
+
     void noCrsSpecified()
     {
       QgsWmsProvider provider( QStringLiteral( "http://localhost:8380/mapserv?xxx&layers=agri_zones&styles=&format=image/jpg" ), QgsDataProvider::ProviderOptions(), mCapabilities );

diff --git a/tests/src/python/test_qgsserver_wms_getmap.py b/tests/src/python/test_qgsserver_wms_getmap.py
@@ -32,7 +32,7 @@
 
 from test_qgsserver import QgsServerTestBase
 from utilities import unitTestDataPath
-from qgis.core import QgsProject
+from qgis.core import QgsProject, QgsVectorLayer
 
 # Strip path and content length because path may vary
 RE_STRIP_UNCHECKABLE = br'MAP=[^"]+|Content-Length: \d+'
@@ -1811,6 +1811,47 @@ def test_multiple_layers_with_equal_name(self):
         r, h = self._result(self._execute_request(qs))
         self._img_diff_error(r, h, "WMS_GetMap_DuplicateNames")
 
+    def test_wms_getmap_plus_sign(self):
+        """Test issue GH #41116"""
+
+        vl = QgsVectorLayer('Point?crs=epsg:4326&field=int:integer', 'test+plus', 'memory')
+        p = QgsProject()
+        p.addMapLayers([vl])
+        qs = "?" + "&".join(["%s=%s" % i for i in list({
+            "SERVICE": "WMS",
+            "VERSION": "1.1.1",
+            "REQUEST": "GetMap",
+            "LAYERS": urllib.parse.quote('test+plus'),
+            "STYLES": "",
+            "FORMAT": "image/png",
+            "BBOX": "-170,-80,170,80",
+            "HEIGHT": "500",
+            "WIDTH": "500",
+            "CRS": "EPSG:4326"
+        }.items())])
+
+        r, h = self._result(self._execute_request_project(qs, p))
+        # No exceptions
+        self.assertEqual(h['Content-Type'], 'image/png')
+        self.assertFalse(b"The layer 'test plus' does not exist" in r)
+
+        # + literal: we get an exception
+        qs = "?" + "&".join(["%s=%s" % i for i in list({
+            "SERVICE": "WMS",
+            "VERSION": "1.1.1",
+            "REQUEST": "GetMap",
+            "LAYERS": 'test+plus',
+            "STYLES": "",
+            "FORMAT": "image/png",
+            "BBOX": "-170,-80,170,80",
+            "HEIGHT": "500",
+            "WIDTH": "500",
+            "CRS": "EPSG:4326"
+        }.items())])
+
+        r, h = self._result(self._execute_request_project(qs, p))
+        self.assertTrue(b"The layer 'test plus' does not exist" in r)
+
 
 if __name__ == '__main__':
     unittest.main()
diff --git a/tests/src/server/testqgsserverquerystringparameter.cpp b/tests/src/server/testqgsserverquerystringparameter.cpp
@@ -23,6 +23,9 @@
 #include "qgsserverapicontext.h"
 #include "qgsserverrequest.h"
 #include "qgsserverexception.h"
+#include "qgsrequesthandler.h"
+#include "qgsbufferserverrequest.h"
+#include "qgsbufferserverresponse.h"
 
 /**
  * \ingroup UnitTests
@@ -56,6 +59,9 @@ class TestQgsServerQueryStringParameter : public QObject
 
     // Test default values
     void testDefaultValues();
+
+    // Test QgsRequestHandler::parseInput (i.e. POST requests) with special chars
+    void testParseInput();
 };
 
 
@@ -174,5 +180,24 @@ void TestQgsServerQueryStringParameter::testDefaultValues()
 
 }
 
+void TestQgsServerQueryStringParameter::testParseInput()
+{
+  // Request with layers "a", "b", "c" and "äös + %&#"
+  QByteArray data( "SERVICE=WMS&VERSION=1.3.0&REQUEST=GetMap&LAYERS=a%2Cb%2Cc%2C%C3%A4%C3%B6s+%2B+%25%26%23" );
+  QgsBufferServerRequest request( QStringLiteral( "http://localhost/wms/test" ), QgsServerRequest::PostMethod, QgsServerRequest::Headers(), &data );
+  QgsBufferServerResponse response;
+
+  QgsRequestHandler requestHandler( request, response );
+  requestHandler.parseInput();
+
+  QgsServerParameters params = request.serverParameters();
+  QMap<QString, QString> paramsMap = params.toMap();
+  QCOMPARE( paramsMap["SERVICE"], QStringLiteral( "WMS" ) );
+  QCOMPARE( paramsMap["VERSION"], QStringLiteral( "1.3.0" ) );
+  QCOMPARE( paramsMap["REQUEST"], QStringLiteral( "GetMap" ) );
+  QCOMPARE( paramsMap["LAYERS"], QStringLiteral( "a,b,c,äös + %&#" ) );
+}
+
+
 QGSTEST_MAIN( TestQgsServerQueryStringParameter )
 #include "testqgsserverquerystringparameter.moc"