Opened 3 years ago

Closed 2 years ago

#20368 closed Cleanup/optimization (fixed)

MemoryError can block rendering of traceback page

Reported by: ironfroggy Owned by: nobody
Component: Template system Version: 1.3
Severity: Normal Keywords:
Cc: walter+django@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

When preparing frame variables for the traceback page on a low memory VM, it is possible to hit a MemoryError during the html escaping. For the purposes of a traceback page, there should be some fallback for this.

In django/views/debug.py:117 the call to force_escape() could be wrapped in a try/except and catch the MemoryError, representing the variable in the frame as something like "could not display, representation too large".

As it stands, this just hides the original error by never producing the traceback page.

Traceback (most recent call last):

  File "/home/vagrant/devel/cms_dev/local/lib/python2.7/site-packages/django/core/servers/basehttp.py", line 283, in run
    self.result = application(self.environ, self.start_response)

  File "/home/vagrant/devel/cms_dev/local/lib/python2.7/site-packages/django/contrib/staticfiles/handlers.py", line 68, in __call__
    return self.application(environ, start_response)

  File "/home/vagrant/devel/cms_dev/local/lib/python2.7/site-packages/django/core/handlers/wsgi.py", line 273, in __call__
    response = self.get_response(request)

  File "/home/vagrant/devel/cms_dev/local/lib/python2.7/site-packages/django/core/handlers/base.py", line 182, in get_response
    response = self.handle_uncaught_exception(request, resolver, sys.exc_info())

  File "/home/vagrant/devel/cms_dev/local/lib/python2.7/site-packages/django/core/handlers/base.py", line 203, in handle_uncaught_exception
    return debug.technical_500_response(request, *exc_info)

  File "/home/vagrant/devel/cms_dev/local/lib/python2.7/site-packages/django/views/debug.py", line 59, in technical_500_response
    html = reporter.get_traceback_html()

  File "/home/vagrant/devel/cms_dev/local/lib/python2.7/site-packages/django/views/debug.py", line 117, in get_traceback_html
    frame['vars'] = [(k, force_escape(pprint(v))) for k, v in frame['vars']]

  File "/home/vagrant/devel/cms_dev/local/lib/python2.7/site-packages/django/template/defaultfilters.py", line 37, in _dec
    return func(*args, **kwargs)

  File "/home/vagrant/devel/cms_dev/local/lib/python2.7/site-packages/django/template/defaultfilters.py", line 398, in force_escape
    return mark_safe(escape(value))

  File "/home/vagrant/devel/cms_dev/local/lib/python2.7/site-packages/django/utils/functional.py", line 259, in wrapper
    return func(*args, **kwargs)

  File "/home/vagrant/devel/cms_dev/local/lib/python2.7/site-packages/django/utils/html.py", line 34, in escape
    return mark_safe(force_unicode(html).replace('&', '&amp;').replace('<', '&lt;').replace('>', '&gt;').replace('"', '&quot;').replace("'", '&#39;'))

MemoryError

Attachments (1)

20368fix-against-56d6fdbbf50b29bd162fd5ab3296a25127d7af65.diff (6.1 KB) - added by Grzegorz Jakacki 3 years ago.
Patch against [56d6fdbbf50b29bd162fd5ab3296a25127d7af65]

Download all attachments as: .zip

Change History (12)

comment:1 Changed 3 years ago by Jef Geskens

Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset
Triage Stage: UnreviewedAccepted

I also get this a lot. Especially when dealing with big variables (for example megabytes of binary data) in your stacktrace...

comment:2 Changed 3 years ago by Jef Geskens

Easy pickings: set

comment:3 Changed 3 years ago by Grzegorz Jakacki

Has patch: set

comment:4 Changed 3 years ago by Tim Graham

Component: UncategorizedTemplate system
Easy pickings: unset
Type: UncategorizedCleanup/optimization

comment:5 Changed 3 years ago by Tim Graham

Similar issue in #20534 which I closed as a duplicate of this one. The patch there looks to be less invasive although I haven't reviewed either in detail.

comment:6 Changed 3 years ago by Walter Doekes

Cc: walter+django@… added

comment:7 Changed 2 years ago by Tim Graham

Patch needs improvement: set

A GitHub PR would make review of the patch easier. I spotted a u'' prefix which isn't valid on Python 3.2 and the exception format needs to be Exception as e for Python 3 compatibility.

comment:8 Changed 2 years ago by Walter Doekes

Here, my fix from #20534:

--- django/views/debug.py.orig	2013-05-31 13:02:36.807798172 +0200
+++ django/views/debug.py	2013-05-31 13:01:52.555274822 +0200
@@ -244,7 +244,17 @@ class ExceptionReporter(object):
         frames = self.get_traceback_frames()
         for i, frame in enumerate(frames):
             if 'vars' in frame:
-                frame['vars'] = [(k, force_escape(pprint(v))) for k, v in frame['vars']]
+                vars = []
+                for k, v in frame['vars']:
+                    v = pprint(v)
+                    # The force_escape filter assume unicode, make sure that works
+                    if isinstance(v, str):
+                        v = v.decode('utf-8', 'replace')  # don't choke on non-utf-8 input
+                    # You may be looking at large blobs of data, trim it
+                    if len(v) > 4096:
+                        v = u'%s... <trimmed %d bytes string>' % (v[0:4096], len(v))
+                    vars.append((k, force_escape(v)))
+                frame['vars'] = vars
             frames[i] = frame
 
         unicode_hint = ''

comment:9 Changed 2 years ago by Tim Graham

Looks reasonable, we'd just need some tests.

comment:10 Changed 2 years ago by Walter Doekes

Now filed as PR: https://github.com/django/django/pull/3054
(replacing the u'' and isinstance(..., str))

comment:11 Changed 2 years ago by Tim Graham <timograham@…>

Resolution: fixed
Status: newclosed

In e0e28bfe715b3f7d4e6cc7ab7bf4000b22c0cf79:

Fixed #20368 -- Made TECHNICAL_500 more robust against bad input.

This limits large variables and avoids non-utf-8 in the TECHNICAL_500 output.

Note: See TracTickets for help on using tickets.
Back to Top