Robert Brewer avatar Robert Brewer committed b09e904

Fix for #820 (start_response with exc_info raises exception even if no output was sent yet). This also fixes related issues in cpwsgi and error output in general.

Comments (0)

Files changed (5)

cherrypy/_cpwsgi.py

         self.setapp()
     
     def setapp(self):
-        # Stage 1: by whatever means necessary, obtain a status, header
-        #          set and body, with an optional exception object.
         try:
             self.request = self.get_request()
             s, h, b = self.get_response()
-            exc = None
+            self.iter_response = iter(b)
+            self.start_response(s, h)
         except self.throws:
             self.close()
             raise
             if not getattr(self.request, "show_tracebacks", True):
                 tb = ""
             s, h, b = _cperror.bare_error(tb)
-            exc = _sys.exc_info()
-        
-        self.iter_response = iter(b)
-        
-        # Stage 2: now that we have a status, header set, and body,
-        #          finish the WSGI conversation by calling start_response.
-        try:
-            self.start_response(s, h, exc)
-        except self.throws:
-            self.close()
-            raise
-        except:
-            if getattr(self.request, "throw_errors", False):
+            self.iter_response = iter(b)
+            
+            try:
+                self.start_response(s, h, _sys.exc_info())
+            except:
+                # "The application must not trap any exceptions raised by
+                # start_response, if it called start_response with exc_info.
+                # Instead, it should allow such exceptions to propagate
+                # back to the server or gateway."
+                # But we still log and call close() to clean up ourselves.
+                _cherrypy.log(traceback=True, severity=40)
                 self.close()
                 raise
