openfaas / python-flask-template

HTTP and Flask-based OpenFaaS templates for Python 3
MIT License
85 stars 86 forks source link

request.get_data() is always empty because of flask/werkzeug bug with chunked transfer encoding #5

Open telackey opened 6 years ago

telackey commented 6 years ago

The python3-flask template (and likely the python2) is bitten by this Flask issue: https://github.com/pallets/flask/issues/2229

Flask / Werkzeug have a bug handling chunked transfer encoding--and perhaps any other time that Content-Length is not set--when combined with WSGI.

When the request is forwarded through the of-watchdog process, http.NewRequest is given a reference to the body io.ReadCloser. Since that is a stream, the outbound request will always use chunked transfer encoding and no Content-Length.

This could be avoided by changing of-watchdog to read the whole body into a buffer and then send it unchunked; but it would make more sense, I think, either to upgrade to a fixed version of Flask/Werkzeug (assuming there is one) or revert back to Flask's development server (app.run(...)) rather than using gevent's WSGIServer, as the built-in server does not seem to be affected.

The following examples are using the same of-watchdog and function code. The only difference is that the first one is using Flask with WSGIServer and the second is using the Flask built-in server.

WSGI server

$ curl -i -XPOST -d "rawdsddddddddddddddddddddddd" 'http://127.0.0.1:9999/'
HTTP/1.1 200 OK
Content-Length: 0
Content-Type: text/html; charset=utf-8
Date: Wed, 25 Jul 2018 23:33:36 GMT

Flask's server

$ curl -i -XPOST -d "rawdsddddddddddddddddddddddd" 'http://127.0.0.1:9999/'
HTTP/1.1 200 OK
Content-Length: 28
Content-Type: text/html; charset=utf-8
Date: Wed, 25 Jul 2018 23:31:29 GMT
Server: Werkzeug/0.14.1 Python/3.6.5

rawdsddddddddddddddddddddddd
ssullivan commented 5 years ago

I recently started to try and build some new functions and use this template and I've run into this same problem.

ssullivan commented 5 years ago

I was able to get this working by applying a suggestion from https://github.com/benoitc/gunicorn/issues/1733#issuecomment-377000612

@app.before_request
def fix_transfer_encoding():
    """
    Sets the "wsgi.input_terminated" environment flag, thus enabling
    Werkzeug to pass chunked requests as streams.  The gunicorn server
    should set this, but it's not yet been implemented.
    """

    transfer_encoding = request.headers.get("Transfer-Encoding", None)
    if transfer_encoding == u"chunked":
        request.environ["wsgi.input_terminated"] = True
alexellis commented 5 years ago

Plain Flask was both simpler to use and quicker to build, but is not said to be production-quality.

I'll take a look at this proposal.

alexellis commented 5 years ago

@ssullivan I'm talking with @LucasRoesler about this. Do we want to revert to "plain flask" which is much faster at build time and doesn't need the work-around?

ssullivan commented 5 years ago

At the moment I’m not familiar with the performance differences between plain flask and the current implementation. If it is more performant and doesn’t have the problem with chunked encoding it seems to make sense to switch to that.

LucasRoesler commented 5 years ago

I think for short term, a working template is pretty important. But in the long term it would be good to use wsgi.

Have we considered using conda instead of pip. Using conda and the conda-forge could greatly speed up the install of packages like wsgi

daMichaelB commented 4 years ago

I was able to get this working by applying a suggestion from benoitc/gunicorn#1733 (comment)

@app.before_request
def fix_transfer_encoding():
    """
    Sets the "wsgi.input_terminated" environment flag, thus enabling
    Werkzeug to pass chunked requests as streams.  The gunicorn server
    should set this, but it's not yet been implemented.
    """

    transfer_encoding = request.headers.get("Transfer-Encoding", None)
    if transfer_encoding == u"chunked":
        request.environ["wsgi.input_terminated"] = True

you saved me hours ! Thank you 👍

alexellis commented 4 years ago

Michael is that not in the OpenFaaS templates yet or are you wanting the hack for something else?

alexellis commented 4 years ago

We also have RAW_BODY available now as an environment variable which may also be useful here

daMichaelB commented 4 years ago

Hey @alexellis , thanks for reaching out! I came here from using the DockerHub Image: tiangolo/uwsgi-nginx-flask. I had the exact same problem there and struggled a lot to find a solution. Maybe there might be a better way to solve this issue, but it works perfectly.