From b596a1cdaacd2d26634d6b178af20f59ea2e76ba Mon Sep 17 00:00:00 2001 From: Libo Song Date: Mon, 14 Aug 2017 17:13:37 -0400 Subject: [PATCH 1/3] Prevent recursion in Jinjava. --- .../hubspot/jinjava/interpret/Context.java | 15 ++++++++++ .../jinjava/interpret/JinjavaInterpreter.java | 12 ++++++-- .../hubspot/jinjava/lib/tag/MacroTagTest.java | 22 +++++++++++++- .../jinjava/tree/ExpressionNodeTest.java | 29 +++++++++++++++++++ 4 files changed, 75 insertions(+), 3 deletions(-) diff --git a/src/main/java/com/hubspot/jinjava/interpret/Context.java b/src/main/java/com/hubspot/jinjava/interpret/Context.java index c75f2eadf..32735cb96 100644 --- 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 superBlock; + private final Stack 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 Stack getRenderStack() { + return renderStack; + } + 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 index 869328296..aed8f700e 100644 --- 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.getRenderStack().contains(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 index be4f4d3a8..4ac1258db 100644 --- 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,27 @@ public void itAllowsMacroRecursionWhenEnabledInConfiguration() throws IOExceptio } } - + @Test + public void itPreventsRecursionForMacroWithVar() { + String jinja = "{%- macro allSpans(spans) %}" + + "{%- for span in spans %}" + + "{{ span.tag }}" + + "{%- endfor %}" + + "{%- endmacro %}" + + "{%- set spans = {" + + " 'html_1' : {" + + " 'tag' : 'html {{ selector }}'," + + " 'span' : 12" + + " }" + + "} %}" + + "{% set selector='{{spans}}' %}" + + "{{ allSpans(spans, '') }}" + + ""; + Node node = new TreeParser(interpreter, jinja).buildTree(); + assertThat(JinjavaInterpreter.getCurrent() == interpreter).isTrue(); + assertThat(interpreter.render(node)).isEqualTo( + "html {html_1={tag=html {html_1={tag=html {{ selector }}, span=12}}, span=12}}"); + } 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 index d98ce4946..d38cc9f1d 100644 --- 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 itAvoidsInfiniteRecursionOfItself() 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 itNoRecursionHere() 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 itAvoidsInfiniteRecursion() 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"); From b6c407384dfd720746f0a354f09811ebd20433b9 Mon Sep 17 00:00:00 2001 From: Jeff Boulter Date: Tue, 15 Aug 2017 11:24:58 -0400 Subject: [PATCH 2/3] update test names --- .../java/com/hubspot/jinjava/tree/ExpressionNodeTest.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test/java/com/hubspot/jinjava/tree/ExpressionNodeTest.java b/src/test/java/com/hubspot/jinjava/tree/ExpressionNodeTest.java index d38cc9f1d..559fa4bf3 100644 --- a/src/test/java/com/hubspot/jinjava/tree/ExpressionNodeTest.java +++ b/src/test/java/com/hubspot/jinjava/tree/ExpressionNodeTest.java @@ -68,7 +68,7 @@ public void itAvoidsInfiniteRecursionWhenVarsContainBraceBlocks() throws Excepti } @Test - public void itAvoidsInfiniteRecursionOfItself() throws Exception { + public void itDoesNotRescursivelyEvaluateExpressionsOfSelf() throws Exception { context.put("myvar", "hello {{myvar}}"); ExpressionNode node = fixture("simplevar"); @@ -77,7 +77,7 @@ public void itAvoidsInfiniteRecursionOfItself() throws Exception { } @Test - public void itNoRecursionHere() throws Exception { + public void itDoesNotRescursivelyEvaluateExpressions() throws Exception { context.put("myvar", "hello {{ place }}"); context.put("place", "{{location}}"); context.put("location", "this is a place."); @@ -87,7 +87,7 @@ public void itNoRecursionHere() throws Exception { } @Test - public void itAvoidsInfiniteRecursion() throws Exception { + public void itDoesNotRescursivelyEvaluateMoreExpressions() throws Exception { context.put("myvar", "hello {{ place }}"); context.put("place", "there, {{ location }}"); context.put("location", "this is {{ place }}"); From 6b48a8ffffd369a206102657674621c8cfc688a1 Mon Sep 17 00:00:00 2001 From: Libo Song Date: Tue, 15 Aug 2017 11:52:54 -0400 Subject: [PATCH 3/3] Encapsulate the stack, simplified the test case. --- .../hubspot/jinjava/interpret/Context.java | 4 ++-- .../jinjava/interpret/JinjavaInterpreter.java | 2 +- .../hubspot/jinjava/lib/tag/MacroTagTest.java | 20 +++++++++---------- 3 files changed, 12 insertions(+), 14 deletions(-) diff --git a/src/main/java/com/hubspot/jinjava/interpret/Context.java b/src/main/java/com/hubspot/jinjava/interpret/Context.java index 32735cb96..3992b1e50 100644 --- a/src/main/java/com/hubspot/jinjava/interpret/Context.java +++ b/src/main/java/com/hubspot/jinjava/interpret/Context.java @@ -396,8 +396,8 @@ public String popRenderStack() { return renderStack.pop(); } - public Stack getRenderStack() { - return renderStack; + public boolean doesRenderStackContain(String template) { + return renderStack.contains(template); } public void addDependency(String type, String identification) { diff --git a/src/main/java/com/hubspot/jinjava/interpret/JinjavaInterpreter.java b/src/main/java/com/hubspot/jinjava/interpret/JinjavaInterpreter.java index aed8f700e..fc72e9446 100644 --- a/src/main/java/com/hubspot/jinjava/interpret/JinjavaInterpreter.java +++ b/src/main/java/com/hubspot/jinjava/interpret/JinjavaInterpreter.java @@ -208,7 +208,7 @@ public String render(Node root, boolean processExtendRoots) { for (Node node : root.getChildren()) { lineNumber = node.getLineNumber(); - if (context.getRenderStack().contains(node.getMaster().getImage())) { + if (context.doesRenderStackContain(node.getMaster().getImage())) { // This is a circular rendering. Stop rendering it here. output.addNode(new RenderedOutputNode(node.getMaster().getImage())); } else { diff --git a/src/test/java/com/hubspot/jinjava/lib/tag/MacroTagTest.java b/src/test/java/com/hubspot/jinjava/lib/tag/MacroTagTest.java index 4ac1258db..37c5d8c8e 100644 --- a/src/test/java/com/hubspot/jinjava/lib/tag/MacroTagTest.java +++ b/src/test/java/com/hubspot/jinjava/lib/tag/MacroTagTest.java @@ -160,24 +160,22 @@ public void itAllowsMacroRecursionWhenEnabledInConfiguration() throws IOExceptio @Test public void itPreventsRecursionForMacroWithVar() { - String jinja = "{%- macro allSpans(spans) %}" + - "{%- for span in spans %}" + - "{{ span.tag }}" + + String jinja = "{%- macro func(var) %}" + + "{%- for f in var %}" + + "{{ f.val }}" + "{%- endfor %}" + "{%- endmacro %}" + - "{%- set spans = {" + - " 'html_1' : {" + - " 'tag' : 'html {{ selector }}'," + - " 'span' : 12" + + "{%- set var = {" + + " 'f' : {" + + " 'val': '{{ self }}'," + " }" + "} %}" + - "{% set selector='{{spans}}' %}" + - "{{ allSpans(spans, '') }}" + + "{% set self='{{var}}' %}" + + "{{ func(var) }}" + ""; Node node = new TreeParser(interpreter, jinja).buildTree(); - assertThat(JinjavaInterpreter.getCurrent() == interpreter).isTrue(); assertThat(interpreter.render(node)).isEqualTo( - "html {html_1={tag=html {html_1={tag=html {{ selector }}, span=12}}, span=12}}"); + "{f={val={f={val={{ self }}}}}}"); } private Node snippet(String jinja) {