From 0498ddd0477c408ac23bf8d555f8da6693ccf63b Mon Sep 17 00:00:00 2001 From: Alessandro Pasotti Date: Wed, 27 Jan 2021 16:16:40 +0100 Subject: [PATCH 1/3] Add test for server + encoding --- tests/src/python/test_qgsserver_wms_getmap.py | 43 ++++++++++++++++++- 1 file changed, 42 insertions(+), 1 deletion(-) diff --git a/tests/src/python/test_qgsserver_wms_getmap.py b/tests/src/python/test_qgsserver_wms_getmap.py index 265a0165ec7d..be0f44f0acfc 100644 --- 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() From 3ca980f01448a6c20d1467089dc3eab82e7d76b0 Mon Sep 17 00:00:00 2001 From: Alessandro Pasotti Date: Wed, 27 Jan 2021 17:18:59 +0100 Subject: [PATCH 2/3] WMS provider: urlencode layer names in GetMap URL requests Fixes #41116 Successfully tested on QGIS Server and GeoServer --- src/providers/wms/qgswmsprovider.cpp | 4 ++-- tests/src/providers/testqgswmsprovider.cpp | 18 ++++++++++++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/providers/wms/qgswmsprovider.cpp b/src/providers/wms/qgswmsprovider.cpp index 77577fa9df2e..4a7bf550c4ce 100644 --- 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/tests/src/providers/testqgswmsprovider.cpp b/tests/src/providers/testqgswmsprovider.cpp index 1fa749ab8a7f..bd39a4ee2f5e 100644 --- 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( "test", "plus+sign" ); + 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 ); From 55573ede933726408700c1eaf3d5213a4d47a26d Mon Sep 17 00:00:00 2001 From: Alessandro Pasotti Date: Tue, 2 Feb 2021 16:47:11 +0100 Subject: [PATCH 3/3] Server: remove double url decoding Fixes #41116 and #39436 (test taken from that) --- src/server/qgsrequesthandler.cpp | 5 +--- .../testqgsserverquerystringparameter.cpp | 25 +++++++++++++++++++ 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/src/server/qgsrequesthandler.cpp b/src/server/qgsrequesthandler.cpp index 0afc90822835..77accaf871e1 100644 --- a/src/server/qgsrequesthandler.cpp +++ b/src/server/qgsrequesthandler.cpp @@ -220,10 +220,7 @@ void QgsRequestHandler::parseInput() const QList 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/server/testqgsserverquerystringparameter.cpp b/tests/src/server/testqgsserverquerystringparameter.cpp index 0daa6a4cce5a..126f6e0087e7 100644 --- 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 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"