Fixed #25099 -- Cleaned up HttpRequest representations in error reporting.

django/http/__init__.py

@@ -1,7 +1,6 @@
 from django.http.cookie import SimpleCookie, parse_cookie
 from django.http.request import (
     HttpRequest, QueryDict, RawPostDataException, UnreadablePostError,
-    build_request_repr,
 )
 from django.http.response import (
     BadHeaderError, FileResponse, Http404, HttpResponse,
@@ -14,7 +13,7 @@ from django.http.utils import conditional_content_removal
 
 __all__ = [
     'SimpleCookie', 'parse_cookie', 'HttpRequest', 'QueryDict',
-    'RawPostDataException', 'UnreadablePostError', 'build_request_repr',
+    'RawPostDataException', 'UnreadablePostError',
     'HttpResponse', 'StreamingHttpResponse', 'HttpResponseRedirect',
     'HttpResponsePermanentRedirect', 'HttpResponseNotModified',
     'HttpResponseBadRequest', 'HttpResponseForbidden', 'HttpResponseNotFound',

django/http/request.py

@@ -5,7 +5,6 @@ import re
 import sys
 from io import BytesIO
 from itertools import chain
-from pprint import pformat
 
 from django.conf import settings
 from django.core import signing
@@ -465,52 +464,6 @@ class QueryDict(MultiValueDict):
         return '&'.join(output)
 
 
-def build_request_repr(request, path_override=None, GET_override=None,
-                       POST_override=None, COOKIES_override=None,
-                       META_override=None):
-    """
-    Builds and returns the request's representation string. The request's
-    attributes may be overridden by pre-processed values.
-    """
-    # Since this is called as part of error handling, we need to be very
-    # robust against potentially malformed input.
-    try:
-        get = (pformat(GET_override)
-               if GET_override is not None
-               else pformat(request.GET))
-    except Exception:
-        get = '<could not parse>'
-    if request._post_parse_error:
-        post = '<could not parse>'
-    else:
-        try:
-            post = (pformat(POST_override)
-                    if POST_override is not None
-                    else pformat(request.POST))
-        except Exception:
-            post = '<could not parse>'
-    try:
-        cookies = (pformat(COOKIES_override)
-                   if COOKIES_override is not None
-                   else pformat(request.COOKIES))
-    except Exception:
-        cookies = '<could not parse>'
-    try:
-        meta = (pformat(META_override)
-                if META_override is not None
-                else pformat(request.META))
-    except Exception:
-        meta = '<could not parse>'
-    path = path_override if path_override is not None else request.path
-    return force_str('<%s\npath:%s,\nGET:%s,\nPOST:%s,\nCOOKIES:%s,\nMETA:%s>' %
-                     (request.__class__.__name__,
-                      path,
-                      six.text_type(get),
-                      six.text_type(post),
-                      six.text_type(cookies),
-                      six.text_type(meta)))
-
-
 # It's neither necessary nor appropriate to use
 # django.utils.encoding.smart_text for parsing URLs and form inputs. Thus,
 # this slightly more restricted function, used by QueryDict.

django/utils/log.py

@@ -4,14 +4,14 @@ import logging
 import logging.config  # needed when logging_config doesn't start with logging.config
 import sys
 import warnings
+from copy import copy
 
 from django.conf import settings
 from django.core import mail
 from django.core.mail import get_connection
 from django.utils.deprecation import RemovedInNextVersionWarning
-from django.utils.encoding import force_text
 from django.utils.module_loading import import_string
-from django.views.debug import ExceptionReporter, get_exception_reporter_filter
+from django.views.debug import ExceptionReporter
 
 # Default logging for Django. This sends an email to the site admins on every
 # HTTP 500 error. Depending on DEBUG, all other log records are either sent to
@@ -90,24 +90,27 @@ class AdminEmailHandler(logging.Handler):
                  else 'EXTERNAL'),
                 record.getMessage()
             )
-            filter = get_exception_reporter_filter(request)
-            request_repr = '\n{}'.format(force_text(filter.get_request_repr(request)))
         except Exception:
             subject = '%s: %s' % (
                 record.levelname,
                 record.getMessage()
             )
             request = None
-            request_repr = "unavailable"
         subject = self.format_subject(subject)
 
+        # Since we add a nicely formatted traceback on our own, create a copy
+        # of the log record without the exception data.
+        no_exc_record = copy(record)
+        no_exc_record.exc_info = None
+        no_exc_record.exc_text = None
+
         if record.exc_info:
             exc_info = record.exc_info
         else:
             exc_info = (None, record.getMessage(), None)
 
-        message = "%s\n\nRequest repr(): %s" % (self.format(record), request_repr)
         reporter = ExceptionReporter(request, is_email=True, *exc_info)
