Skip to content

Commit 823f2dd

Browse files
elpasonyalldawson
elpaso
authored and
nyalldawson
committedFeb 6, 2019
Merge pull request #8929 from elpaso/bugfix-21059-server-post-request-issues
Fix POST requests for QGIS server Cherry-picked from master b129850
·
final-3_4_15final-3_4_5
1 parent bc4ccd0 commit 823f2dd

File tree

12 files changed

+236
-108
lines changed

12 files changed

+236
-108
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: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ QgsFcgiServerRequest::QgsFcgiServerRequest()
7171
}
7272

7373
// Store the URL before the server rewrite that could have been set in QUERY_STRING
74-
mOriginalUrl = url;
74+
setOriginalUrl( url );
7575

7676
// OGC parameters are passed with the query string, which is normally part of
7777
// the REQUEST_URI, we override the query string url in case it is defined
@@ -133,26 +133,33 @@ QByteArray QgsFcgiServerRequest::data() const
133133
return mData;
134134
}
135135

136-
QUrl QgsFcgiServerRequest::url() const
137-
{
138-
return mOriginalUrl.isEmpty() ? QgsServerRequest::url() : mOriginalUrl;
139-
}
140-
141136
// Read post put data
142137
void QgsFcgiServerRequest::readData()
143138
{
144139
// Check if we have CONTENT_LENGTH defined
145140
const char *lengthstr = getenv( "CONTENT_LENGTH" );
146141
if ( lengthstr )
147142
{
148-
#ifdef QGISDEBUG
149-
qDebug() << "fcgi: reading " << lengthstr << " bytes from stdin";
150-
#endif
151143
bool success = false;
152144
int length = QString( lengthstr ).toInt( &success );
145+
// Note: REQUEST_BODY is not part of CGI standard, and it is not
146+
// normally passed by any CGI web server and it is implemented only
147+
// to allow unit tests to inject a request body and simulate a POST
148+
// request
149+
const char *request_body = getenv( "REQUEST_BODY" );
150+
if ( success && request_body )
151+
{
152+
QString body( request_body );
153+
body.truncate( length );
154+
mData.append( body.toUtf8() );
155+
length = 0;
156+
}
157+
#ifdef QGISDEBUG
158+
qDebug() << "fcgi: reading " << lengthstr << " bytes from " << ( request_body ? "REQUEST_BODY" : "stdin" );
159+
#endif
153160
if ( success )
154161
{
155-
// XXX This not efficiont at all !!
162+
// XXX This not efficient at all !!
156163
for ( int i = 0; i < length; ++i )
157164
{
158165
mData.append( getchar() );

‎src/server/qgsfcgiserverrequest.h

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

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

@@ -64,8 +52,6 @@ class SERVER_EXPORT QgsFcgiServerRequest: public QgsServerRequest
6452

6553
QByteArray mData;
6654
bool mHasError = false;
67-
//! Url before the server rewrite
68-
QUrl mOriginalUrl;
6955
};
7056

7157
#endif

‎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 \a 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
@@ -37,28 +37,36 @@ namespace QgsWfs
3737

3838
QString serviceUrl( const QgsServerRequest &request, const QgsProject *project )
3939
{
40-
QString href;
40+
QUrl href;
4141
if ( project )
4242
{
43-
href = QgsServerProjectUtils::wfsServiceUrl( *project );
43+
href.setUrl( QgsServerProjectUtils::wfsServiceUrl( *project ) );
4444
}
4545

4646
// Build default url
4747
if ( href.isEmpty() )
4848
{
49-
QUrl url = request.url();
5049

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

57-
url.setQuery( params.urlQuery() );
58-
href = url.toString();
66+
href.setQuery( q );
5967
}
6068

61-
return href;
69+
return href.toString();
6270
}
6371

6472
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
@@ -64,7 +64,7 @@ namespace QgsWmts
6464
// Build default url
6565
if ( href.isEmpty() )
6666
{
67-
QUrl url = request.url();
67+
QUrl url = request.originalUrl();
6868

6969
QgsWmtsParameters params;
7070
params.load( QUrlQuery( url ) );

‎tests/src/python/test_qgsserver_request.py

Lines changed: 131 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,37 @@
2017
__revision__ = '$Format:%H$'
2118

2219

20+
import os
21+
import re
22+
import unittest
23+
from urllib.parse import parse_qs, urlencode, urlparse
24+
2325
from qgis.PyQt.QtCore import QUrl
24-
from qgis.server import QgsServerRequest, QgsFcgiServerRequest
26+
from qgis.server import (QgsBufferServerResponse, QgsFcgiServerRequest,
27+
QgsServerRequest)
28+
from test_qgsserver import QgsServerTestBase
2529

2630

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

2945
def test_requestHeaders(self):
3046
"""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)
47+
headers = {'header-key-1': 'header-value-1',
48+
'header-key-2': 'header-value-2'}
49+
request = QgsServerRequest(
50+
'http://somesite.com/somepath', QgsServerRequest.GetMethod, headers)
3351
for k, v in request.headers().items():
3452
self.assertEqual(headers[k], v)
3553
request.removeHeader('header-key-1')
@@ -40,7 +58,8 @@ def test_requestHeaders(self):
4058

4159
def test_requestParameters(self):
4260
"""Test request parameters"""
43-
request = QgsServerRequest('http://somesite.com/somepath?parm1=val1&parm2=val2', QgsServerRequest.GetMethod)
61+
request = QgsServerRequest(
62+
'http://somesite.com/somepath?parm1=val1&parm2=val2', QgsServerRequest.GetMethod)
4463
parameters = {'PARM1': 'val1', 'PARM2': 'val2'}
4564
for k, v in request.parameters().items():
4665
self.assertEqual(parameters[k], v)
@@ -52,62 +71,130 @@ def test_requestParameters(self):
5271

5372
def test_requestParametersDecoding(self):
5473
"""Test request parameters decoding"""
55-
request = QgsServerRequest('http://somesite.com/somepath?parm1=val1%20%2B+val2', QgsServerRequest.GetMethod)
74+
request = QgsServerRequest(
75+
'http://somesite.com/somepath?parm1=val1%20%2B+val2', QgsServerRequest.GetMethod)
5676
self.assertEqual(request.parameters()['PARM1'], 'val1 + val2')
5777

5878
def test_requestUrl(self):
5979
"""Test url"""
60-
request = QgsServerRequest('http://somesite.com/somepath', QgsServerRequest.GetMethod)
61-
self.assertEqual(request.url().toString(), 'http://somesite.com/somepath')
80+
request = QgsServerRequest(
81+
'http://somesite.com/somepath', QgsServerRequest.GetMethod)
82+
self.assertEqual(request.url().toString(),
83+
'http://somesite.com/somepath')
6284
request.setUrl(QUrl('http://someother.com/someotherpath'))
63-
self.assertEqual(request.url().toString(), 'http://someother.com/someotherpath')
85+
self.assertEqual(request.url().toString(),
86+
'http://someother.com/someotherpath')
6487

6588
def test_requestMethod(self):
66-
request = QgsServerRequest('http://somesite.com/somepath', QgsServerRequest.GetMethod)
89+
request = QgsServerRequest(
90+
'http://somesite.com/somepath', QgsServerRequest.GetMethod)
6791
self.assertEqual(request.method(), QgsServerRequest.GetMethod)
6892
request.setMethod(QgsServerRequest.PostMethod)
6993
self.assertEqual(request.method(), QgsServerRequest.PostMethod)
7094

7195
def test_fcgiRequest(self):
7296
"""Test various combinations of FCGI env parameters with rewritten urls"""
7397

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
98+
def _test_url(original_url, rewritten_url, env={}):
99+
self._set_env(env)
84100
request = QgsFcgiServerRequest()
85-
self.assertEqual(request.url().toString(), url)
101+
self.assertEqual(request.originalUrl().toString(), original_url)
102+
self.assertEqual(request.url().toString(), rewritten_url)
86103
# Check MAP
87104
if 'QUERY_STRING' in env:
88-
map = {k.upper(): v[0] for k, v in parse_qs(env['QUERY_STRING']).items()}['MAP']
105+
map = {k.upper(): v[0] for k, v in parse_qs(
106+
env['QUERY_STRING']).items()}['MAP']
89107
else:
90-
map = {k.upper(): v[0] for k, v in parse_qs(urlparse(env['REQUEST_URI']).query).items()}['MAP']
108+
map = {k.upper(): v[0] for k, v in parse_qs(
109+
urlparse(env['REQUEST_URI']).query).items()}['MAP']
91110
self.assertEqual(request.parameter('MAP'), map)
92111

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-
})
112+
_test_url('http://somesite.com/somepath/project1/',
113+
'http://somesite.com/somepath/project1/?map=/my/project1.qgs', {
114+
'REQUEST_URI': '/somepath/project1/',
115+
'SERVER_NAME': 'somesite.com',
116+
'QUERY_STRING': 'map=/my/project1.qgs'
117+
})
118+
119+
_test_url('http://somesite.com/somepath/path/?token=QGIS2019',
120+
'http://somesite.com/somepath/path/?map=/my/path.qgs', {
121+
'REQUEST_URI': '/somepath/path/?token=QGIS2019',
122+
'SERVER_NAME': 'somesite.com',
123+
'QUERY_STRING': 'map=/my/path.qgs',
124+
})
125+
126+
_test_url('http://somesite.com/somepath/index.html?map=/my/path.qgs',
127+
'http://somesite.com/somepath/index.html?map=/my/path.qgs',
128+
{
129+
'REQUEST_URI': '/somepath/index.html?map=/my/path.qgs',
130+
'SERVER_NAME': 'somesite.com',
131+
})
132+
133+
_test_url('http://somesite.com/somepath?map=/my/path.qgs',
134+
'http://somesite.com/somepath?map=/my/path.qgs',
135+
{
136+
'REQUEST_URI': '/somepath?map=/my/path.qgs',
137+
'SERVER_NAME': 'somesite.com',
138+
})
139+
140+
def test_fcgiRequestPOST(self):
141+
"""Test various combinations of FCGI POST parameters with rewritten urls"""
142+
143+
def _check_links(params, method='GET'):
144+
data = urlencode(params)
145+
if method == 'GET':
146+
env = {
147+
'SERVER_NAME': 'www.myserver.com',
148+
'REQUEST_URI': '/aproject/',
149+
'QUERY_STRING': data,
150+
'REQUEST_METHOD': 'GET',
151+
}
152+
else:
153+
env = {
154+
'SERVER_NAME': 'www.myserver.com',
155+
'REQUEST_URI': '/aproject/',
156+
'REQUEST_BODY': data,
157+
'CONTENT_LENGTH': str(len(data)),
158+
'REQUEST_METHOD': 'POST',
159+
}
160+
161+
self._set_env(env)
162+
request = QgsFcgiServerRequest()
163+
response = QgsBufferServerResponse()
164+
self.server.handleRequest(request, response)
165+
self.assertFalse(b'ServiceExceptionReport' in response.body())
166+
167+
if method == 'POST':
168+
self.assertEqual(request.data(), data.encode('utf8'))
169+
else:
170+
original_url = request.originalUrl().toString()
171+
self.assertTrue(original_url.startswith('http://www.myserver.com/aproject/'))
172+
self.assertEqual(original_url.find(urlencode({'MAP': params['map']})), -1)
173+
174+
exp = re.compile(r'href="([^"]+)"', re.DOTALL | re.MULTILINE)
175+
elems = exp.findall(bytes(response.body()).decode('utf8'))
176+
self.assertTrue(len(elems) > 0)
177+
for href in elems:
178+
self.assertTrue(href.startswith('http://www.myserver.com/aproject/'))
179+
self.assertEqual(href.find(urlencode({'MAP': params['map']})), -1)
180+
181+
# Test post request handler
182+
params = {
183+
'map': os.path.join(self.testdata_path, 'test_project_wfs.qgs'),
184+
'REQUEST': 'GetCapabilities',
185+
'SERVICE': 'WFS',
186+
}
187+
_check_links(params)
188+
_check_links(params, 'POST')
189+
params['SERVICE'] = 'WMS'
190+
_check_links(params)
191+
_check_links(params, 'POST')
192+
params['SERVICE'] = 'WCS'
193+
_check_links(params)
194+
_check_links(params, 'POST')
195+
params['SERVICE'] = 'WMTS'
196+
_check_links(params)
197+
_check_links(params, 'POST')
111198

112199

113200
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.