Skip to content

Commit 691176b

Browse files
elpasoelpaso
committedJan 21, 2019
Fix POST requests for QGIS server
Followup #8830 that fixed a regression with rewritten urls in the server, unfortunately my original solution introduced a side-effect on the POST request, with the new approach I'm introducing a new method to retrieve the URL as seen by the web server: by default this is the same URL seen by QGIS server, but in case a rewrite module made some changes, the original URL will be used as a base URL if not overridden by a config setting. This PR comes with an extended set of tests that should cover both (rewritten and unrewritten) cases for GET and POST and for WFS/WFS/WCS and WMTS.
·
ltr-3_40final-3_6_0
1 parent 1b11ba4 commit 691176b

File tree

12 files changed

+241
-106
lines changed

12 files changed

+241
-106
lines changed
 

‎python/server/auto_generated/qgsfcgiserverrequest.sip.in

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -32,20 +32,6 @@ Class defining fcgi request
3232
Returns true if an error occurred during initialization
3333
%End
3434

35-
virtual QUrl url() const;
36-
37-
%Docstring
38-
39-
:return: the request url
40-
41-
Overrides base implementation because FCGI is typically behind
42-
a proxy server and QGIS Server will see a rewritten QUERY_STRING.
43-
FCGI implementation stores the REQUEST_URI (which is the URL seen
44-
by the proxy before it gets rewritten) and returns it instead of
45-
the rewritten one.
46-
%End
47-
48-
4935
};
5036

5137
/************************************************************************

‎python/server/auto_generated/qgsserverrequest.sip.in

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -56,14 +56,13 @@ Constructor
5656

5757
virtual ~QgsServerRequest();
5858

59-
virtual QUrl url() const;
59+
QUrl url() const;
6060
%Docstring
6161

62-
:return: the request url
62+
:return: the request url as seen by QGIS server
6363

64-
Subclasses may override in case the original URL (not rewritten, e.g.from
65-
a web server rewrite module) needs to be returned instead of the URL
66-
seen by QGIS server.
64+
.. seealso:: :py:func:`originalUrl`
65+
server, by default the two are equal
6766
%End
6867

6968
QgsServerRequest::Method method() const;
@@ -139,13 +138,35 @@ is available.
139138
void setUrl( const QUrl &url );
140139
%Docstring
141140
Set the request url
141+
%End
142+
143+
QUrl originalUrl() const;
144+
%Docstring
145+
Returns the request url as seen by the web server,
146+
by default this is equal to the url seen by QGIS server
147+
148+
.. seealso:: :py:func:`url`
149+
150+
.. versionadded:: 3.6
142151
%End
143152

144153
void setMethod( QgsServerRequest::Method method );
145154
%Docstring
146155
Set the request method
147156
%End
148157

158+
protected:
159+
160+
void setOriginalUrl( const QUrl &url );
161+
%Docstring
162+
Set the request original \url (the request url as seen by the web server)
163+
164+
.. seealso:: :py:func:`setUrl`
165+
166+
.. versionadded:: 3.6
167+
%End
168+
169+
149170
};
150171

151172
/************************************************************************

‎src/server/qgsfcgiserverrequest.cpp

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ QgsFcgiServerRequest::QgsFcgiServerRequest()
6868
}
6969

7070
// Store the URL before the server rewrite that could have been set in QUERY_STRING
71-
mOriginalUrl = url;
71+
setOriginalUrl( url );
7272

7373
// OGC parameters are passed with the query string, which is normally part of
7474
// the REQUEST_URI, we override the query string url in case it is defined
@@ -130,26 +130,29 @@ QByteArray QgsFcgiServerRequest::data() const
130130
return mData;
131131
}
132132

133-
QUrl QgsFcgiServerRequest::url() const
134-
{
135-
return mOriginalUrl.isEmpty() ? QgsServerRequest::url() : mOriginalUrl;
136-
}
137-
138133
// Read post put data
139134
void QgsFcgiServerRequest::readData()
140135
{
141136
// Check if we have CONTENT_LENGTH defined
142137
const char *lengthstr = getenv( "CONTENT_LENGTH" );
143138
if ( lengthstr )
144139
{
145-
#ifdef QGISDEBUG
146-
qDebug() << "fcgi: reading " << lengthstr << " bytes from stdin";
147-
#endif
148140
bool success = false;
149141
int length = QString( lengthstr ).toInt( &success );
142+
#ifdef QGISDEBUG
143+
const char *request_body = getenv( "REQUEST_BODY" );
144+
if ( success && request_body )
145+
{
146+
QString body( request_body );
147+
body.truncate( length );
148+
mData.append( body.toUtf8() );
149+
length = 0;
150+
}
151+
qDebug() << "fcgi: reading " << lengthstr << " bytes from " << ( request_body ? "REQUEST_BODY" : "stdin" );
152+
#endif
150153
if ( success )
151154
{
152-
// XXX This not efficiont at all !!
155+
// XXX This not efficient at all !!
153156
for ( int i = 0; i < length; ++i )
154157
{
155158
mData.append( getchar() );

‎src/server/qgsfcgiserverrequest.h

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -41,18 +41,6 @@ class SERVER_EXPORT QgsFcgiServerRequest: public QgsServerRequest
4141
*/
4242
bool hasError() const { return mHasError; }
4343

44-
/**
45-
* \returns the request url
46-
*
47-
* Overrides base implementation because FCGI is typically behind
48-
* a proxy server and QGIS Server will see a rewritten QUERY_STRING.
49-
* FCGI implementation stores the REQUEST_URI (which is the URL seen
50-
* by the proxy before it gets rewritten) and returns it instead of
51-
* the rewritten one.
52-
*/
53-
QUrl url() const override;
54-
55-
5644
private:
5745
void readData();
5846

‎src/server/qgsserverrequest.cpp

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ QgsServerRequest::QgsServerRequest( const QString &url, Method method, const Hea
2727

2828
QgsServerRequest::QgsServerRequest( const QUrl &url, Method method, const Headers &headers )
2929
: mUrl( url )
30+
, mOriginalUrl( url )
3031
, mMethod( method )
3132
, mHeaders( headers )
3233
{
@@ -60,6 +61,16 @@ QUrl QgsServerRequest::url() const
6061
return mUrl;
6162
}
6263

64+
QUrl QgsServerRequest::originalUrl() const
65+
{
66+
return mOriginalUrl;
67+
}
68+
69+
void QgsServerRequest::setOriginalUrl( const QUrl &url )
70+
{
71+
mOriginalUrl = url;
72+
}
73+
6374
QgsServerRequest::Method QgsServerRequest::method() const
6475
{
6576
return mMethod;

‎src/server/qgsserverrequest.h

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -82,13 +82,12 @@ class SERVER_EXPORT QgsServerRequest
8282
virtual ~QgsServerRequest() = default;
8383

8484
/**
85-
* \returns the request url
85+
* \returns the request url as seen by QGIS server
8686
*
87-
* Subclasses may override in case the original URL (not rewritten, e.g.from
88-
* a web server rewrite module) needs to be returned instead of the URL
89-
* seen by QGIS server.
87+
* \see originalUrl for the unrewritten url as seen by the web
88+
* server, by default the two are equal
9089
*/
91-
virtual QUrl url() const;
90+
QUrl url() const;
9291

9392
/**
9493
* \returns the request method
@@ -159,13 +158,36 @@ class SERVER_EXPORT QgsServerRequest
159158
*/
160159
void setUrl( const QUrl &url );
161160

161+
/**
162+
* Returns the request url as seen by the web server,
163+
* by default this is equal to the url seen by QGIS server
164+
*
165+
* \see url() for the rewritten url
166+
* \since QGIS 3.6
167+
*/
168+
QUrl originalUrl() const;
169+
162170
/**
163171
* Set the request method
164172
*/
165173
void setMethod( QgsServerRequest::Method method );
166174

175+
protected:
176+
177+
/**
178+
* Set the request original \url (the request url as seen by the web server)
179+
*
180+
* \see setUrl() for the rewritten url
181+
* \since QGIS 3.6
182+
*/
183+
void setOriginalUrl( const QUrl &url );
184+
185+
167186
private:
187+
// Url as seen by QGIS server after web server rewrite
168188
QUrl mUrl;
189+
// Unrewritten url as seen by the web server
190+
QUrl mOriginalUrl;
169191
Method mMethod = GetMethod;
170192
// We mark as mutable in order
171193
// to support lazy initialization

‎src/server/services/wcs/qgswcsutils.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -250,7 +250,7 @@ namespace QgsWcs
250250
// Build default url
251251
if ( href.isEmpty() )
252252
{
253-
QUrl url = request.url();
253+
QUrl url = request.originalUrl();
254254
QUrlQuery q( url );
255255

256256
for ( auto param : q.queryItems() )

‎src/server/services/wfs/qgswfsutils.cpp

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -36,28 +36,36 @@ namespace QgsWfs
3636

3737
QString serviceUrl( const QgsServerRequest &request, const QgsProject *project )
3838
{
39-
QString href;
39+
QUrl href;
4040
if ( project )
4141
{
42-
href = QgsServerProjectUtils::wfsServiceUrl( *project );
42+
href.setUrl( QgsServerProjectUtils::wfsServiceUrl( *project ) );
4343
}
4444

4545
// Build default url
4646
if ( href.isEmpty() )
4747
{
48-
QUrl url = request.url();
4948

50-
QgsWfsParameters params;
51-
params.load( QUrlQuery( url ) );
52-
params.remove( QgsServerParameter::REQUEST );
53-
params.remove( QgsServerParameter::VERSION_SERVICE );
54-
params.remove( QgsServerParameter::SERVICE );
49+
static QSet<QString> sFilter
50+
{
51+
QStringLiteral( "REQUEST" ),
52+
QStringLiteral( "VERSION" ),
53+
QStringLiteral( "SERVICE" ),
54+
};
55+
56+
href = request.originalUrl();
57+
QUrlQuery q( href );
58+
59+
for ( auto param : q.queryItems() )
60+
{
61+
if ( sFilter.contains( param.first.toUpper() ) )
62+
q.removeAllQueryItems( param.first );
63+
}
5564

56-
url.setQuery( params.urlQuery() );
57-
href = url.toString();
65+
href.setQuery( q );
5866
}
5967

60-
return href;
68+
return href.toString();
6169
}
6270

6371
QString layerTypeName( const QgsMapLayer *layer )

‎src/server/services/wms/qgswmsutils.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ namespace QgsWms
5656
QStringLiteral( "_DC" )
5757
};
5858

59-
href = request.url();
59+
href = request.originalUrl();
6060
QUrlQuery q( href );
6161

6262
for ( auto param : q.queryItems() )

‎src/server/services/wmts/qgswmtsutils.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ namespace QgsWmts
5959
// Build default url
6060
if ( href.isEmpty() )
6161
{
62-
QUrl url = request.url();
62+
QUrl url = request.originalUrl();
6363

6464
QgsWmtsParameters params;
6565
params.load( QUrlQuery( url ) );

‎tests/src/python/test_qgsserver_request.py

Lines changed: 140 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,6 @@
99
(at your option) any later version.
1010
1111
"""
12-
import unittest
13-
import os
14-
from urllib.parse import parse_qs, urlparse
1512

1613
__author__ = 'Alessandro Pasotti'
1714
__date__ = '29/04/2017'
@@ -20,16 +17,38 @@
2017
__revision__ = '$Format:%H$'
2118

2219

20+
import os
21+
import unittest
22+
from urllib.parse import parse_qs, urlencode, urlparse
23+
24+
from lxml import etree as ET
25+
2326
from qgis.PyQt.QtCore import QUrl
24-
from qgis.server import QgsServerRequest, QgsFcgiServerRequest
27+
from qgis.server import (QgsBufferServerResponse, QgsFcgiServerRequest,
28+
QgsServerRequest)
29+
from test_qgsserver import QgsServerTestBase
2530

2631

27-
class QgsServerRequestTest(unittest.TestCase):
32+
class QgsServerRequestTest(QgsServerTestBase):
33+
34+
@staticmethod
35+
def _set_env(env={}):
36+
for k in ('QUERY_STRING', 'REQUEST_URI', 'SERVER_NAME', 'CONTENT_LENGTH', 'SERVER_PORT', 'SCRIPT_NAME', 'REQUEST_BODY', 'REQUEST_METHOD'):
37+
try:
38+
del os.environ[k]
39+
except KeyError:
40+
pass
41+
try:
42+
os.environ[k] = env[k]
43+
except KeyError:
44+
pass
2845

2946
def test_requestHeaders(self):
3047
"""Test request headers"""
31-
headers = {'header-key-1': 'header-value-1', 'header-key-2': 'header-value-2'}
32-
request = QgsServerRequest('http://somesite.com/somepath', QgsServerRequest.GetMethod, headers)
48+
headers = {'header-key-1': 'header-value-1',
49+
'header-key-2': 'header-value-2'}
50+
request = QgsServerRequest(
51+
'http://somesite.com/somepath', QgsServerRequest.GetMethod, headers)
3352
for k, v in request.headers().items():
3453
self.assertEqual(headers[k], v)
3554
request.removeHeader('header-key-1')
@@ -40,7 +59,8 @@ def test_requestHeaders(self):
4059

4160
def test_requestParameters(self):
4261
"""Test request parameters"""
43-
request = QgsServerRequest('http://somesite.com/somepath?parm1=val1&parm2=val2', QgsServerRequest.GetMethod)
62+
request = QgsServerRequest(
63+
'http://somesite.com/somepath?parm1=val1&parm2=val2', QgsServerRequest.GetMethod)
4464
parameters = {'PARM1': 'val1', 'PARM2': 'val2'}
4565
for k, v in request.parameters().items():
4666
self.assertEqual(parameters[k], v)
@@ -52,62 +72,138 @@ def test_requestParameters(self):
5272

5373
def test_requestParametersDecoding(self):
5474
"""Test request parameters decoding"""
55-
request = QgsServerRequest('http://somesite.com/somepath?parm1=val1%20%2B+val2', QgsServerRequest.GetMethod)
75+
request = QgsServerRequest(
76+
'http://somesite.com/somepath?parm1=val1%20%2B+val2', QgsServerRequest.GetMethod)
5677
self.assertEqual(request.parameters()['PARM1'], 'val1 + val2')
5778

5879
def test_requestUrl(self):
5980
"""Test url"""
60-
request = QgsServerRequest('http://somesite.com/somepath', QgsServerRequest.GetMethod)
61-
self.assertEqual(request.url().toString(), 'http://somesite.com/somepath')
81+
request = QgsServerRequest(
82+
'http://somesite.com/somepath', QgsServerRequest.GetMethod)
83+
self.assertEqual(request.url().toString(),
84+
'http://somesite.com/somepath')
6285
request.setUrl(QUrl('http://someother.com/someotherpath'))
63-
self.assertEqual(request.url().toString(), 'http://someother.com/someotherpath')
86+
self.assertEqual(request.url().toString(),
87+
'http://someother.com/someotherpath')
6488

6589
def test_requestMethod(self):
66-
request = QgsServerRequest('http://somesite.com/somepath', QgsServerRequest.GetMethod)
90+
request = QgsServerRequest(
91+
'http://somesite.com/somepath', QgsServerRequest.GetMethod)
6792
self.assertEqual(request.method(), QgsServerRequest.GetMethod)
6893
request.setMethod(QgsServerRequest.PostMethod)
6994
self.assertEqual(request.method(), QgsServerRequest.PostMethod)
7095

7196
def test_fcgiRequest(self):
7297
"""Test various combinations of FCGI env parameters with rewritten urls"""
7398

74-
def _test_url(url, env={}):
75-
for k in ('QUERY_STRING', 'REQUEST_URI', 'SERVER_NAME', 'SERVER_PORT', 'SCRIPT_NAME'):
76-
try:
77-
del os.environ[k]
78-
except KeyError:
79-
pass
80-
try:
81-
os.environ[k] = env[k]
82-
except KeyError:
83-
pass
99+
def _test_url(original_url, rewritten_url, env={}):
100+
self._set_env(env)
84101
request = QgsFcgiServerRequest()
85-
self.assertEqual(request.url().toString(), url)
102+
self.assertEqual(request.originalUrl().toString(), original_url)
103+
self.assertEqual(request.url().toString(), rewritten_url)
86104
# Check MAP
87105
if 'QUERY_STRING' in env:
88-
map = {k.upper(): v[0] for k, v in parse_qs(env['QUERY_STRING']).items()}['MAP']
106+
map = {k.upper(): v[0] for k, v in parse_qs(
107+
env['QUERY_STRING']).items()}['MAP']
89108
else:
90-
map = {k.upper(): v[0] for k, v in parse_qs(urlparse(env['REQUEST_URI']).query).items()}['MAP']
109+
map = {k.upper(): v[0] for k, v in parse_qs(
110+
urlparse(env['REQUEST_URI']).query).items()}['MAP']
91111
self.assertEqual(request.parameter('MAP'), map)
92112

93-
_test_url('http://somesite.com/somepath/index.html?map=/my/path.qgs', {
94-
'REQUEST_URI': '/somepath/index.html?map=/my/path.qgs',
95-
'SERVER_NAME': 'somesite.com',
96-
})
97-
_test_url('http://somesite.com/somepath?map=/my/path.qgs', {
98-
'REQUEST_URI': '/somepath?map=/my/path.qgs',
99-
'SERVER_NAME': 'somesite.com',
100-
})
101-
_test_url('http://somesite.com/somepath/path', {
102-
'REQUEST_URI': '/somepath/path',
103-
'SERVER_NAME': 'somesite.com',
104-
'QUERY_STRING': 'map=/my/path.qgs'
105-
})
106-
_test_url('http://somesite.com/somepath/path/?token=QGIS2019', {
107-
'REQUEST_URI': '/somepath/path/?token=QGIS2019',
108-
'SERVER_NAME': 'somesite.com',
109-
'QUERY_STRING': 'map=/my/path.qgs',
110-
})
113+
_test_url('http://somesite.com/somepath/project1/',
114+
'http://somesite.com/somepath/project1/?map=/my/project1.qgs', {
115+
'REQUEST_URI': '/somepath/project1/',
116+
'SERVER_NAME': 'somesite.com',
117+
'QUERY_STRING': 'map=/my/project1.qgs'
118+
})
119+
120+
_test_url('http://somesite.com/somepath/path/?token=QGIS2019',
121+
'http://somesite.com/somepath/path/?map=/my/path.qgs', {
122+
'REQUEST_URI': '/somepath/path/?token=QGIS2019',
123+
'SERVER_NAME': 'somesite.com',
124+
'QUERY_STRING': 'map=/my/path.qgs',
125+
})
126+
127+
_test_url('http://somesite.com/somepath/index.html?map=/my/path.qgs',
128+
'http://somesite.com/somepath/index.html?map=/my/path.qgs',
129+
{
130+
'REQUEST_URI': '/somepath/index.html?map=/my/path.qgs',
131+
'SERVER_NAME': 'somesite.com',
132+
})
133+
134+
_test_url('http://somesite.com/somepath?map=/my/path.qgs',
135+
'http://somesite.com/somepath?map=/my/path.qgs',
136+
{
137+
'REQUEST_URI': '/somepath?map=/my/path.qgs',
138+
'SERVER_NAME': 'somesite.com',
139+
})
140+
141+
def test_fcgiRequestPOST(self):
142+
"""Test various combinations of FCGI POST parameters with rewritten urls"""
143+
144+
def _check_links(params, method='GET'):
145+
data = urlencode(params)
146+
if method == 'GET':
147+
env = {
148+
'SERVER_NAME': 'www.myserver.com',
149+
'REQUEST_URI': '/aproject/',
150+
'QUERY_STRING': data,
151+
'REQUEST_METHOD': 'GET',
152+
}
153+
else:
154+
env = {
155+
'SERVER_NAME': 'www.myserver.com',
156+
'REQUEST_URI': '/aproject/',
157+
'REQUEST_BODY': data,
158+
'CONTENT_LENGTH': str(len(data)),
159+
'REQUEST_METHOD': 'POST',
160+
}
161+
162+
self._set_env(env)
163+
request = QgsFcgiServerRequest()
164+
response = QgsBufferServerResponse()
165+
self.server.handleRequest(request, response)
166+
self.assertFalse(b'ServiceExceptionReport' in response.body())
167+
168+
if method == 'POST':
169+
self.assertEqual(request.data(), data.encode('utf8'))
170+
else:
171+
original_url = request.originalUrl().toString()
172+
self.assertTrue(original_url.startswith('http://www.myserver.com/aproject/'))
173+
self.assertEqual(original_url.find(urlencode({'MAP': params['map']})), -1)
174+
175+
e = ET.fromstring(bytes(response.body()))
176+
elems = []
177+
for ns in ('wms', 'wfs', 'wcs'):
178+
elems += e.xpath('//xxx:OnlineResource', namespaces={'xxx': 'http://www.opengis.net/%s' % ns})
179+
elems += e.xpath('//ows:Get/ows:OnlineResource', namespaces={'ows': 'http://www.opengis.net/ows'})
180+
elems += e.xpath('//ows:Post', namespaces={'ows': 'http://www.opengis.net/ows'})
181+
elems += e.xpath('//ows:Get', namespaces={'ows': 'http://www.opengis.net/ows'})
182+
elems += e.xpath('//ows:Get', namespaces={'ows': 'http://www.opengis.net/ows/1.1'})
183+
self.assertTrue(len(elems) > 0)
184+
for _e in elems:
185+
attrs = _e.attrib
186+
href = attrs['{http://www.w3.org/1999/xlink}href']
187+
self.assertTrue(href.startswith('http://www.myserver.com/aproject/'))
188+
self.assertEqual(href.find(urlencode({'MAP': params['map']})), -1)
189+
190+
# Test post request handler
191+
params = {
192+
'map': os.path.join(self.testdata_path, 'test_project_wfs.qgs'),
193+
'REQUEST': 'GetCapabilities',
194+
'SERVICE': 'WFS',
195+
}
196+
_check_links(params)
197+
_check_links(params, 'POST')
198+
params['SERVICE'] = 'WMS'
199+
_check_links(params)
200+
_check_links(params, 'POST')
201+
params['SERVICE'] = 'WCS'
202+
_check_links(params)
203+
_check_links(params, 'POST')
204+
params['SERVICE'] = 'WMTS'
205+
_check_links(params)
206+
_check_links(params, 'POST')
111207

112208

113209
if __name__ == '__main__':

‎tests/src/python/test_qgsserver_wfst.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,10 @@
44
Tests for WFS-T provider using QGIS Server through qgis_wrapped_server.py.
55
66
This is an integration test for QGIS Desktop WFS-T provider and QGIS Server
7-
WFS-T that check if QGIS can talk to and uderstand itself.
7+
WFS-T that check if QGIS can talk to and understand itself.
88
99
The test uses testdata/wfs_transactional/wfs_transactional.qgs and three
10-
initially empty shapefiles layrs with points, lines and polygons.
10+
initially empty shapefiles layers with points, lines and polygons.
1111
1212
All WFS-T calls are executed through the QGIS WFS data provider.
1313

0 commit comments

Comments
 (0)
Please sign in to comment.