Merge pull request #142 from HubSpot/prevent-recursion

Prevent recursion in Jinjava.
HubSpot · Aug 15, 2017 · 1d8ff37 · 1d8ff37
2 parents 44c58c3 + 8b98ced
commit 1d8ff37
Show file tree

Hide file tree

Showing 4 changed files with 73 additions and 3 deletions.
diff --git a/src/main/java/com/hubspot/jinjava/interpret/Context.java b/src/main/java/com/hubspot/jinjava/interpret/Context.java
@@ -23,6 +23,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.Stack;
 import java.util.stream.Collectors;
 
 import com.google.common.collect.HashMultimap;
@@ -74,6 +75,8 @@ public enum Library {
   private Boolean autoEscape;
   private List<? extends Node> superBlock;
 
+  private final Stack<String> renderStack = new Stack<>();
+
   public Context() {
     this(null, null, null);
   }
@@ -385,6 +388,18 @@ public void setRenderDepth(int renderDepth) {
     this.renderDepth = renderDepth;
   }
 
+  public void pushRenderStack(String template) {
+    renderStack.push(template);
+  }
+
+  public String popRenderStack() {
+    return renderStack.pop();
+  }
+
+  public boolean doesRenderStackContain(String template) {
+    return renderStack.contains(template);
+  }
+
   public void addDependency(String type, String identification) {
     this.dependencies.get(type).add(identification);
   }

diff --git a/src/main/java/com/hubspot/jinjava/interpret/JinjavaInterpreter.java b/src/main/java/com/hubspot/jinjava/interpret/JinjavaInterpreter.java
@@ -45,6 +45,7 @@
 import com.hubspot.jinjava.tree.output.BlockPlaceholderOutputNode;
 import com.hubspot.jinjava.tree.output.OutputList;
 import com.hubspot.jinjava.tree.output.OutputNode;
+import com.hubspot.jinjava.tree.output.RenderedOutputNode;
 import com.hubspot.jinjava.util.Variable;
 import com.hubspot.jinjava.util.WhitespaceUtils;
 
@@ -207,8 +208,15 @@ public String render(Node root, boolean processExtendRoots) {
 
     for (Node node : root.getChildren()) {
       lineNumber = node.getLineNumber();
-      OutputNode out = node.render(this);
-      output.addNode(out);
+      if (context.doesRenderStackContain(node.getMaster().getImage())) {
+        // This is a circular rendering. Stop rendering it here.
+        output.addNode(new RenderedOutputNode(node.getMaster().getImage()));
+      } else {
+        context.pushRenderStack(node.getMaster().getImage());
+        OutputNode out = node.render(this);
+        context.popRenderStack();
+        output.addNode(out);
+      }
     }
 
     // render all extend parents, keeping the last as the root output

diff --git a/src/test/java/com/hubspot/jinjava/lib/tag/MacroTagTest.java b/src/test/java/com/hubspot/jinjava/lib/tag/MacroTagTest.java
@@ -158,7 +158,25 @@ public void itAllowsMacroRecursionWhenEnabledInConfiguration() throws IOExceptio
     }
   }
 
-
+  @Test
+  public void itPreventsRecursionForMacroWithVar() {
+    String jinja = "{%- macro func(var) %}" +
+        "{%- for f in var %}" +
+        "{{ f.val }}" +
+        "{%- endfor %}" +
+        "{%- endmacro %}" +
+        "{%- set var = {" +
+        "    'f' : {" +
+        "        'val': '{{ self }}'," +
+        "    }" +
+        "} %}" +
+        "{% set self='{{var}}' %}" +
+        "{{ func(var) }}" +
+        "";
+    Node node = new TreeParser(interpreter, jinja).buildTree();
+    assertThat(interpreter.render(node)).isEqualTo(
+        "{f={val={f={val={{ self }}}}}}");
+  }
 
   private Node snippet(String jinja) {
     return new TreeParser(interpreter, jinja).buildTree().getChildren().getFirst();

diff --git a/src/test/java/com/hubspot/jinjava/tree/ExpressionNodeTest.java b/src/test/java/com/hubspot/jinjava/tree/ExpressionNodeTest.java
@@ -67,6 +67,35 @@ public void itAvoidsInfiniteRecursionWhenVarsContainBraceBlocks() throws Excepti
     assertThat(node.render(interpreter).toString()).isEqualTo("hello {{ place }}");
   }
 
+  @Test
+  public void itDoesNotRescursivelyEvaluateExpressionsOfSelf() throws Exception {
+    context.put("myvar", "hello {{myvar}}");
+
+    ExpressionNode node = fixture("simplevar");
+    // It renders once, and then stop further rendering after detecting recursion.
+    assertThat(node.render(interpreter).toString()).isEqualTo("hello hello {{myvar}}");
+  }
+
+  @Test
+  public void itDoesNotRescursivelyEvaluateExpressions() throws Exception {
+    context.put("myvar", "hello {{ place }}");
+    context.put("place", "{{location}}");
+    context.put("location", "this is a place.");
+
+    ExpressionNode node = fixture("simplevar");
+    assertThat(node.render(interpreter).toString()).isEqualTo("hello this is a place.");
+  }
+
+  @Test
+  public void itDoesNotRescursivelyEvaluateMoreExpressions() throws Exception {
+    context.put("myvar", "hello {{ place }}");
+    context.put("place", "there, {{ location }}");
+    context.put("location", "this is {{ place }}");
+
+    ExpressionNode node = fixture("simplevar");
+    assertThat(node.render(interpreter).toString()).isEqualTo("hello there, this is {{ place }}");
+  }
+
   @Test
   public void itRendersStringRange() throws Exception {
     context.put("theString", "1234567890");