Issue #1134 new gives 500 for GET requests

Jim Paris
created an issue

This server: {{{


import cherrypy class Root(object): @cherrypy.expose def upload(self): body = return "got %d bytes, thanks\n" % len(body) cherrypy.config.update({ 'server.socket_host': '', 'server.socket_port': 1234, }) cherrypy.quickstart(Root()) }}} with a HTTP PUT works fine: {{{ $ curl -T <(echo hello) http://localhost:1234/upload got 6 bytes, thanks }}} but gives a 500 Internal Server Error with a GET: {{{ $ curl http://localhost:1234/upload <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" ""> <html> <head> <meta http-equiv="Content-Type" content="text/html; charset=utf-8"></meta> <title>500 Internal Server Error</title> <style type="text/css"> #powered_by { margin-top: 20px; border-top: 2px solid black; font-style: italic; }

#traceback {
    color: red;

</head> <body> <h2>500 Internal Server Error</h2> <p>The server encountered an unexpected condition which prevented it from fulfilling the request.</p> <pre id="traceback">Traceback (most recent call last): File "/usr/lib/python2.7/dist-packages/cherrypy/", line 656, in respond response.body = self.handler() File "/usr/lib/python2.7/dist-packages/cherrypy/lib/", line 188, in call self.body = self.oldhandler(*args, kwargs) File "/usr/lib/python2.7/dist-packages/cherrypy/", line 34, in __call__ return self.callable(*self.args, self.kwargs) File "", line 7, in upload body = File "/usr/lib/python2.7/dist-packages/cherrypy/", line 455, in read return, fp_out) TypeError: read() takes at most 2 arguments (3 given) </pre> <div id="powered_by"> <span>Powered by <a href="">CherryPy 3.2.2</a></span> </div> </body> </html> }}} The following patch seems to fix it: {{{ diff -r 2934ecf8075b cherrypy/ --- a/cherrypy/ Fri Mar 09 16:26:05 2012 -0500 +++ b/cherrypy/ Mon Mar 26 12:42:27 2012 -0400 @@ -452,7 +452,7 @@ doc="""A deprecated alias for :attr:content_type<cherrypy._cpreqbody.Entity.content_type>.""")

 def read(self, size=None, fp_out=None):
  • return, fp_out)
  • return

    def readline(self, size=None): return self.fp.readline(size) }}} Example after patch: {{{ $ curl -T <(echo hello) http://localhost:1234/upload got 6 bytes, thanks $ curl http://localhost:1234/upload got 0 bytes, thanks }}}

Comments (6)

  1. Jason R. Coombs

    We probably shouldn't remove the use of fp_out without first understanding why it was there in the first place. I removed the fp_out parameter from the `.read()` signature itself and ran the test suite, but there were no failures. I can find no documentation on the purpose of that parameter. I wonder under what circumstances fp_out has meaning. Neither Python file objects nor StringIO objects accept this parameter.

    I guess the next step is to run blame on the code and trace back the code change to determine why it was added.

  2. Robert Brewer

    The fp_out argument is there to support low-level code that reads the body in chunks, allowing each chunk to be written to a file rather than all chunks buffered in memory.

  3. Jim Paris reporter

    self.body is first initialized in as:

                        self.body = _cpreqbody.RequestBody(
                            self.rfile, self.headers, request_params=self.params)

    This sets self.body.fp = self.rfile, which for a GET request is something like cherrypy.wsgiserver.wsgiserver2.KnownLengthRFile -- this does not expect or handle a fp_out parameter.

    If (and only if) the method is in self.methods_with_bodies, respond() continues by calling self.body.process(). This function replaces self.body.fp with SizedReader, which _does_ handle fp_out.

  4. space one

    don't apply this patch! this can be used: body = '' if cherrypy.request.method in cherrypy.request.methods_with_bodies: body =

  5. Jim Paris reporter

    Do you say "don't apply this patch" because you think the patch is wrong, or just because you think it's unnecessary? I still argue that the code is wrong as written and that line can never execute succesfully -- at that point in time, there is no way that the function even takes a fp_out parameter.

  6. Log in to comment