Robert Brewer avatar Robert Brewer committed af59058

Fix for #940 (408 timeout fix for ticket 847 needs backport to cherrpy 3.1 branch).

Comments (0)

Files changed (3)

cherrypy/test/test.py

         'test_proxy',
         'test_caching',
         'test_config',
-##        'test_conn',
+        'test_conn',
         'test_core',
         'test_tools',
         'test_encoding',

cherrypy/test/test_conn.py

         'server.max_request_body_size': 1001,
         'environment': 'test_suite',
         })
+    cherrypy.server.httpserver.timeout = timeout
 
 
 from cherrypy.test import helper
                 self.assertRaises(httplib.NotConnected, self.getPage, "/")
     
     def test_HTTP11_Timeout(self):
+        # If we timeout without sending any data,
+        # the server will close the conn with a 408.
         if cherrypy.server.protocol_version != "HTTP/1.1":
-            print "skipped ",
-            return
+            return self.skip()
+        
+        self.PROTOCOL = "HTTP/1.1"
+        
+        # Connect but send nothing.
+        self.persistent = True
+        conn = self.HTTP_CONN
+        conn.auto_open = False
+        conn.connect()
+        
+        # Wait for our socket timeout
+        time.sleep(timeout * 2)
+        
+        # The request should have returned 408 already.
+        response = conn.response_class(conn.sock, method="GET")
+        response.begin()
+        self.assertEqual(response.status, 408)
+        conn.close()
+        
+        # Connect but send half the headers only.
+        self.persistent = True
+        conn = self.HTTP_CONN
+        conn.auto_open = False
+        conn.connect()
+        conn.send('GET /hello HTTP/1.1')
+        conn.send(("Host: %s" % self.HOST).encode('ascii'))
+        
+        # Wait for our socket timeout
+        time.sleep(timeout * 2)
+        
+        # The conn should have already sent 408.
+        response = conn.response_class(conn.sock, method="GET")
+        response.begin()
+        self.assertEqual(response.status, 408)
+        conn.close()
+    
+    def test_HTTP11_Timeout_after_request(self):
+        # If we timeout after at least one request has succeeded,
+        # the server will close the conn without 408.
+        if cherrypy.server.protocol_version != "HTTP/1.1":
+            return self.skip()
         
         self.PROTOCOL = "HTTP/1.1"
         
         # Wait for our socket timeout
         time.sleep(timeout * 2)
         response = conn.response_class(conn.sock, method="GET")
-        response.begin()
-        self.assertEqual(response.status, 408)
+        try:
+            response.begin()
+        except:
+            if not isinstance(sys.exc_info()[1],
+                              (socket.error, httplib.BadStatusLine)):
+                self.fail("Writing to timed out socket didn't fail"
+                          " as it should have: %s" % sys.exc_info()[1])
+        else:
+            self.fail("Writing to timed out socket didn't fail"
+                      " as it should have: %s" %
+                      response.read())
+        
         conn.close()
         
         # Retry the request on a new connection, which should work
         conn.putrequest("POST", "/upload", skip_host=True)
         conn.putheader("Host", self.HOST)
         conn.putheader("Content-Type", "text/plain")
-        conn.putheader("Content-Length", 9999)
+        conn.putheader("Content-Length", "9999")
         conn.endheaders()
         response = conn.getresponse()
         self.status, self.headers, self.body = webtest.shb(response)

cherrypy/wsgiserver/__init__.py

         self.wsgi_app = wsgi_app
         
         self.ready = False
+        self.started_request = False
         self.started_response = False
         self.status = ""
         self.outheaders = []
         # (although your TCP stack might suffer for it: cf Apache's history
         # with FIN_WAIT_2).
         request_line = self.rfile.readline()
+        # Set started_request to True so communicate() knows to send 408
+        # from here on out.
+        self.started_request = True
         if not request_line:
             # Force self.ready = False so the connection will close.
             self.ready = False
                 self.ready = False
                 return
         
+        if not request_line.endswith('\r\n'):
+            self.simple_response(400, "HTTP requires CRLF terminators")
+            return
+        
         environ = self.environ
         
         try:
             self.simple_response(400, "Malformed Request-Line")
             return
         
+        environ["REQUEST_URI"] = path
         environ["REQUEST_METHOD"] = method
         
         # path may be an abs_path (including "http://host.domain.tld");
         # But note that "...a URI must be separated into its components
         # before the escaped characters within those components can be
         # safely decoded." http://www.ietf.org/rfc/rfc2396.txt, sec 2.4.2
