Commits

Robert Brewer  committed 1d4055e

Fix for #622, #742, #736. The wsgiserver would respond without closing connection and without reading the full request. Fixed now.

  • Participants
  • Parent commits 6bd51d5

Comments (0)

Files changed (6)

File cherrypy/_cprequest.py

                     if self.process_request_body:
                         if self.method not in self.methods_with_bodies:
                             self.process_request_body = False
-                        
-                        if self.process_request_body:
-                            # Prepare the SizeCheckWrapper for the req body
-                            mbs = getattr(cherrypy.server,
-                                          "max_request_body_size", 0)
-                            if mbs > 0:
-                                self.rfile = http.SizeCheckWrapper(self.rfile, mbs)
                     
                     self.hooks.run('before_request_body')
                     if self.process_request_body:
             # any message-body that had a transfer-coding, and we expect
             # the HTTP server to have supplied a Content-Length header
             # which is valid for the decoded entity-body.
-            return
+            raise cherrypy.HTTPError(411)
         
         # FieldStorage only recognizes POST, so fake it.
         methenv = {'REQUEST_METHOD': "POST"}

File cherrypy/_cpwsgi.py

 
 class CPHTTPRequest(wsgiserver.HTTPRequest):
     
-    def parse_request(self):
-        mhs = _cherrypy.server.max_request_header_size
-        if mhs > 0:
-            self.rfile = _http.SizeCheckWrapper(self.rfile, mhs)
-        
-        try:
-            wsgiserver.HTTPRequest.parse_request(self)
-        except _http.MaxSizeExceeded:
-            self.simple_response("413 Request Entity Too Large")
-            _cherrypy.log(traceback=True)
-    
-    def decode_chunked(self):
-        """Decode the 'chunked' transfer coding."""
-        if isinstance(self.rfile, _http.SizeCheckWrapper):
-            self.rfile = self.rfile.rfile
-        mbs = _cherrypy.server.max_request_body_size
-        if mbs > 0:
-            self.rfile = _http.SizeCheckWrapper(self.rfile, mbs)
-        try:
-            return wsgiserver.HTTPRequest.decode_chunked(self)
-        except _http.MaxSizeExceeded:
-            self.simple_response("413 Request Entity Too Large")
-            _cherrypy.log(traceback=True)
-            return False
+    def __init__(self, sendall, environ, wsgi_app):
+        s = _cherrypy.server
+        self.max_request_header_size = s.max_request_header_size or 0
+        self.max_request_body_size = s.max_request_body_size or 0
+        wsgiserver.HTTPRequest.__init__(self, sendall, environ, wsgi_app)
 
 
 class CPHTTPConnection(wsgiserver.HTTPConnection):
 
 
 class CPWSGIServer(wsgiserver.CherryPyWSGIServer):
-    
     """Wrapper for wsgiserver.CherryPyWSGIServer.
     
     wsgiserver has been designed to not reference CherryPy in any way,
     so that it can be used in other frameworks and applications. Therefore,
     we wrap it here, so we can set our own mount points from cherrypy.tree.
-    
     """
     
     ConnectionClass = CPHTTPConnection

File cherrypy/lib/http.py

         return header_list
 
 
-class MaxSizeExceeded(Exception):
-    pass
-
-class SizeCheckWrapper(object):
-    """Wraps a file-like object, raising MaxSizeExceeded if too large."""
-    
-    def __init__(self, rfile, maxlen):
-        self.rfile = rfile
-        self.maxlen = maxlen
-        self.bytes_read = 0
-    
-    def _check_length(self):
-        if self.maxlen and self.bytes_read > self.maxlen:
-            raise MaxSizeExceeded()
-    
-    def read(self, size = None):
-        data = self.rfile.read(size)
-        self.bytes_read += len(data)
-        self._check_length()
-        return data
-    
-    def readline(self, size = None):
-        if size is not None:
-            data = self.rfile.readline(size)
-            self.bytes_read += len(data)
-            self._check_length()
-            return data
-        
-        # User didn't specify a size ...
-        # We read the line in chunks to make sure it's not a 100MB line !
-        res = []
-        while True:
-            data = self.rfile.readline(256)
-            self.bytes_read += len(data)
-            self._check_length()
-            res.append(data)
-            # See http://www.cherrypy.org/ticket/421
-            if len(data) < 256 or data[-1:] == "\n":
-                return ''.join(res)
-    
-    def readlines(self, sizehint = 0):
-        # Shamelessly stolen from StringIO
-        total = 0
-        lines = []
-        line = self.readline()
-        while line:
-            lines.append(line)
-            total += len(line)
-            if 0 < sizehint <= total:
-                break
-            line = self.readline()
-        return lines
-    
-    def close(self):
-        self.rfile.close()
-    
-    def __iter__(self):
-        return self
-    
-    def next(self):
-        data = self.rfile.next()
-        self.bytes_read += len(data)
-        self._check_length()
-        return data
+from cherrypy.wsgiserver import SizeCheckWrapper, MaxSizeExceeded
 
 
 class Host(object):

