Merge pull request #363 from HubSpot/fix-relative-path-blocks

Avoid tag cycles when keeping track of parent paths for blocks
HubSpot · Aug 23, 2019 · 04f2446 · 04f2446
2 parents e867e3e + 477bebf
commit 04f2446
Show file tree

Hide file tree

Showing 9 changed files with 78 additions and 11 deletions.
diff --git a/src/main/java/com/hubspot/jinjava/interpret/JinjavaInterpreter.java b/src/main/java/com/hubspot/jinjava/interpret/JinjavaInterpreter.java
@@ -49,6 +49,7 @@
 import com.hubspot.jinjava.random.DeferredRandomNumberGenerator;
 import com.hubspot.jinjava.tree.Node;
 import com.hubspot.jinjava.tree.TreeParser;
+import com.hubspot.jinjava.tree.output.BlockInfo;
 import com.hubspot.jinjava.tree.output.BlockPlaceholderOutputNode;
 import com.hubspot.jinjava.tree.output.OutputList;
 import com.hubspot.jinjava.tree.output.OutputNode;
@@ -58,7 +59,7 @@
 
 public class JinjavaInterpreter {
 
-  private final Multimap<String, List<? extends Node>> blocks = ArrayListMultimap.create();
+  private final Multimap<String, BlockInfo> blocks = ArrayListMultimap.create();
   private final LinkedList<Node> extendParentRoots = new LinkedList<>();
 
   private Context context;
@@ -113,8 +114,8 @@ public void addExtendParentRoot(Node root) {
     extendParentRoots.add(root);
   }
 
-  public void addBlock(String name, LinkedList<? extends Node> value) {
-    blocks.put(name, value);
+  public void addBlock(String name, BlockInfo blockInfo) {
+    blocks.put(name, blockInfo);
   }
 
   /**
@@ -282,17 +283,28 @@ private void resolveBlockStubs(OutputList output, Stack<String> blockNames) {
     for (BlockPlaceholderOutputNode blockPlaceholder : output.getBlocks()) {
 
       if (!blockNames.contains(blockPlaceholder.getBlockName())) {
-        Collection<List<? extends Node>> blockChain = blocks.get(blockPlaceholder.getBlockName());
-        List<? extends Node> block = Iterables.getFirst(blockChain, null);
+        Collection<BlockInfo> blockChain = blocks.get(blockPlaceholder.getBlockName());
+        BlockInfo block = Iterables.getFirst(blockChain, null);
 
-        if (block != null) {
-          List<? extends Node> superBlock = Iterables.get(blockChain, 1, null);
+        if (block != null && block.getNodes() != null) {
+          List<? extends Node> superBlock = Optional.ofNullable(Iterables.get(blockChain, 1, null))
+              .map(BlockInfo::getNodes).orElse(null);
           context.setSuperBlock(superBlock);
 
           OutputList blockValueBuilder = new OutputList(config.getMaxOutputSize());
 
-          for (Node child : block) {
+          for (Node child : block.getNodes()) {
+            boolean pushedParentPathOntoStack = false;
+            if (block.getParentPath().isPresent() && !getContext().getCurrentPathStack().contains(block.getParentPath().get())) {
+              getContext().getCurrentPathStack().push(block.getParentPath().get(), lineNumber, position);
+              pushedParentPathOntoStack = true;
+            }
+
             blockValueBuilder.addNode(child.render(this));
+
+            if (pushedParentPathOntoStack) {
+              getContext().getCurrentPathStack().pop();
+            }
           }
 
           blockNames.push(blockPlaceholder.getBlockName());

diff --git a/src/main/java/com/hubspot/jinjava/lib/tag/BlockTag.java b/src/main/java/com/hubspot/jinjava/lib/tag/BlockTag.java
@@ -21,6 +21,7 @@
 import com.hubspot.jinjava.interpret.JinjavaInterpreter;
 import com.hubspot.jinjava.interpret.TemplateSyntaxException;
 import com.hubspot.jinjava.tree.TagNode;
+import com.hubspot.jinjava.tree.output.BlockInfo;
 import com.hubspot.jinjava.tree.output.BlockPlaceholderOutputNode;
 import com.hubspot.jinjava.tree.output.OutputNode;
 import com.hubspot.jinjava.util.HelperStringTokenizer;
@@ -57,7 +58,7 @@ public OutputNode interpretOutput(TagNode tagNode, JinjavaInterpreter interprete
 
     String blockName = WhitespaceUtils.unquote(tagData.next());
 
-    interpreter.addBlock(blockName, tagNode.getChildren());
+    interpreter.addBlock(blockName, new BlockInfo(tagNode.getChildren(), interpreter.getContext().getCurrentPathStack().peek()));
 
     return new BlockPlaceholderOutputNode(blockName);
   }

diff --git a/src/main/java/com/hubspot/jinjava/tree/output/BlockInfo.java b/src/main/java/com/hubspot/jinjava/tree/output/BlockInfo.java
@@ -0,0 +1,27 @@
+package com.hubspot.jinjava.tree.output;
+
+import java.util.List;
+import java.util.Optional;
+
+import com.hubspot.jinjava.tree.Node;
+
+@SuppressWarnings("OptionalUsedAsFieldOrParameterType")
+public class BlockInfo {
+
+  private final List<? extends Node> nodes;
+
+  private final Optional<String> parentPath;
+
+  public BlockInfo(List<? extends Node> nodes, Optional<String> parentPath) {
+    this.nodes = nodes;
+    this.parentPath = parentPath;
+  }
+
+  public List<? extends Node> getNodes() {
+    return nodes;
+  }
+
+  public Optional<String> getParentPath() {
+    return parentPath;
+  }
+}
diff --git a/src/test/java/com/hubspot/jinjava/interpret/JinjavaInterpreterTest.java b/src/test/java/com/hubspot/jinjava/interpret/JinjavaInterpreterTest.java
@@ -4,6 +4,7 @@
 
 import java.time.ZonedDateTime;
 import java.util.HashMap;
+import java.util.Optional;
 
 import org.junit.Before;
 import org.junit.Test;
@@ -14,6 +15,7 @@
 import com.hubspot.jinjava.interpret.JinjavaInterpreter.InterpreterScopeClosable;
 import com.hubspot.jinjava.interpret.TemplateError.ErrorReason;
 import com.hubspot.jinjava.tree.TextNode;
+import com.hubspot.jinjava.tree.output.BlockInfo;
 import com.hubspot.jinjava.tree.parse.TextToken;
 
 public class JinjavaInterpreterTest {
@@ -40,14 +42,16 @@ public void resolveBlockStubsWithMissingNamedBlock() {
 
   @Test
   public void resolveBlockStubs() {
-    interpreter.addBlock("foobar", Lists.newLinkedList(Lists.newArrayList((new TextNode(new TextToken("sparta", -1, -1))))));
+    interpreter.addBlock("foobar", new BlockInfo(Lists.newLinkedList(Lists.newArrayList((new TextNode(new TextToken("sparta", -1, -1))))),
+        Optional.empty()));
     String content = "this is {% block foobar %}foobar{% endblock %}!";
     assertThat(interpreter.render(content)).isEqualTo("this is sparta!");
   }
 
   @Test
   public void resolveBlockStubsWithSpecialChars() {
-    interpreter.addBlock("foobar", Lists.newLinkedList(Lists.newArrayList(new TextNode(new TextToken("$150.00", -1, -1)))));
+    interpreter.addBlock("foobar", new BlockInfo(Lists.newLinkedList(Lists.newArrayList(new TextNode(new TextToken("$150.00", -1, -1)))),
+        Optional.empty()));
     String content = "this is {% block foobar %}foobar{% endblock %}!";
     assertThat(interpreter.render(content)).isEqualTo("this is $150.00!");
   }

diff --git a/src/test/java/com/hubspot/jinjava/lib/tag/ExtendsTagTest.java b/src/test/java/com/hubspot/jinjava/lib/tag/ExtendsTagTest.java
@@ -113,6 +113,20 @@ public void itExtendsViaRelativePath() throws IOException {
     assertThat(result).contains("This is a relative path extends");
   }
 
+  @Test
+  public void itHandlesRelativePathsInBlocksInExtendingTemplate() throws IOException {
+    jinjava.getGlobalContext().put(CURRENT_PATH_CONTEXT_KEY, "relative/relative-block.jinja");
+    String result = jinjava.render(locator.fixture("relative/relative-block.jinja"), new HashMap<>());
+    assertThat(result).contains("hello");
+  }
+
+  @Test
+  public void itHandlesRelativePathsInBlocksFromExtendedTemplate() throws IOException {
+    jinjava.getGlobalContext().put(CURRENT_PATH_CONTEXT_KEY, "relative/relative-block-2.jinja");
+    String result = jinjava.render(locator.fixture("relative/relative-block-2.jinja"), new HashMap<>());
+    assertThat(result).contains("hello");
+  }
+
   private static class ExtendsTagTestResourceLocator implements ResourceLocator {
     private RelativePathResolver relativePathResolver = new RelativePathResolver();
 

diff --git a/src/test/resources/tags/extendstag/hello.html b/src/test/resources/tags/extendstag/hello.html
@@ -0,0 +1 @@
+hello
diff --git a/src/test/resources/tags/extendstag/relative-block-2-base.jinja b/src/test/resources/tags/extendstag/relative-block-2-base.jinja
@@ -0,0 +1,3 @@
+{% block test %}
+{% include "./hello.html" %}
+{% endblock test %}
diff --git a/src/test/resources/tags/extendstag/relative/relative-block-2.jinja b/src/test/resources/tags/extendstag/relative/relative-block-2.jinja
@@ -0,0 +1 @@
+{% extends "./../relative-block-2-base.jinja" %}
diff --git a/src/test/resources/tags/extendstag/relative/relative-block.jinja b/src/test/resources/tags/extendstag/relative/relative-block.jinja
@@ -0,0 +1,4 @@
+{% extends "./../super-base.html" %}
+{% block sidebar %}
+{% include "./../hello.html" %}
+{% endblock %}