-        atoms = [unquote(x) for x in quoted_slash.split(path)]
+        try:
+            atoms = [unquote(x) for x in quoted_slash.split(path)]
+        except ValueError, ex:
+            self.simple_response("400 Bad Request", ex.args[0])
+            return
         path = "%2F".join(atoms)
         environ["PATH_INFO"] = path
         
         # Note that, like wsgiref and most other WSGI servers,
-        # we unquote the path but not the query string.
+        # we "% HEX HEX"-unquote the path but not the query string.
         environ["QUERY_STRING"] = qs
         
         # Compare request and server HTTP protocol versions, in case our
             if line == '\r\n':
                 # Normal end of headers
                 break
+            if not line.endswith('\r\n'):
+                raise ValueError("HTTP requires CRLF terminators")
             
             if line[0] in ' \t':
                 # It's a continuation line.
         data = StringIO.StringIO()
         while True:
             line = self.rfile.readline().strip().split(";", 1)
-            chunk_size = int(line.pop(0), 16)
+            try:
+                chunk_size = line.pop(0)
+                chunk_size = int(chunk_size, 16)
+            except ValueError:
+                self.simple_response("400 Bad Request",
+                     "Bad chunked transfer size: " + repr(chunk_size))
+                return
             if chunk_size <= 0:
                 break
 ##            if line: chunk_extension = line[0]
                 # a NON-EMPTY string, or upon the application's first
                 # invocation of the write() callable." (PEP 333)
                 if chunk:
+                    if isinstance(chunk, unicode):
+                        chunk = chunk.encode('ISO-8859-1')
                     self.write(chunk)
         finally:
             if hasattr(response, "close"):
         
         buf.append("\r\n")
         if msg:
+            if isinstance(msg, unicode):
+                msg = msg.encode("ISO-8859-1")
             buf.append(msg)
         
         try:
     
     def communicate(self):
         """Read each request and respond appropriately."""
+        request_seen = False
         try:
             while True:
                 # (re)set req to None so that if something goes wrong in
                 # This order of operations should guarantee correct pipelining.
                 req.parse_request()
                 if not req.ready:
+                    # Something went wrong in the parsing (and the server has
+                    # probably already made a simple_response). Return and
+                    # let the conn close.
                     return
                 
+                request_seen = True
                 req.respond()
                 if req.close_connection:
                     return
-        
         except socket.error, e:
             errnum = e.args[0]
             if errnum == 'timed out':
-                if req and not req.sent_headers:
-                    req.simple_response("408 Request Timeout")
+                # Don't error if we're between requests; only error
+                # if 1) no request has been started at all, or 2) we're
+                # in the middle of a request.
+                # See http://www.cherrypy.org/ticket/853
+                if (not request_seen) or (req and req.started_request):
+                    # Don't bother writing the 408 if the response
+                    # has already started being written.
+                    if req and not req.sent_headers:
+                        try:
+                            req.simple_response("408 Request Timeout")
+                        except FatalSSLAlert:
+                            # Close the connection.
+                            return
             elif errnum not in socket_errors_to_ignore:
                 if req and not req.sent_headers:
-                    req.simple_response("500 Internal Server Error",
-                                        format_exc())
+                    try:
+                        req.simple_response("500 Internal Server Error",
+                                            format_exc())
+                    except FatalSSLAlert:
+                        # Close the connection.
+                        return
             return
         except (KeyboardInterrupt, SystemExit):
             raise
-        except FatalSSLAlert, e:
+        except FatalSSLAlert:
             # Close the connection.
             return
         except NoSSLError:
                     "The client sent a plain HTTP request, but "
                     "this server only speaks HTTPS on this port.")
                 self.linger = True
-        except Exception, e:
+        except Exception:
             if req and not req.sent_headers:
-                req.simple_response("500 Internal Server Error", format_exc())
+                try:
+                    req.simple_response("500 Internal Server Error", format_exc())
+                except FatalSSLAlert:
+                    # Close the connection.
+                    return
     
     linger = False
     
                 continue
             break
         if not self.socket:
-            raise socket.error, msg
+            raise socket.error(msg)
         
         # Timeout so KeyboardInterrupt can be caught on Win32
         self.socket.settimeout(1)
Tip: Filter by directory path e.g. /media app.js to search for public/media/app.js.
Tip: Use camelCasing e.g. ProjME to search for ProjectModifiedEvent.java.
Tip: Filter by extension type e.g. /repo .js to search for all .js files in the /repo directory.
Tip: Separate your search with spaces e.g. /ssh pom.xml to search for src/ssh/pom.xml.
Tip: Use ↑ and ↓ arrow keys to navigate and return to view the file.
Tip: You can also navigate files with Ctrl+j (next) and Ctrl+k (previous) and view the file with Ctrl+o.
Tip: You can also navigate files with Alt+j (next) and Alt+k (previous) and view the file with Alt+o.