geopython / OWSLib

OWSLib is a Python package for client programming with Open Geospatial Consortium (OGC) web service (hence OWS) interface standards, and their related content models.
https://owslib.readthedocs.io
BSD 3-Clause "New" or "Revised" License
381 stars 273 forks source link

Issue with URL encoding of parameters in WMS #846

Open ThomasG77 opened 1 year ago

ThomasG77 commented 1 year ago

Why? See below two URLs

Unworking URL. Obtained with Owslib that encode characters /,, et : and it's normal

http://ogc.geo-ide.developpement-durable.gouv.fr/wxs?map=/opt/data/carto/geoide-catalogue/1.4/org_38178/908a2fc2-6752-4eae-952a-142393e657b7.internet.map&service=WMS&version=1.3.0&request=GetMap&layers=N_PERIM_MAET_ZINF_S_R11&styles=&width=800&height=448&crs=EPSG%3A4326&bbox=48.1107%2C1.44041%2C49.2484%2C3.56583&format=image%2Fpng&transparent=FALSE&bgcolor=0xFFFFFF&exceptions=XML

vs

Working URL

http://ogc.geo-ide.developpement-durable.gouv.fr/wxs?map=/opt/data/carto/geoide-catalogue/1.4/org_38178/908a2fc2-6752-4eae-952a-142393e657b7.internet.map&service=WMS&version=1.3.0&request=GetMap&layers=N_PERIM_MAET_ZINF_S_R11&styles=&width=800&height=448&crs=EPSG:4326&bbox=48.1107,1.44041,49.2484,3.56583&format=image/png&transparent=FALSE&bgcolor=0xFFFFFF&exceptions=XML

The issue seems to be on the server side of the service I consume that does not decode the encoded parameters.

Code to reproduce how I got the following URL

from owslib.wms import WebMapService

wms = WebMapService('https://ogc.geo-ide.developpement-durable.gouv.fr/wxs?map=/opt/data/carto/geoide-catalogue/1.4/org_38178/908a2fc2-6752-4eae-952a-142393e657b7.internet.map', version='1.3.0')

response = wms.getmap(layers=['N_PERIM_MAET_ZINF_S_R11',],
    bbox=(1.44041, 48.1107, 3.56583, 49.2484),
    format='image/png',
    size=(800, 428),
    srs='EPSG:4326',
)
with open(f"N_PERIM_MAET_ZINF_S_R11.png", 'wb') as out:
    out.write(response.read())

Possible approach

To solve the issue, I want to use the safe option of https://docs.python.org/3/library/urllib.parse.html#urllib.parse.urlencode

getmap method would get a new safe option, empty by default (like the normal urlencode behavior) but could be changed at https://github.com/geopython/OWSLib/blob/master/owslib/map/wms111.py#L258 and https://github.com/geopython/OWSLib/blob/master/owslib/map/wms111.py#L258 and https://github.com/geopython/OWSLib/blob/master/owslib/map/wms130.py#L304 where

data = urlencode(request)

would be replaced by

data = urlencode(request,safe)

So, it can help solve the issue when you do not control the server encode/decode behavior while not breaking anything. I would be able to provide to the new safe option the value :/, in my case (already tested with hardcoded value while debugging) .

PS: I can do a PR but need to confirm the logic could be fine on owslib project. Moreover, I've only look at the WMS code part but this could be applied maybe to other types of webservices with the same issue.

sbrunner commented 1 year ago

When you create the WebMapService the map path should be encoded:

wms = WebMapService('https://ogc.geo-ide.developpement-durable.gouv.fr/wxs?map=%2Fopt%2Fdata%2Fcarto%2Fgeoide-catalogue%2F1.4%2Forg_38178%2F908a2fc2-6752-4eae-952a-142393e657b7.internet.map', version='1.3.0')

And you should be directly in https because the http to https redirect will break the URL...

ThomasG77 commented 1 year ago

From a correct code perspective, the suggestion is surely right but it does not solve my sample code at all as it gives the same issue after change.

~Did I miss something obvious?!~

Ok. I understand that changing http to https in the samples URLs solve the issue but it's owlib that return/do http calls instead of https and I can't change the behavior without fixing issue at owlib level.

From my understanding, the issue is related to the fact owslib internally use http scheme from xlink:href in the capabilities from https://ogc.geo-ide.developpement-durable.gouv.fr/wxs?map=/opt/data/carto/geoide-catalogue/1.4/org_38178/908a2fc2-6752-4eae-952a-142393e657b7.internet.map&service=WMS&request=GetCapabilities so although the redirection http to https create "garbage", I do not see how a more obvious way to solve the issue as my first proposition.

sbrunner commented 1 year ago

With the version 0.25.0 it's working...

sbrunner commented 1 year ago

I think that a better solution is to have an option to skip this kind of code... https://github.com/geopython/OWSLib/blob/018c7f83fb042a6390d65aaa683110743609f3e3/owslib/map/wms130.py#L283-L285 and use directly self.url :-)

ThomasG77 commented 1 year ago

Using 0.27.2 and not working. Tested also with 0.26.0 and breaking too.

I've tested 0.25.0 and working with both cases e.g map url part encoded and unencoded... ~Can you confirm/infirm regression?~

Diagnostic: side effect due to another fix e.g

https://github.com/geopython/OWSLib/commit/b3acd1bf01bc3a5efaa5dedc38d9d45833eeee31#diff-a8ea809e6cb216f4c6c765a0d19397bc6d463c883e37e16d9f4a08db87f2cdedL18-L200

Tested that changing back owslib/util.py in latest code solve the issue but will establish again the issue fixed on the WFS side (the cause of the code breaking change)

sbrunner commented 1 year ago

Finally, when I see the full code this looks relay strange https://github.com/geopython/OWSLib/blob/018c7f83fb042a6390d65aaa683110743609f3e3/owslib/map/wms130.py#L304 The result is given to openURL, used here: https://github.com/geopython/OWSLib/blob/018c7f83fb042a6390d65aaa683110743609f3e3/owslib/util.py#LL200C33-L200C33 and given to requests here: https://github.com/geopython/OWSLib/blob/018c7f83fb042a6390d65aaa683110743609f3e3/owslib/util.py#L208 In requests documentation, we see that prams should be a dictionary https://tedboy.github.io/requests/generated/generated/requests.Request.html And there is a duplication Then this code https://github.com/geopython/OWSLib/blob/018c7f83fb042a6390d65aaa683110743609f3e3/owslib/map/wms130.py#L308 should be:

        u = openURL(base_url, request, method, timeout=timeout or self.timeout, auth=self.auth, headers=self.headers)

The same issue is in many place in the code...