Commits

Robert Brewer  committed 0d78b73

Fix for #556 (Allow error hooks to raise InternalRedirect). As a bonus, all uncaught errors in Request.run now get a fallback handler (before only errors in handle_error got that).

  • Participants
  • Parent commits 03da9e5

Comments (0)

Files changed (2)

File cherrypy/_cprequest.py

     methods_with_bodies = ("POST", "PUT")
     body = None
     body_read = False
+    headers_read = False
     
     # Dispatch attributes
     dispatch = Dispatcher()
             self.cookie = Cookie.SimpleCookie()
             self.handler = None
             
-            # Get the 'Host' header, so we can do HTTPRedirects properly.
-            self.process_headers()
-            
             # path_info should be the path from the
             # app root (script_name) to the handler.
             self.script_name = self.app.script_name
                     self.redirections.append(pi)
         except (KeyboardInterrupt, SystemExit):
             raise
-        except cherrypy.TimeoutError:
-            raise
         except:
             if self.throw_errors:
                 raise
-            self.handle_error(sys.exc_info())
+            else:
+                # Can't use handle_error because we may not have hooks yet.
+                cherrypy.log(traceback=True)
+                
+                # Failure in setup, error handler or finalize. Bypass them.
+                if self.show_tracebacks:
+                    exc = sys.exc_info()
+                    try:
+                        dbltrace = ("\n===First Error===\n\n%s"
+                                    "\n\n===Second Error===\n\n%s\n\n")
+                        body = dbltrace % (format_exc(exc), format_exc())
+                    finally:
+                        del exc
+                else:
+                    body = ""
+                r = bare_error(body)
+                response = cherrypy.response
+                response.status, response.header_list, response.body = r
         
         if self.method == "HEAD":
             # HEAD requests MUST NOT return a message-body in the response.
         """Generate a response for the resource at self.path_info."""
         try:
             try:
-                if cherrypy.response.timed_out:
-                    raise cherrypy.TimeoutError()
-                
-                if self.app is None:
-                    raise cherrypy.NotFound()
-                
-                self.hooks = HookMap(self.hookpoints)
-                self.get_resource(path_info)
-                self.tool_up()
-                self.hooks.run('on_start_resource')
-                
-                if not self.body_read:
+                try:
+                    if cherrypy.response.timed_out:
+                        raise cherrypy.TimeoutError()
+                    
+                    if self.app is None:
+                        raise cherrypy.NotFound()
+                    
+                    if not self.headers_read:
+                        # Get the 'Host' header, so we can do HTTPRedirects properly.
+                        self.process_headers()
+                    
+                    self.hooks = HookMap(self.hookpoints)
+                    self.get_resource(path_info)
+                    self.tool_up()
+                    
+                    self.hooks.run('on_start_resource')
+                    
+                    if not self.body_read:
+                        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 request body
+                            mbs = cherrypy.server.max_request_body_size
+                            if mbs > 0:
+                                self.rfile = http.SizeCheckWrapper(self.rfile, mbs)
+                    
+                    self.hooks.run('before_request_body')
                     if self.process_request_body:
-                        if self.method not in self.methods_with_bodies:
-                            self.process_request_body = False
+                        self.process_body()
                     
-                    if self.process_request_body:
-                        # Prepare the SizeCheckWrapper for the request body
-                        mbs = cherrypy.server.max_request_body_size
-                        if mbs > 0:
-                            self.rfile = http.SizeCheckWrapper(self.rfile, mbs)
-                
-                self.hooks.run('before_request_body')
-                if self.process_request_body:
-                    self.process_body()
-                
-                self.hooks.run('before_main')
-                if self.handler:
-                    self.handler()
-                self.hooks.run('before_finalize')
-                cherrypy.response.finalize()
-            except (cherrypy.HTTPRedirect, cherrypy.HTTPError), inst:
-                inst.set_response()
-                self.hooks.run('before_finalize')
-                cherrypy.response.finalize()
-        finally:
-            self.hooks.run('on_end_resource')
+                    self.hooks.run('before_main')
+                    if self.handler:
+                        self.handler()
+                    self.hooks.run('before_finalize')
+                    cherrypy.response.finalize()
+                except (cherrypy.HTTPRedirect, cherrypy.HTTPError), inst:
+                    inst.set_response()
+                    self.hooks.run('before_finalize')
+                    cherrypy.response.finalize()
+            finally:
+                self.hooks.run('on_end_resource')
+        except cherrypy.InternalRedirect:
+            raise
+        except:
+            if self.throw_errors:
+                raise
+            self.handle_error(sys.exc_info())
     
     def process_headers(self):
+        # Guard against re-reading body (e.g. on InternalRedirect)
+        if self.headers_read:
+            return
+        self.headers_read = True
+        
         self.params = http.parse_query_string(self.query_string)
         
         # Process the headers into self.headers
             self.params.update(http.params_from_CGI_form(forms))
     
     def handle_error(self, exc):
-        response = cherrypy.response
         try:
             self.hooks.run("before_error_response")
             if self.error_response:
                 self.error_response()
             self.hooks.run("after_error_response")
-            response.finalize()
-            return
+            cherrypy.response.finalize()
         except cherrypy.HTTPRedirect, inst:
-            try:
-                inst.set_response()
-                response.finalize()
-                return
-            except (KeyboardInterrupt, SystemExit):
-                raise
-            except:
-                # Fall through to the second error handler
-                pass
-        except (KeyboardInterrupt, SystemExit):
-            raise
-        except:
-            # Fall through to the second error handler
-            pass
-        
-        # Failure in error handler or finalize. Bypass them.
-        if self.show_tracebacks:
-            dbltrace = ("\n===First Error===\n\n%s"
-                        "\n\n===Second Error===\n\n%s\n\n")
-            body = dbltrace % (format_exc(exc), format_exc())
-        else:
-            body = ""
-        r = bare_error(body)
-        response.status, response.header_list, response.body = r
+            inst.set_response()
+            cherrypy.response.finalize()
 
 
 def file_generator(input, chunkSize=65536):

File cherrypy/test/test_core.py

         def stringify(self):
             return str(cherrypy.HTTPRedirect("/"))
     
+    
     def login_redir():
         if not getattr(cherrypy.request, "login", None):
             raise cherrypy.InternalRedirect("/internalredirect/login")
     tools.login_redir = _cptools.Tool('before_main', login_redir)
     
+    def redir_custom():
+        raise cherrypy.InternalRedirect("/internalredirect/custom_err")
+    
     class InternalRedirect(Test):
         
         def index(self):
         
         def login(self):
             return "Please log in"
-
-
+        login._cp_config = {'hooks.before_error_response': redir_custom}
+        
+        def custom_err(self):
+            return "Something went horribly wrong."
+    
     class Image(Test):
         
         def getImagesByUser(self, user_id):
         self.assertBody('I am a redirected page.')
         self.assertStatus(200)
         
+        # InternalRedirect on error
+        self.getPage("/internalredirect/login/illegal/extra/vpath/atoms")
+        self.assertStatus(200)
+        self.assertBody("Something went horribly wrong.")
+        
         # HTTPRedirect on error
         self.getPage("/redirect/error/")
         self.assertStatus(('302 Found', '303 See Other'))