Fix for #800 (ability to override default error template). Many thanks to Scott Chapman for the ideas and Nicolas Grilly for the fix.

3 files changed, 96 insertions, 43 deletions

diff --git a/cherrypy/_cperror.py b/cherrypy/_cperror.py
index 23914e10..506a6652 100644
--- a/cherrypy/_cperror.py
+++ b/cherrypy/_cperror.py
@@ -2,6 +2,7 @@
 
 from cgi import escape as _escape
 from sys import exc_info as _exc_info
+from traceback import format_exception as _format_exception
 from urlparse import urljoin as _urljoin
 from cherrypy.lib import http as _http
 
@@ -148,6 +149,32 @@ class HTTPRedirect(CherryPyException):
         raise self
 
 
+def clean_headers(status):
+    """Remove any headers which should not apply to an error response."""
+    import cherrypy
+    
+    response = cherrypy.response
+    
+    # Remove headers which applied to the original content,
+    # but do not apply to the error page.
+    respheaders = response.headers
+    for key in ["Accept-Ranges", "Age", "ETag", "Location", "Retry-After",
+                "Vary", "Content-Encoding", "Content-Length", "Expires",
+                "Content-Location", "Content-MD5", "Last-Modified"]:
+        if respheaders.has_key(key):
+            del respheaders[key]
+    
+    if status != 416:
+        # A server sending a response with status code 416 (Requested
+        # range not satisfiable) SHOULD include a Content-Range field
+        # with a byte-range-resp-spec of "*". The instance-length
+        # specifies the current length of the selected resource.
+        # A response with status code 206 (Partial Content) MUST NOT
+        # include a Content-Range field with a byte-range- resp-spec of "*".
+        if respheaders.has_key("Content-Range"):
+            del respheaders["Content-Range"]
+
+
 class HTTPError(CherryPyException):
     """ Exception used to return an HTTP error code (4xx-5xx) to the client.
         This exception will automatically set the response status and body.
@@ -173,24 +200,7 @@ class HTTPError(CherryPyException):
         
         response = cherrypy.response
         
-        # Remove headers which applied to the original content,
-        # but do not apply to the error page.
-        respheaders = response.headers
-        for key in ["Accept-Ranges", "Age", "ETag", "Location", "Retry-After",
-                    "Vary", "Content-Encoding", "Content-Length", "Expires",
-                    "Content-Location", "Content-MD5", "Last-Modified"]:
-            if respheaders.has_key(key):
-                del respheaders[key]
-        
-        if self.status != 416:
-            # A server sending a response with status code 416 (Requested
-            # range not satisfiable) SHOULD include a Content-Range field
-            # with a byte-range-resp-spec of "*". The instance-length
-            # specifies the current length of the selected resource.
-            # A response with status code 206 (Partial Content) MUST NOT
-            # include a Content-Range field with a byte-range- resp-spec of "*".
-            if respheaders.has_key("Content-Range"):
-                del respheaders["Content-Range"]
+        clean_headers(self.status)
         
         # In all cases, finalize will be called after this method,
         # so don't bother cleaning up response values here.
@@ -198,12 +208,12 @@ class HTTPError(CherryPyException):
         tb = None
         if cherrypy.request.show_tracebacks:
             tb = format_exc()
-        respheaders['Content-Type'] = "text/html"
+        response.headers['Content-Type'] = "text/html"
         
         content = self.get_error_page(self.status, traceback=tb,
                                       message=self.message)
         response.body = content
-        respheaders['Content-Length'] = len(content)
+        response.headers['Content-Length'] = len(content)
         
         _be_ie_unfriendly(self.status)
     
@@ -278,28 +288,31 @@ def get_error_page(status, **kwargs):
         kwargs['traceback'] = ''
     if kwargs.get('version') is None:
         kwargs['version'] = cherrypy.__version__
+    
     for k, v in kwargs.iteritems():
         if v is None:
             kwargs[k] = ""
         else:
             kwargs[k] = _escape(kwargs[k])
     
-    template = _HTTPErrorTemplate
-    
-    # Replace the default template with a custom one?
-    error_page_file = cherrypy.request.error_page.get(code, '')
-    if error_page_file:
+    # Use a custom template or callable for the error page?
+    pages = cherrypy.request.error_page
+    error_page = pages.get(code) or pages.get('default')
+    if error_page:
         try:
-            template = file(error_page_file, 'rb').read()
+            if callable(error_page):
+                return error_page(**kwargs)
+            else:
+                return file(error_page, 'rb').read() % kwargs
         except:
+            e = _format_exception(*_exc_info())[-1]
             m = kwargs['message']
             if m:
                 m += "<br />"
-            m += ("In addition, the custom error page "
-                  "failed:\n<br />%s" % (_exc_info()[1]))
+            m += "In addition, the custom error page failed:\n<br />%s" % e
             kwargs['message'] = m
     
-    return template % kwargs
+    return _HTTPErrorTemplate % kwargs
 
 
 _ie_friendly_error_sizes = {
@@ -368,3 +381,4 @@ def bare_error(extrabody=None):
              ('Content-Length', str(len(body)))],
             [body])
 
+
diff --git a/cherrypy/_cprequest.py b/cherrypy/_cprequest.py
index 6382e325..2ac1dfb9 100644
--- a/cherrypy/_cprequest.py
+++ b/cherrypy/_cprequest.py
@@ -145,7 +145,9 @@ def response_namespace(k, v):
 
 def error_page_namespace(k, v):
     """Attach error pages declared in config."""
-    cherrypy.request.error_page[int(k)] = v
+    if k != 'default':
+        k = int(k)
+    cherrypy.request.error_page[k] = v
 
 
 hookpoints = ['on_start_resource', 'before_request_body',
@@ -392,11 +394,27 @@ class Request(object):
     
     error_page = {}
     error_page__doc = """
