Prevent recursion in Jinjava.#142
Conversation
| return renderStack.pop(); | ||
| } | ||
|
|
||
| public Stack getRenderStack() { |
There was a problem hiding this comment.
If we really want to protect the underlying stack from being modified outside the Context, then I think you just need to implement doesStackContain(). Otherwise this allows total access to the stack contents.
| "{{ allSpans(spans, '') }}" + | ||
| ""; | ||
| Node node = new TreeParser(interpreter, jinja).buildTree(); | ||
| assertThat(JinjavaInterpreter.getCurrent() == interpreter).isTrue(); |
There was a problem hiding this comment.
what is this line testing for?
|
|
||
| @Test | ||
| public void itPreventsRecursionForMacroWithVar() { | ||
| String jinja = "{%- macro allSpans(spans) %}" + |
There was a problem hiding this comment.
Can this example be simplified? It reflects the original issue, but I think the the test would be easier to understand if it the content of spans was smaller and it wasn't so specific to the original issue with spans, since it really could be a problem with any type of map.
|
|
||
| @Test | ||
| public void itNoRecursionHere() throws Exception { | ||
| public void itDoesNotRescursivelyEvaluateExpressions() throws Exception { |
There was a problem hiding this comment.
In fact, there is not recursion here.
|
|
||
|
|
||
| @Test | ||
| public void itPreventsRecursionForMacroWithVar() { |
There was a problem hiding this comment.
Maybe this test is not necessary, since the recursion cases are tested in the ExpressionNodeTest. This is to confirm that the recursion happens even when macro is called.
2 participants
Although we limit the rendering depth at 10, but that can still be large if the recursive expression has multiple self references. With this change, we still render the recursion once, then stop render is any further.