-            
-            _cherrypy.log(traceback=True, severity=40)
-            self.close()
-            
-            # CherryPy test suite expects bare_error body to be output,
-            # so don't call start_response (which, according to PEP 333,
-            # may raise its own error at that point).
-            s, h, b = _cperror.bare_error()
-            self.iter_response = iter(b)
     
     def iredirect(self, path, query_string):
         """Doctor self.environ and perform an internal redirect.
                 self.close()
                 raise
             
-            _cherrypy.log(traceback=True, severity=40)
+            tb = _cperror.format_exc()
+            _cherrypy.log(tb, severity=40)
+            if not getattr(self.request, "show_tracebacks", True):
+                tb = ""
+            s, h, b = _cperror.bare_error(tb)
+            # Empty our iterable (so future calls raise StopIteration)
+            self.iter_response = iter([])
             
-            # CherryPy test suite expects bare_error body to be output,
-            # so don't call start_response (which, according to PEP 333,
-            # may raise its own error at that point).
-            s, h, b = _cperror.bare_error()
-            self.iter_response = iter([])
+            try:
+                self.start_response(s, h, _sys.exc_info())
+            except:
+                # "The application must not trap any exceptions raised by
+                # start_response, if it called start_response with exc_info.
+                # Instead, it should allow such exceptions to propagate
+                # back to the server or gateway."
+                # But we still log and call close() to clean up ourselves.
+                _cherrypy.log(traceback=True, severity=40)
+                self.close()
+                raise
+            
             return "".join(b)
     
     def close(self):

cherrypy/test/test_core.py

             self.getPage("/error/page_yield")
             self.assertErrorPage(500, pattern=valerr)
             
-            self.getPage("/error/page_streamed")
-            # Because this error is raised after the response body has
-            # started, the status should not change to an error status.
-            self.assertStatus(200)
-            self.assertBody("word upUnrecoverable error in the server.")
+            if cherrypy.server.protocol_version == "HTTP/1.0":
+                self.getPage("/error/page_streamed")
+                # Because this error is raised after the response body has
+                # started, the status should not change to an error status.
+                self.assertStatus(200)
+                self.assertBody("word up")
+            else:
+                # Under HTTP/1.1, the chunked transfer-coding is used.
+                # The HTTP client will choke when the output is incomplete.
+                self.assertRaises(ValueError, self.getPage,
+                                  "/error/page_streamed")
             
             # No traceback should be present
             self.getPage("/error/cause_err_in_finalize")

cherrypy/test/test_encoding.py

         # readable page, since 1) the gzip header is already set,
         # and 2) we may have already written some of the body.
         # The fix is to never stream yields when using gzip.
-        self.getPage('/gzip/noshow_stream',
-                     headers=[("Accept-Encoding", "gzip")])
-        self.assertHeader('Content-Encoding', 'gzip')
-        self.assertMatchesBody(r"Unrecoverable error in the server.$")
+        if cherrypy.server.protocol_version == "HTTP/1.0":
+            self.getPage('/gzip/noshow_stream',
+                         headers=[("Accept-Encoding", "gzip")])
+            self.assertHeader('Content-Encoding', 'gzip')
+            self.assertInBody('\x1f\x8b\x08\x00')
+        else:
+            # The wsgiserver will simply stop sending data, and the HTTP client
+            # will error due to an incomplete chunk-encoded stream.
+            self.assertRaises(ValueError, self.getPage, '/gzip/noshow_stream',
+                              headers=[("Accept-Encoding", "gzip")])
 
 
 if __name__ == "__main__":

cherrypy/test/test_tools.py

             raise ValueError()
         
         def errinstream(self, id=None):
+            yield "nonconfidential"
             raise ValueError()
             yield "confidential"
         
         self.assertBody("True")
         
         # If body is "razdrez", then on_end_request is being called too early.
-        self.getPage("/demo/errinstream?id=5")
-        # Because this error is raised after the response body has
-        # started, the status should not change to an error status.
-        self.assertStatus("200 OK")
-        self.assertBody("Unrecoverable error in the server.")
+        if cherrypy.server.protocol_version == "HTTP/1.0":
+            self.getPage("/demo/errinstream?id=5")
+            # Because this error is raised after the response body has
+            # started, the status should not change to an error status.
+            self.assertStatus("200 OK")
+            self.assertBody("nonconfidential")
+        else:
+            # Because this error is raised after the response body has
+            # started, and because it's chunked output, an error is raised by
+            # the HTTP client when it encounters incomplete output.
+            self.assertRaises(ValueError, self.getPage,
+                              "/demo/errinstream?id=5")
         # If this fails, then on_end_request isn't being called at all.
         time.sleep(0.1)
         self.getPage("/demo/ended/5")

cherrypy/wsgiserver/__init__.py

         try:
             self._respond()
         except MaxSizeExceeded:
-            self.simple_response("413 Request Entity Too Large")
+            if not self.sent_headers:
+                self.simple_response("413 Request Entity Too Large")
             return
     
     def _respond(self):
     
     def start_response(self, status, headers, exc_info = None):
         """WSGI callable to begin the HTTP response."""
-        if self.started_response:
-            if not exc_info:
-                raise AssertionError("WSGI start_response called a second "
-                                     "time with no exc_info.")
-            else:
-                try:
-                    raise exc_info[0], exc_info[1], exc_info[2]
-                finally:
-                    exc_info = None
+        # "The application may call start_response more than once,
+        # if and only if the exc_info argument is provided."
+        if self.started_response and not exc_info:
+            raise AssertionError("WSGI start_response called a second "
+                                 "time with no exc_info.")
+        
+        # "if exc_info is provided, and the HTTP headers have already been
+        # sent, start_response must raise an error, and should raise the
+        # exc_info tuple."
+        if self.sent_headers:
+            try:
+                raise exc_info[0], exc_info[1], exc_info[2]
+            finally:
+                exc_info = None
+        
         self.started_response = True
         self.status = status
         self.outheaders.extend(headers)
         except socket.error, e:
             errnum = e.args[0]
             if errnum == 'timed out':
-                if req:
+                if req and not req.sent_headers:
                     req.simple_response("408 Request Timeout")
             elif errnum not in socket_errors_to_ignore:
-                if req:
+                if req and not req.sent_headers:
                     req.simple_response("500 Internal Server Error",
                                         format_exc())
             return
         except NoSSLError:
             # Unwrap our wfile
             req.wfile = CP_fileobject(self.socket, "wb", -1)
-            req.simple_response("400 Bad Request",
-                                "The client sent a plain HTTP request, but "
-                                "this server only speaks HTTPS on this port.")
+            if req and not req.sent_headers:
+                req.simple_response("400 Bad Request",
+                    "The client sent a plain HTTP request, but "
+                    "this server only speaks HTTPS on this port.")
         except Exception, e:
-            if req:
+            if req and not req.sent_headers:
                 req.simple_response("500 Internal Server Error", format_exc())
     
     def close(self):
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.