-    A dict of {error code: response filename} pairs. The named response
-    files should be Python string-formatting templates, and can expect by
-    default to receive the format values with the mapping keys 'status',
-    'message', 'traceback', and 'version'. The set of format mappings
-    can be extended by overriding HTTPError.set_response."""
+    A dict of {error code: response filename or callable} pairs.
+    
+    The error code must be an int representing a given HTTP error code,
+    or the string 'default', which will be used if no matching entry
+    is found for a given numeric code.
+    
+    If a filename is provided, the file should contain a Python string-
+    formatting template, and can expect by default to receive format 
+    values with the mapping keys %(status)s, %(message)s, %(traceback)s,
+    and %(version)s. The set of format mappings can be extended by
+    overriding HTTPError.set_response.
+    
+    If a callable is provided, it will be called by default with keyword 
+    arguments 'status', 'message', 'traceback', and 'version', as for a
+    string-formatting template. The callable must return a string which
+    will be set to response.body. It may also override headers or perform
+    any other processing.
+    
+    If no entry is given for an error code, and no 'default' entry exists,
+    a default template will be used.
+    """
     
     show_tracebacks = True
     show_tracebacks__doc = """
@@ -715,7 +733,7 @@ class Request(object):
             self.params.update(p)
     
     def handle_error(self, exc):
-        """Handle the last exception. (Core)"""
+        """Handle the last unanticipated exception. (Core)"""
         try:
             self.hooks.run("before_error_response")
             if self.error_response:
@@ -883,3 +901,4 @@ class Response(object):
             self.timed_out = True
 
 
+
diff --git a/cherrypy/test/test_core.py b/cherrypy/test/test_core.py
index b1c6b518..94bf1b36 100644
--- a/cherrypy/test/test_core.py
+++ b/cherrypy/test/test_core.py
@@ -239,16 +239,26 @@ def setup_server():
         def as_refyield(self):
             for chunk in self.as_yield():
                 yield chunk
-
-
+    
+    
+    def callable_error_page(status, **kwargs):
+        return "Error %s - Well, I'm very sorry but you haven't paid!" % status
+    
+    
     class Error(Test):
         
         _cp_config = {'tools.log_tracebacks.on': True,
                       }
         
-        def custom(self):
-            raise cherrypy.HTTPError(404, "No, <b>really</b>, not found!")
-        custom._cp_config = {'error_page.404': os.path.join(localDir, "static/index.html")}
+        def custom(self, err='404'):
+            raise cherrypy.HTTPError(int(err), "No, <b>really</b>, not found!")
+        custom._cp_config = {'error_page.404': os.path.join(localDir, "static/index.html"),
+                             'error_page.401': callable_error_page,
+                             }
+        
+        def custom_default(self):
+            return 1 + 'a' # raise an unexpected error
+        custom_default._cp_config = {'error_page.default': callable_error_page}
         
         def noexist(self):
             raise cherrypy.HTTPError(404, "No, <b>really</b>, not found!")
@@ -720,18 +730,28 @@ class CoreRequestHandlingTest(helper.CPWebCase):
         finally:
             ignore.pop()
         
-        # Test custom error page.
+        # Test custom error page for a specific error.
         self.getPage("/error/custom")
         self.assertStatus(404)
         self.assertBody("Hello, world\r\n" + (" " * 499))
         
+        # Test custom error page for a specific error.
+        self.getPage("/error/custom?err=401")
+        self.assertStatus(401)
+        self.assertBody("Error 401 Unauthorized - Well, I'm very sorry but you haven't paid!")
+        
+        # Test default custom error page.
+        self.getPage("/error/custom_default")
+        self.assertStatus(500)
+        self.assertBody("Error 500 Internal Server Error - Well, I'm very sorry but you haven't paid!".ljust(513))
+        
         # Test error in custom error page (ticket #305).
         # Note that the message is escaped for HTML (ticket #310).
         self.getPage("/error/noexist")
         self.assertStatus(404)
         msg = ("No, &lt;b&gt;really&lt;/b&gt;, not found!<br />"
                "In addition, the custom error page failed:\n<br />"
-               "[Errno 2] No such file or directory: 'nonexistent.html'")
+               "IOError: [Errno 2] No such file or directory: 'nonexistent.html'")
         self.assertInBody(msg)
         
         if (hasattr(self, 'harness') and