+        message = "%s\n\n%s" % (self.format(no_exc_record), reporter.get_traceback_text())
         html_message = reporter.get_traceback_html() if self.include_html else None
         self.send_mail(subject, message, fail_silently=True, html_message=html_message)
 

django/views/debug.py

@@ -6,9 +6,7 @@ import types
 
 from django.conf import settings
 from django.core.urlresolvers import Resolver404, resolve
-from django.http import (
+from django.http import HttpResponse, HttpResponseNotFound
-    HttpRequest, HttpResponse, HttpResponseNotFound, build_request_repr,
-)
 from django.template import Context, Engine, TemplateDoesNotExist
 from django.template.defaultfilters import force_escape, pprint
 from django.utils import lru_cache, six, timezone
@@ -113,12 +111,6 @@ class ExceptionReporterFilter(object):
     contain lenient default behaviors.
     """
 
-    def get_request_repr(self, request):
-        if request is None:
-            return repr(None)
-        else:
-            return build_request_repr(request, POST_override=self.get_post_parameters(request))
-
     def get_post_parameters(self, request):
         if request is None:
             return {}
@@ -186,16 +178,13 @@ class SafeExceptionReporterFilter(ExceptionReporterFilter):
     def cleanse_special_types(self, request, value):
         try:
             # If value is lazy or a complex object of another kind, this check
-            # might raise an exception. isinstance checks that lazy HttpRequests
+            # might raise an exception. isinstance checks that lazy
-            # or MultiValueDicts will have a return value.
+            # MultiValueDicts will have a return value.
-            is_request = isinstance(value, HttpRequest)
+            is_multivalue_dict = isinstance(value, MultiValueDict)
         except Exception as e:
             return '{!r} while evaluating {!r}'.format(e, value)
 
-        if is_request:
+        if is_multivalue_dict:
-            # Cleanse the request's POST parameters.
-            value = self.get_request_repr(value)
-        elif isinstance(value, MultiValueDict):
             # Cleanse MultiValueDicts (request.POST is the one we usually care about)
             value = self.get_cleansed_multivaluedict(request, value)
         return value
@@ -1126,9 +1115,11 @@ Settings:
 Using settings module {{ settings.SETTINGS_MODULE }}{% for k, v in settings.items|dictsort:"0" %}
 {{ k }} = {{ v|stringformat:"r" }}{% endfor %}
 
+{% if not is_email %}
 You're seeing this error because you have DEBUG = True in your
 Django settings file. Change that to False, and Django will
 display a standard page generated by the handler for this status code.
+{% endif %}
 """)
 
 TECHNICAL_404_TEMPLATE = """

docs/howto/error-reporting.txt

@@ -255,13 +255,6 @@ following methods:
     Returns ``True`` to activate the filtering operated in the other methods.
     By default the filter is active if :setting:`DEBUG` is ``False``.
 
-.. method:: SafeExceptionReporterFilter.get_request_repr(request)
-
-    Returns the representation string of the request object, that is, the
-    value that would be returned by ``repr(request)``, except it uses the
-    filtered dictionary of POST parameters as determined by
-    :meth:`SafeExceptionReporterFilter.get_post_parameters`.
-
 .. method:: SafeExceptionReporterFilter.get_post_parameters(request)
 
     Returns the filtered dictionary of POST parameters. By default it replaces

docs/releases/1.9.txt

@@ -683,6 +683,20 @@ console, for example.
 If you are overriding Django's default logging, you should check to see how
 your configuration merges with the new defaults.
 
+``HttpRequest`` details in error reporting
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+It was redundant to display the full details of the
+:class:`~django.http.HttpRequest` each time it appeared as a stack frame
+variable in the HTML version of the debug page and error email. Thus, the HTTP
+request will now display the same standard representation as other variables
+(``repr(request)``). As a result, the method
+``ExceptionReporterFilter.get_request_repr()`` was removed.
+
+The contents of the text version of the email were modified to provide a
+traceback of the same structure as in the case of AJAX requests. The traceback
+details are rendered by the ``ExceptionReporter.get_traceback_text()`` method.
+
 Removal of time zone aware global adapters and converters for datetimes
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 

tests/logging_tests/tests.py

@@ -334,7 +334,7 @@ class AdminEmailHandlerTest(SimpleTestCase):
         msg = mail.outbox[0]
         self.assertEqual(msg.to, ['admin@example.com'])
         self.assertEqual(msg.subject, "[Django] ERROR (EXTERNAL IP): message")
-        self.assertIn("path:%s" % url_path, msg.body)
+        self.assertIn("Report at %s" % url_path, msg.body)
 
     @override_settings(
         MANAGERS=[('manager', 'manager@example.com')],
@@ -419,7 +419,7 @@ class SecurityLoggerTest(SimpleTestCase):
     def test_suspicious_email_admins(self):
         self.client.get('/suspicious/')
         self.assertEqual(len(mail.outbox), 1)
-        self.assertIn('path:/suspicious/,', mail.outbox[0].body)
+        self.assertIn('Report at /suspicious/', mail.outbox[0].body)
 
 
 class SettingsCustomLoggingTest(AdminScriptTestCase):

tests/requests/tests.py

@@ -10,7 +10,7 @@ from django.core.exceptions import SuspiciousOperation
 from django.core.handlers.wsgi import LimitedStream, WSGIRequest
 from django.http import (
     HttpRequest, HttpResponse, RawPostDataException, UnreadablePostError,
-    build_request_repr, parse_cookie,
+    parse_cookie,
 )
 from django.test import RequestFactory, SimpleTestCase, override_settings
 from django.test.client import FakePayload
@@ -60,9 +60,6 @@ class RequestsTests(SimpleTestCase):
         request.COOKIES = {'post-key': 'post-value'}
         request.META = {'post-key': 'post-value'}
         self.assertEqual(repr(request), str_prefix("<HttpRequest: GET '/somepath/'>"))
-        self.assertEqual(build_request_repr(request), str_prefix("<HttpRequest\npath:/somepath/,\nGET:{%(_)s'get-key': %(_)s'get-value'},\nPOST:{%(_)s'post-key': %(_)s'post-value'},\nCOOKIES:{%(_)s'post-key': %(_)s'post-value'},\nMETA:{%(_)s'post-key': %(_)s'post-value'}>"))
-        self.assertEqual(build_request_repr(request, path_override='/otherpath/', GET_override={'a': 'b'}, POST_override={'c': 'd'}, COOKIES_override={'e': 'f'}, META_override={'g': 'h'}),
-                         str_prefix("<HttpRequest\npath:/otherpath/,\nGET:{%(_)s'a': %(_)s'b'},\nPOST:{%(_)s'c': %(_)s'd'},\nCOOKIES:{%(_)s'e': %(_)s'f'},\nMETA:{%(_)s'g': %(_)s'h'}>"))
 
     def test_httprequest_repr_invalid_method_and_path(self):
         request = HttpRequest()
@@ -74,22 +71,6 @@ class RequestsTests(SimpleTestCase):
         request.path = ""
         self.assertEqual(repr(request), str_prefix("<HttpRequest>"))
 
-    def test_bad_httprequest_repr(self):
-        """
-        If an exception occurs when parsing GET, POST, COOKIES, or META, the
-        repr of the request should show it.
-        """
-        class Bomb(object):
-            """An object that raises an exception when printed out."""
-            def __repr__(self):
-                raise Exception('boom!')
-
-        bomb = Bomb()
-        for attr in ['GET', 'POST', 'COOKIES', 'META']:
-            request = HttpRequest()
-            setattr(request, attr, {'bomb': bomb})
-            self.assertIn('%s:<could not parse>' % attr, build_request_repr(request))
-
     def test_wsgirequest(self):
         request = WSGIRequest({'PATH_INFO': 'bogus', 'REQUEST_METHOD': 'bogus', 'wsgi.input': BytesIO(b'')})
         self.assertEqual(list(request.GET.keys()), [])
@@ -147,9 +128,6 @@ class RequestsTests(SimpleTestCase):
         request.COOKIES = {'post-key': 'post-value'}
         request.META = {'post-key': 'post-value'}
         self.assertEqual(repr(request), str_prefix("<WSGIRequest: GET '/somepath/'>"))
-        self.assertEqual(build_request_repr(request), str_prefix("<WSGIRequest\npath:/somepath/,\nGET:{%(_)s'get-key': %(_)s'get-value'},\nPOST:{%(_)s'post-key': %(_)s'post-value'},\nCOOKIES:{%(_)s'post-key': %(_)s'post-value'},\nMETA:{%(_)s'post-key': %(_)s'post-value'}>"))
-        self.assertEqual(build_request_repr(request, path_override='/otherpath/', GET_override={'a': 'b'}, POST_override={'c': 'd'}, COOKIES_override={'e': 'f'}, META_override={'g': 'h'}),
-                         str_prefix("<WSGIRequest\npath:/otherpath/,\nGET:{%(_)s'a': %(_)s'b'},\nPOST:{%(_)s'c': %(_)s'd'},\nCOOKIES:{%(_)s'e': %(_)s'f'},\nMETA:{%(_)s'g': %(_)s'h'}>"))
 
     def test_wsgirequest_path_info(self):
         def wsgi_str(path_info):