File cherrypy/test/test_conn.py

+"""Tests for TCP connection handling, including proper and timely close."""
+
 from cherrypy.test import test
 test.prefer_parent_path()
 
 pov = 'pPeErRsSiIsStTeEnNcCeE oOfF vViIsSiIoOnN'
 
 def setup_server():
+    
+    def raise500():
+        raise cherrypy.HTTPError(500)
+    
     class Root:
         
         def index(self):
         stream.exposed = True
         stream._cp_config = {'response.stream': True}
         
+        def error(self, code=500):
+            raise cherrypy.HTTPError(code)
+        error.exposed = True
+        
         def upload(self):
             return ("thanks for '%s' (%s)" %
                     (cherrypy.request.body.read(),
             cherrypy.response.status = response_code
             return "Code = %s" % response_code
         custom.exposed = True
+        
+        def err_before_read(self):
+            return "ok"
+        err_before_read.exposed = True
+        err_before_read._cp_config = {'hooks.on_start_resource': raise500}
     
     cherrypy.tree.mount(Root())
     cherrypy.config.update({
-        'server.max_request_body_size': 100,
+        'server.max_request_body_size': 1001,
         'environment': 'test_suite',
         })
 
         self.assertStatus(200)
         self.assertBody("thanks for 'I am a small file' (text/plain)")
     
+    def test_readall_or_close(self):
+        if cherrypy.server.protocol_version != "HTTP/1.1":
+            self.PROTOCOL = "HTTP/1.0"
+        else:
+            self.PROTOCOL = "HTTP/1.1"
+        
+        if self.scheme == "https":
+            self.HTTP_CONN = httplib.HTTPSConnection
+        else:
+            self.HTTP_CONN = httplib.HTTPConnection
+        
+        self.persistent = True
+        conn = self.HTTP_CONN
+        
+        # Get a POST page with an error
+        conn.putrequest("POST", "/err_before_read", skip_host=True)
+        conn.putheader("Host", self.HOST)
+        conn.putheader("Content-Type", "text/plain")
+        conn.putheader("Content-Length", "1000")
+        conn.putheader("Expect", "100-continue")
+        conn.endheaders()
+        response = conn.response_class(conn.sock, method="POST")
+        
+        # ...assert and then skip the 100 response
+        version, status, reason = response._read_status()
+        self.assertEqual(status, 100)
+        while True:
+            skip = response.fp.readline().strip()
+            if not skip:
+                break
+        
+        # ...send the body
+        conn.send("x" * 1000)
+        
+        # ...get the final response
+        response.begin()
+        self.status, self.headers, self.body = webtest.shb(response)
+        self.assertStatus(500)
+        
+        # Now try a working page with an Expect header...
+        conn._output('POST /upload HTTP/1.1')
+        conn._output("Host: %s" % self.HOST)
+        conn._output("Content-Type: text/plain")
+        conn._output("Content-Length: 17")
+        conn._output("Expect: 100-continue")
+        conn._send_output()
+        response = conn.response_class(conn.sock, method="POST")
+        
+        # ...assert and then skip the 100 response
+        version, status, reason = response._read_status()
+        self.assertEqual(status, 100)
+        while True:
+            skip = response.fp.readline().strip()
+            if not skip:
+                break
+        
+        # ...send the body
+        conn.send("I am a small file")
+        
+        # ...get the final response
+        response.begin()
+        self.status, self.headers, self.body = webtest.shb(response)
+        self.assertStatus(200)
+        self.assertBody("thanks for 'I am a small file' (text/plain)")
+    
     def test_No_Message_Body(self):
         if cherrypy.server.protocol_version != "HTTP/1.1":
             print "skipped ",
         
         # Try a chunked request that exceeds server.max_request_body_size.
         # Note that the delimiters and trailer are included.
-        body = "5f\r\n" + ("x" * 95) + "\r\n0\r\n\r\n"
+        body = "3e3\r\n" + ("x" * 995) + "\r\n0\r\n\r\n"
         conn.putrequest("POST", "/upload", skip_host=True)
         conn.putheader("Host", self.HOST)
         conn.putheader("Transfer-Encoding", "chunked")
         conn.putheader("Content-Type", "text/plain")
+        # Chunked requests don't need a content-length
 ##        conn.putheader("Content-Length", len(body))
         conn.endheaders()
         conn.send(body)

File cherrypy/test/test_core.py

         self.getPage("/headerelements/get_elements?headername=Expect", [e])
         self.assertBody('100-continue')
         
-        self.getPage("/expect/expectation_failed", [('Content-Length', '200'), e])
+        self.getPage("/expect/expectation_failed", [e])
         self.assertStatus(417)
     
     def testHeaderElements(self):

File cherrypy/wsgiserver/__init__.py

         return ['']
 
 
+class MaxSizeExceeded(Exception):
+    pass
+
+class SizeCheckWrapper(object):
+    """Wraps a file-like object, raising MaxSizeExceeded if too large."""
+    
+    def __init__(self, rfile, maxlen):
+        self.rfile = rfile
+        self.maxlen = maxlen
+        self.bytes_read = 0
+    
+    def _check_length(self):
+        if self.maxlen and self.bytes_read > self.maxlen:
+            raise MaxSizeExceeded()
+    
+    def read(self, size=None):
+        data = self.rfile.read(size)
+        self.bytes_read += len(data)
+        self._check_length()
+        return data
+    
+    def readline(self, size=None):
+        if size is not None:
+            data = self.rfile.readline(size)
+            self.bytes_read += len(data)
+            self._check_length()
+            return data
+        
+        # User didn't specify a size ...
+        # We read the line in chunks to make sure it's not a 100MB line !
+        res = []
+        while True:
+            data = self.rfile.readline(256)
+            self.bytes_read += len(data)
+            self._check_length()
+            res.append(data)
+            # See http://www.cherrypy.org/ticket/421
+            if len(data) < 256 or data[-1:] == "\n":
+                return ''.join(res)
+    
+    def readlines(self, sizehint=0):
+        # Shamelessly stolen from StringIO
+        total = 0
+        lines = []
+        line = self.readline()
+        while line:
+            lines.append(line)
+            total += len(line)
+            if 0 < sizehint <= total:
+                break
+            line = self.readline()
+        return lines
+    
+    def close(self):
+        self.rfile.close()
+    
+    def __iter__(self):
+        return self
+    
+    def next(self):
+        data = self.rfile.next()
+        self.bytes_read += len(data)
+        self._check_length()
+        return data
+
+
 class HTTPRequest(object):
     """An HTTP Request (and response).
     
         send_headers.
     """
     
+    max_request_header_size = 0
+    max_request_body_size = 0
+    
     def __init__(self, sendall, environ, wsgi_app):
         self.rfile = environ['wsgi.input']
         self.sendall = sendall
     
     def parse_request(self):
         """Parse the next HTTP request start-line and message-headers."""
+        self.rfile.maxlen = self.max_request_header_size
+        self.rfile.bytes_read = 0
+        
+        try:
+            self._parse_request()
+        except MaxSizeExceeded:
+            self.simple_response("413 Request Entity Too Large")
+            return
+    
+    def _parse_request(self):
         # HTTP/1.1 connections are persistent by default. If a client
         # requests a page, then idles (leaves the connection open),
         # then rfile.readline() will raise socket.error("timed out").
             self.simple_response("400 Bad Request", repr(ex.args))
             return
         
+        # Set AUTH_TYPE, REMOTE_USER
         creds = environ.get("HTTP_AUTHORIZATION", "").split(" ", 1)
         environ["AUTH_TYPE"] = creds[0]
         if creds[0].lower() == 'basic':
             if te:
                 te = [x.strip().lower() for x in te.split(",") if x.strip()]
         
-        read_chunked = False
+        self.chunked_read = False
         
         if te:
             for enc in te:
                 if enc == "chunked":
-                    read_chunked = True
+                    self.chunked_read = True
                 else:
                     # Note that, even if we see "chunked", we must reject
                     # if there is an extension we don't recognize.
                     self.close_connection = True
                     return
         
-        if read_chunked:
-            if not self.decode_chunked():
-                return
-        
         # From PEP 333:
         # "Servers and gateways that implement HTTP 1.1 must provide
         # transparent support for HTTP 1.1's "expect/continue" mechanism.
     
     def respond(self):
         """Call the appropriate WSGI app and write its iterable output."""
+        # Set rfile.maxlen to ensure we don't read past Content-Length.
+        # This will also be used to read the entire request body if errors
+        # are raised before the app can read the body.
+        if self.chunked_read:
+            # If chunked, Content-Length will be 0.
+            self.rfile.maxlen = self.max_request_body_size
+        else:
+            cl = int(self.environ.get("CONTENT_LENGTH", 0))
+            self.rfile.maxlen = min(cl, self.max_request_body_size)
+        self.rfile.bytes_read = 0
+        
+        try:
+            self._respond()
+        except MaxSizeExceeded:
+            self.simple_response("413 Request Entity Too Large")
+            return
+    
+    def _respond(self):
+        if self.chunked_read:
+            if not self.decode_chunked():
+                self.close_connection = True
+                return
+        
         response = self.wsgi_app(self.environ, self.start_response)
         try:
             for chunk in response:
         finally:
             if hasattr(response, "close"):
                 response.close()
+        
         if (self.ready and not self.sent_headers):
             self.sent_headers = True
             self.send_headers()
                 if not self.close_connection:
                     self.outheaders.append(("Connection", "Keep-Alive"))
         
+        if (not self.close_connection) and (not self.chunked_read):
+            # Read any remaining request body data on the socket.
+            # "If an origin server receives a request that does not include an
+            # Expect request-header field with the "100-continue" expectation,
+            # the request includes a request body, and the server responds
+            # with a final status code before reading the entire request body
+            # from the transport connection, then the server SHOULD NOT close
+            # the transport connection until it has read the entire request,
+            # or until the client closes the connection. Otherwise, the client
+            # might not reliably receive the response message. However, this
+            # requirement is not be construed as preventing a server from
+            # defending itself against denial-of-service attacks, or from
+            # badly broken client implementations."
+            size = self.rfile.maxlen - self.rfile.bytes_read
+            if size > 0:
+                self.rfile.read(size)
+        
         if "date" not in hkeys:
             self.outheaders.append(("Date", rfc822.formatdate()))
         
             self.rfile = sock.makefile("rb", self.rbufsize)
             self.sendall = sock.sendall
         
-        self.environ["wsgi.input"] = self.rfile
+        # Wrap wsgi.input but not HTTPConnection.rfile itself.
+        # We're also not setting maxlen yet; we'll do that separately
+        # for headers and body for each iteration of self.communicate
+        # (if maxlen is 0 the wrapper doesn't check length).
+        self.environ["wsgi.input"] = SizeCheckWrapper(self.rfile, 0)
     
     def communicate(self):
         """Read each request and respond appropriately."""
                 req = None
                 req = self.RequestHandlerClass(self.sendall, self.environ,
                                                self.wsgi_app)
+                
                 # This order of operations should guarantee correct pipelining.
                 req.parse_request()
                 if not req.ready:
                     return
+                
                 req.respond()
                 if req.close_connection:
                     return
+        
         except socket.error, e:
             errnum = e.args[0]
             if errnum not in socket_errors_to_ignore: