Fix edge case in exception handler causing recursion #260
Conversation
During Plone's porting efforts to Python 3.6 a bug was found that caused Python to dump core with a stack overflow. This is caused in the field handling, where a template calls a macro which then inserts the rendered body of another pagetemplate. The inner page template is rendered and calls the exception handler correctly, which causes the exception's `__str__` method to be replaced with an instance of `ExceptionFormatter`. However, as this is within a macro the `visit_Macro` function causes that exception to be added to `rcontext['__error__']``, meaning that when the outer template attempts to format its exception the handling of the `__errors__` list causes recursion into the `ExceptionFormatter` until the maximum recursion depth is reached at which point the error handling is aborted as unformattable by the traceback module in the standard library. If the exception is sufficiently large it smashes the stack before this can happen, causing a core dump. If it isn't sufficiently large a confusingly repetitious error is shown.
|
This is intended to fix plone/Products.CMFPlone#2370 |
|
There's been something fishy about the rendering of nested errors for some time – this looks good, but I have one question: does it work in practice on "normal" template errors...? Because I find that the output just often times makes no sense – and it used to! |
|
I reported on this bug in the linked Plone issue. With the patch added to Chamelon 3.2, I got a nice backtrace instead of the coredump (that's expected behavior since the data is totally incorrect) and the same backtrace for the more 'normal' template bug (divide by zero) so it's complete success for me. Here is the backtrace for the 'normal' exception: The raise_with_traceback line in the middle of the trace is of course not what could be seen in a debugger if one was setting a breakpoint at the exception point. Not a big problem IMO. |
|
I guarded the change with a check to see if Perhaps the issue with the normal errors is that the formatted error is always based on If you look at the Plone issue linked the error passes through an unrelated looking |
|
@malthe I've done some poking, and from a first approximation I think that setting |
|
By way of comparison, the following change: index 7d2986d..cff68d3 100644
--- a/src/chameleon/compiler.py
+++ b/src/chameleon/compiler.py
@@ -1493,8 +1493,8 @@ class Compiler(object):
render = "render"
else:
render = "render_%s" % mangle(node.name)
-
- return template(
+ token_reset = template("__token = None")
+ return token_reset + template(
"f(__stream, econtext.copy(), rcontext, __i18n_domain)",
f=render) + \
template("econtext.update(rcontext)")means the error: becomes |
|
I think that token fix looks about right then – it's something that I recently added, to reduce cost of the high-level "try/except" blocks. Now, Chameleon just assigns a single error token to a predefined global value. |
This was discussed in issue #260.
|
PR here for your token fix: #261. |
this version contains a fix for malthe/chameleon#260
this version contains a fix for malthe/chameleon#260
During Plone's porting efforts to Python 3.6 a bug was found
that caused Python to dump core with a stack overflow. This
is caused in the field handling, where a template calls a
macro which then inserts the rendered body of another pagetemplate.
The inner page template is rendered and calls the exception handler
correctly, which causes the exception's
__str__method to bereplaced with an instance of
ExceptionFormatter. However, asthis is within a macro the
visit_Macrofunction causes thatexception to be added to
rcontext['__error__'], meaning thatwhen the outer template attempts to format its exception the
handling of the
__errors__list causes recursion into theExceptionFormatteruntil the maximum recursion depth is reachedat which point the error handling is aborted as unformattable
by the traceback module in the standard library.
If the exception is sufficiently large it smashes the stack before
this can happen, causing a core dump. If it isn't sufficiently
large a confusingly repetitious error is shown.