Changeset 1786

Timestamp:

10/27/07 19:55:13

Author:

fumanchu

Message:

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

Files:

• trunk/cherrypy/_cprequest.py (modified) (2 diffs)
• trunk/cherrypy/_cpwsgi.py (modified) (3 diffs)
• trunk/cherrypy/lib/http.py (modified) (1 diff)
• trunk/cherrypy/test/test_conn.py (modified) (6 diffs)
• trunk/cherrypy/test/test_core.py (modified) (1 diff)
• trunk/cherrypy/wsgiserver/__init__.py (modified) (11 diffs)

trunk/cherrypy/_cprequest.py

@@ -555 +555 @@
-                        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')
@@ -661 +654 @@
-            # the HTTP server to have supplied a Content-Length header
-            # which is valid for the decoded entity-body.
-eturn
+aise cherrypy.HTTPError(411)
-        
-        # FieldStorage only recognizes POST, so fake it.

trunk/cherrypy/_cpwsgi.py

@@ -333 +333 @@
-class CPHTTPRequest(wsgiserver.HTTPRequest):
-    
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
+
+
+
+
+
-
-
@@ -365 +346 @@
-
-class CPWSGIServer(wsgiserver.CherryPyWSGIServer):
-    
-    """Wrapper for wsgiserver.CherryPyWSGIServer.
-    
@@ -371 +351 @@
-    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.
-    
-    """
-    

trunk/cherrypy/lib/http.py

@@ -377 +377 @@
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
+
-
-

trunk/cherrypy/test/test_conn.py

@@ +1 @@
+"""Tests for TCP connection handling, including proper and timely close."""
+
-from cherrypy.test import test
-test.prefer_parent_path()
@@ -16 +18 @@
-
-def setup_server():
+    
+    def raise500():
+        raise cherrypy.HTTPError(500)
+    
-    class Root:
-        
@@ -41 +47 @@
-        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)" %
@@ -51 +61 @@
-            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({
-
+1
-        'environment': 'test_suite',
-        })
@@ -345 +360 @@
-        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":
@@ -413 +493 @@
-        # Try a chunked request that exceeds server.max_request_body_size.
-        # Note that the delimiters and trailer are included.
-5f\r\n" + ("x" * 
+3e3\r\n" + ("x" * 9
-        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()

trunk/cherrypy/test/test_core.py

@@ -796 +796 @@
-        self.assertBody('100-continue')
-        
-('Content-Length', '200'), 
+
-        self.assertStatus(417)
-    

trunk/cherrypy/wsgiserver/__init__.py

@@ -120 +120 @@
-
-
+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).
@@ -155 +221 @@
-    """
-    
+    max_request_header_size = 0
+    max_request_body_size = 0
+    
-    def __init__(self, sendall, environ, wsgi_app):
-        self.rfile = environ['wsgi.input']
@@ -171 +240 @@
-    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),
@@ -261 +340 @@
-            return
-        
+        # Set AUTH_TYPE, REMOTE_USER
-        creds = environ.get("HTTP_AUTHORIZATION", "").split(" ", 1)
-        environ["AUTH_TYPE"] = creds[0]
@@ -283 +363 @@
-                te = [x.strip().lower() for x in te.split(",") if x.strip()]
-        
-read_chunke
+self.chunked_rea
-        
-        if te:
-            for enc in te:
-                if enc == "chunked":
-read_chunke
+self.chunked_rea
-                else:
-                    # Note that, even if we see "chunked", we must reject
@@ -295 +375 @@
-                    self.close_connection = True
-                    return
-        
-        if read_chunked:
-            if not self.decode_chunked():
-                return
-        
-        # From PEP 333:
@@ -386 +462 @@
-    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:
@@ -400 +499 @@
-            if hasattr(response, "close"):
-                response.close()
+        
-        if (self.ready and not self.sent_headers):
-            self.sent_headers = True
@@ -488 +588 @@
-                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:
@@ -613 +730 @@
-            self.sendall = sock.sendall
-        
-
+
+
+
+
+
-    
-    def communicate(self):
@@ -625 +746 @@
-                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]