New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wsgiserver.ChunkedRFile infinite loop #1486

Closed
mensonen opened this Issue Aug 29, 2016 · 2 comments

Comments

2 participants
@mensonen

mensonen commented Aug 29, 2016

When running a CherryPyWSGIServer and a request is made with a Transfer-Encoding: chunked header, attempting to read wsgi.input without specifying a size (default) causes an infinite loop.

E.g following sample service:

from cherrypy.wsgiserver import CherryPyWSGIServer

def read_post(env, start_response):
    data = env["wsgi.input"].read()
    start_response("200 OK", [("Content-Type", "text/plain")])
    return [data]

server = CherryPyWSGIServer(("localhost", 8080), read_post)
try:
    server.start()
except KeyboardInterrupt:
    server.stop()

And testing it with:

~ $ curl -v  --data "post data" --header "Transfer-Encoding: chunked" http://localhost:8080/
*   Trying ::1...
* Connected to localhost (::1) port 8080 (#0)
> POST / HTTP/1.1
> Host: localhost:8080
> User-Agent: curl/7.48.0
> Accept: */*
> Transfer-Encoding: chunked
> Content-Type: application/x-www-form-urlencoded
> 
> 9
* upload completely sent off: 16 out of 9 bytes
^C

Hangs indefinitely and server has to be killed.

This appears to be because ChunkedRFile.read loops over self.buffer, until it gets no data at all, but self.buffer is never reset after the loop iteration. readline suffers from the same bug.

@mensonen

This comment has been minimized.

mensonen commented Sep 6, 2016

As a workaround I have applied the following monkey patch at the top of our codebase and it appears to hold okay, our production servers have also been crash-free since the change:

from cherrypy.wsgiserver import ChunkedRFile
try:
    from cherrypy.wsgiserver.wsgiserver2 import EMPTY
except ImportError:
    from cherrypy.wsgiserver import EMPTY

def patched_chunked_read(self, size=None):
    data = EMPTY
    while True:
        if size and len(data) >= size:
            return data

        if not self.buffer:
            self._fetch()
            if not self.buffer:
                # EOF
                return data

        if size:
            remaining = size - len(data)
            data += self.buffer[:remaining]
            self.buffer = self.buffer[remaining:]
        else:
            data += self.buffer
            self.buffer = EMPTY

ChunkedRFile.read = patched_chunked_read

I suppose ChunkedRFile.readline could be fixed in a similar way.

@webknjaz

This comment has been minimized.

Member

webknjaz commented Sep 29, 2016

This looks related to #1131

netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this issue Jul 19, 2017

wiz
Updated py-cheroot to 5.7.0.
v5.7.0
======

- CI improvements:
  * Don't run tests during deploy stage
  * Use VM based build job env only for pyenv envs
  * Opt-in for beta trusty image @ Travis CI
  * Be verbose when running tests (show test names)
  * Show xfail/skip details during test run

- #34: Fix ``_handle_no_ssl`` error handler calls

- #21: Fix ``test_conn`` tests:
  * Improve setup_server def in HTTP connection tests
  * Fix HTTP streaming tests
  * Fix HTTP/1.1 pipelining test under Python 3
  * Fix ``test_readall_or_close`` test
  * Fix ``test_No_Message_Body``
  * Clarify ``test_598`` fail reason

- #36: Add GitHub templates for PR, issue && contributing

- #27: Default HTTP Server header to Cheroot version str

- Cleanup _compat functions from server module

v5.6.0
======

- Fix all PEP 257 related errors in all non-test modules.

  ``cheroot/test/*`` folder is only one left allowed to fail with this linter.

- #30: Optimize chunked body reader loop by returning empty data is the size is 0.

  Ref: cherrypy/cherrypy#1602

- Reset buffer if the body size is unknown

  Ref: cherrypy/cherrypy#1486

- Add missing size hint to SizeCheckWrapper

  Ref: cherrypy/cherrypy#1131

v5.5.2
======

- #32: Ignore "unknown error" and "https proxy request" SSL errors.

  Ref: sabnzbd/sabnzbd#820

  Ref: sabnzbd/sabnzbd#860

v5.5.1
======

- Make Appveyor list separate tests in corresponding tab.

- #29: Configure Travis CI build stages.

  Prioritize tests by stages.

  Move deploy stage to be run very last after all other stages finish.

- #31: Ignore "Protocol wrong type for socket" (EPROTOTYPE) @ OSX for non-blocking sockets.

  This was originally fixed for regular sockets in cherrypy/cherrypy#1392.

  Ref: https://forums.sabnzbd.org/viewtopic.php?f=2&t=22728&p=112251

v5.5.0
======

- #17 via #25: Instead of a read_headers function, cheroot now
  supplies a HeaderReader class to perform the same function.

  Any HTTPRequest object may override the header_reader attribute
  to customize the handling of incoming headers.

  The server module also presents a provisional implementation of
  a DropUnderscoreHeaderReader that will exclude any headers
  containing an underscore. It remains an exercise for the
  implementer to demonstrate how this functionality might be
  employed in a server such as CherryPy.

- #26: Configured TravisCI to run tests under OS X.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment