geopython / pywps

PyWPS is an implementation of the Web Processing Service standard from the Open Geospatial Consortium. PyWPS is written in Python.
https://pywps.org
MIT License
177 stars 117 forks source link

Base64 input issue when using the file handler #241

Closed elemoine closed 7 years ago

elemoine commented 7 years ago

Description

I get an exception when using the file handler on a complex input whose format is a base64-encoded image.

The traceback:

Mar 20 14:44:30 jessie uwsgi[8720]: File "/opt/wpsapp/venv/lib/python3.4/site-packages/pywps/app/Process.py", line 222, in _run_process                                                                            
Mar 20 14:44:30 jessie uwsgi[8720]: wps_response = self.handler(wps_request, wps_response)                                                                                                                         
Mar 20 14:44:30 jessie uwsgi[8720]: File "/home/love/love.git/wpsapp/wpsapp/processes/testcase.py", line 26, in _handler                                                                                           
Mar 20 14:44:30 jessie uwsgi[8720]: i1 = request.inputs['i'][0].file  # NOQA                                                                                                                                       
Mar 20 14:44:30 jessie uwsgi[8720]: File "/opt/wpsapp/venv/lib/python3.4/site-packages/pywps/inout/basic.py", line 151, in get_file                                                                                
Mar 20 14:44:30 jessie uwsgi[8720]: stream_file.write(self.source)                                                                                                                                                 
Mar 20 14:44:30 jessie uwsgi[8720]: TypeError: must be str, not bytes                                                                                                                                              
Mar 20 14:44:30 jessie uwsgi[8720]: Retrieving file and line number where exception occurred                                                                                                                       
Mar 20 14:44:30 jessie uwsgi[8720]: Process error: testcase.py._handler Line 26 must be str, not bytes

Environment

Steps to reproduce

Here is a test process that can be used to reproduce the issue:

from pywps import Process, ComplexInput, Format

class TestCase(Process):

    def __init__(self):

        fmt = Format('image/png', encoding='base64')
        inputs = [
            ComplexInput('i', 'Image', supported_formats=[fmt])
        ]
        outputs = [
        ]
        super().__init__(
            self._handler,
            identifier='testcase',
            title='TestCase',
            inputs=inputs,
            outputs=outputs
        )

    def _handler(self, request, response):
        i = request.inputs['i'][0].file  # NOQA
        return response

And a corresponding Execute request:

<?xml version='1.0' encoding='utf-8'?>
<wps:Execute Service="WPS" version="1.0.0" xmlns:ows="http://www.opengis.net/ows/1.1" xmlns:wps="http://www.opengis.net/wps/1.0.0"><ows:Identifier>testcase</ows:Identifier><wps:DataInputs><wps:Input><ows:Identifier>i</ows:Identifier><wps:Data><wps:ComplexData encoding="base64" mimeType="image/png">iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAAAAAA6fptVAAAACklEQVR4nGNiAAAABgADNjd8qAAAAABJRU5ErkJggg==</wps:ComplexData></wps:Data></wps:Input></wps:DataInputs></wps:Execute>

And this is the command I use to post the request:

$ curl -X POST -d @execute.xml -H 'Content-Type: text/xml;charset=UTF-8' -D /tmp/headers.txt 'localhost:8080/wps'
<!-- PyWPS 4.0.0 -->
<wps:ExecuteResponse xmlns:gml="http://www.opengis.net/gml" xmlns:ows="http://www.opengis.net/ows/1.1" xmlns:wps="http://www.opengis.net/wps/1.0.0" xmlns:xlink="http://www.w3.org/1999/xlink" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://www.opengis.net/wps/1.0.0 http://schemas.opengis.net/wps/1.0.0/wpsExecute_response.xsd" service="WPS" version="1.0.0" xml:lang="en-US" serviceInstance="http://localhost/wps?service=WPS&amp;request=GetCapabilities">
  <wps:Process wps:processVersion="None">
    <ows:Identifier>testcase</ows:Identifier>
    <ows:Title>TestCase</ows:Title>
  </wps:Process>
  <wps:Status creationTime="2017-03-20T14:55:02Z">
    <wps:ProcessFailed>
      <wps:ExceptionReport>
        <ows:Exception locater="None" exceptionCode="NoApplicableCode">
          <ows:ExceptionText>Process error: testcase.py._handler Line 26 must be str, not bytes</ows:ExceptionText>
        </ows:Exception>
      </wps:ExceptionReport>
    </wps:ProcessFailed>
  </wps:Status>
</wps:ExecuteResponse>

Additional information

The error occurs when writing the base64-decoded data into a file. For writing bytes the "binary" flag should be used when opening the file. It works if I use open(stream_file_name, 'wb').

elemoine commented 7 years ago

The problem is Python3-specific. Here is a simple fix: https://github.com/geopython/pywps/compare/develop...elemoine:base64. I can create a PR if you think this is the correct way to fix it. Thanks.

jachym commented 7 years ago

Works for me, can you PR please?

elemoine commented 7 years ago

Works for me, can you PR please?

Yes, I will.

elemoine commented 7 years ago

@jachym the commit that you looked at solves the problem for inputs. But I've actually experienced the same problem on output. To solve the problem on the output side I had to use hasattr(self, 'data_format') again. See the new commit that I've added to my base64 branch. I can create a PR but I am not sure you will like the use of hasattr here again. I don't see any other way right now.

jachym commented 7 years ago

Hi @elemoine

well, I did not study computer science and all I know about programming theory is just taken from the ground on my way through life. Therefore please do not take my statements too ultimate - I certainly do not have strong opinion on most of programming patterns and anti patterns.

I'm not sure, whether checking from some class, whether there might be some attribute (introduced in child class) is good approach in OOP. I've prepared pull request for your branch https://github.com/elemoine/pywps/pull/2 which tries to choose little bit different approach: instead of checking for possible attribute in parent class (in this case IOHandler), I've introduced new method, which can be overwritten in child classes (and so it's done in ComplexBase class - the method returns openmode based on your logic.

Does this work for you?

elemoine commented 7 years ago

I've prepared pull request for your branch elemoine#2 which tries to choose little bit different approach: instead of checking for possible attribute in parent class (in this case IOHandler), I've introduced new method, which can be overwritten in child classes (and so it's done in ComplexBase class - the method returns openmode based on your logic.

The issue is that BasicComplex, where you override get_open_mode, is actually not a child class of IOHandler. Instead, both are parent classes of ComplexInput and ComplexOutput. The override may work thanks to the order of classes in the inheritance definition, but this is fragile in my opinion.

So for now I think the simple approach used in my base64 branch is better. I am going to create a PR based on that branch.