Fix concatenation of query params to WFS 2.0.0 url when it already contains params #944
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
WebFeatureService_.getGETGetFeatureRequest() is used by WebFeatureService_2_0_0.getfeature() to prepare the getfeature() request url from a base url and dictionary of query parameters. The implementation doesn't correctly handle a base url that already contains a parameter list. For example, if base url is:
https://my.url.com.au/ows?authkey=myauthkey
then the url returned by getGETGetFeatureRequest will be:
https://my.url.com.au/ows?authkey=myauthkey?service=WFS&version=2.0.0&request=GetFeature&typename=mylayer
Note that this url contains two ?. Unfortunately openUrl() doesn't successfully open this url (in the case of the 'authkey' parameter, a 401 Unauthorized error occurs because presumably the geoserver doesn't parse the url in the way that we expect). Regardless of the precise issue (if any) that occurs with a url containing multiple ?, it doesn't seem correct to concatenate the url in this way.
There is already a function in util.py that builds a GET url taking into account a query expression in the base url - build_get_url() - so this has been used in getGETGetFeatureRequest to concatenate the WFS parameters to the base url. There is one case that build_get_url() might not handle correctly - where the query parameter dictionary has a list (sequence) for a value - for example {"myparam": ['myval']}. Ideally the call to urlencode in build_get_url() would specify doseq=True but it doesn't.
Interestingly getGETGetFeatureRequest() is not used by the WFS 1.0.0 or 1.1.0 getfeature() requests. However, it appears that both of these have the same problem with an extra ?, but the url is successfully encoded - it appears that openURL is called differently with these implementations, where base_url and data are kept separate in the call to openURL, so requests.request() is able to fix the duplicate ?.