Inline partial leak fix #655

Author: jknack

handlebars/src/main/java/com/github/jknack/handlebars/internal/BaseTemplate.java

@@ -214,7 +214,7 @@ public String filename() {
 
   @Override
   public int[] position() {
-    return new int[]{line, column };
+    return new int[]{line, column};
   }
 
   /**
@@ -255,32 +255,32 @@ public <T> TypeSafeTemplate<T> as() {
    * @return A new {@link TypeSafeTemplate}.
    */
   private static Object newTypeSafeTemplate(final Class<?> rootType, final Template template) {
-    return Proxy.newProxyInstance(rootType.getClassLoader(), new Class[]{rootType },
+    return Proxy.newProxyInstance(rootType.getClassLoader(), new Class[]{rootType},
         new InvocationHandler() {
 
           private final Map<String, Object> attributes = new HashMap<>();
           private final Object[] emptyArgs = {};
 
           private boolean isDefault(final Method method) {
             return ((method.getModifiers()
-                    & (Modifier.ABSTRACT | Modifier.PUBLIC | Modifier.STATIC)) == Modifier.PUBLIC)
-                    && method.getDeclaringClass().isInterface();
+                & (Modifier.ABSTRACT | Modifier.PUBLIC | Modifier.STATIC)) == Modifier.PUBLIC)
+                && method.getDeclaringClass().isInterface();
           }
 
           private Object invokeDefaultMethod(final Method method, final Class<?> lookupClass,
-                                             final Object proxy, final Object ... args)
-                  throws Throwable {
+              final Object proxy, final Object... args)
+              throws Throwable {
             // Jumping through these hoops is needed because calling unreflectSpecial requires that
             // the lookup instance have private access to the special caller. None of the static
             // factory methods for Lookup will give us an instance with the access modes we need,
             // so we work around it by calling the private constructor via reflection.
             Constructor<MethodHandles.Lookup> constructor = MethodHandles.Lookup.class
-                    .getDeclaredConstructor(Class.class, int.class);
+                .getDeclaredConstructor(Class.class, int.class);
             constructor.setAccessible(true);
             return constructor.newInstance(lookupClass, -1 /* trusted */)
-                    .unreflectSpecial(method, lookupClass)
-                    .bindTo(proxy)
-                    .invokeWithArguments(args);
+                .unreflectSpecial(method, lookupClass)
+                .bindTo(proxy)
+                .invokeWithArguments(args);
           }
 
           @Override
@@ -304,8 +304,8 @@ public Object invoke(final Object proxy, final Method method, final Object[] met
             }
 
             if (args.length == 1
-                    && methodName.equals("equals")
-                    && method.getParameterTypes()[0] == Object.class) {
+                && methodName.equals("equals")
+                && method.getParameterTypes()[0] == Object.class) {
               return args[0] == proxy;
             }
 
@@ -374,11 +374,11 @@ protected void collectReferenceParameters(final Collection<String> result) {
 
   @Override
   public String toJavaScript() {
-      if (javaScript == null) {
-        javaScript = handlebars.precompileInline(text());
-      }
-      return javaScript;
+    if (javaScript == null) {
+      javaScript = handlebars.precompileInline(text());
     }
+    return javaScript;
+  }
 
   /**
    * @return True if this template has decorators.

handlebars/src/main/java/com/github/jknack/handlebars/internal/Partial.java

@@ -35,6 +35,8 @@
 import com.github.jknack.handlebars.Handlebars;
 import com.github.jknack.handlebars.HandlebarsError;
 import com.github.jknack.handlebars.HandlebarsException;
+import com.github.jknack.handlebars.PathCompiler;
+import com.github.jknack.handlebars.PathExpression;
 import com.github.jknack.handlebars.Template;
 import com.github.jknack.handlebars.io.TemplateLoader;
 import com.github.jknack.handlebars.io.TemplateSource;
@@ -50,6 +52,12 @@
  */
 class Partial extends HelperResolver {
 
+  /** Special hash/map properties that we need to override. */
+  private static final List<List<PathExpression>> OVERRIDE_PROPERTIES = Arrays.asList(
+      PathCompiler.compile("size"),
+      PathCompiler.compile("empty")
+  );
+
   /**
    * The partial path.
    */
@@ -84,6 +92,9 @@ class Partial extends HelperResolver {
   /** A partial block body. */
   private Template partial;
 
+  /** Used to clear context after partial block got executed. See #655. */
+  private boolean decorate;
+
   /**
    * Creates a new {@link Partial}.
    *
@@ -111,7 +122,23 @@ public void before(final Context context, final Writer writer) throws IOExceptio
   @Override
   public void after(final Context context, final Writer writer) throws IOException {
     LinkedList<Map<String, Template>> partials = context.data(Context.INLINE_PARTIALS);
-    partials.removeLast();
+    if (partials.size() > 0) {
+      partials.removeLast();
+    }
+  }
+
+  @Override public boolean decorate() {
+    return decorate;
+  }
+
+  /**
+   * Used to clear context after partial block got executed. See #655
+   * @param decorate True to clear context.
+   * @return This partial.
+   */
+  Partial setDecorate(final boolean decorate) {
+    this.decorate = decorate;
+    return this;
   }
 
   @Override
@@ -130,7 +157,7 @@ protected void merge(final Context context, final Writer writer)
 
       if (pathIsPartialBlock && parentIsNotLastPartialBlock) {
         throw new IllegalArgumentException(
-                callee + " does not provide a @partial-block for " + this
+            callee + " does not provide a @partial-block for " + this
         );
       }
 
@@ -141,10 +168,10 @@ protected void merge(final Context context, final Writer writer)
 
         inlineTemplates.put("@partial-block",
             new PartialBlockForwardingTemplate(this,
-              this.partial,
-              inlineTemplates.get("@partial-block"),
-              callee,
-              handlebars
+                this.partial,
+                inlineTemplates.get("@partial-block"),
+                callee,
+                handlebars
             )
         );
       }
@@ -192,13 +219,13 @@ protected void merge(final Context context, final Writer writer)
             throw fnf;
           }
         }
-
+      } else {
+        partials.removeLast();
       }
       context.data(Context.CALLEE, this);
       Map<String, Object> hash = hash(context);
       // HACK: hide/override local attribute with parent version (if any)
-      hash.put("size", context.get("size"));
-      hash.put("empty", context.get("empty"));
+      override(context, hash, OVERRIDE_PROPERTIES);
       Context ctx = Context.newPartialContext(context, this.scontext, hash);
       template.apply(ctx, writer);
       context.data(Context.CALLEE, callee);
@@ -212,6 +239,24 @@ protected void merge(final Context context, final Writer writer)
     }
   }
 
+  /**
+   * Check for property conflicts and resolve them. Basically we ignore internal properties from
+   * Map if they already present in current context.
+   *
+   * @param context Current context.
+   * @param hash Partial context.
+   * @param properties Property to check for.
+   */
+  private void override(final Context context, final Map<String, Object> hash,
+      final List<List<PathExpression>> properties) {
+    for (List<PathExpression> path : properties) {
+      String key = path.toString();
+      if (!hash.containsKey(key)) {
+        hash.put(key, context.get(path));
+      }
+    }
+  }
+
   /**
    * @param callee parent template of the currently traversed template
    * @param partialBlock partial block candidate

handlebars/src/main/java/com/github/jknack/handlebars/internal/TemplateBuilder.java

@@ -594,6 +594,7 @@ public Object visitPartialBlock(final PartialBlockContext ctx) {
 
     String startDelim = ctx.start.getText();
     Template partial = new Partial(handlebars, info.path, info.context, info.hash)
+        .setDecorate(true)
         .setPartial(fn)
         .startDelimiter(startDelim.substring(0, startDelim.length() - 1))
         .endDelimiter(ctx.stop.getText())

handlebars/src/test/java/com/github/jknack/handlebars/issues/Issue643.java

@@ -10,8 +10,8 @@ public void shouldAllowES6LetOrConstLiterals() throws Exception {
     shouldCompileTo("template: {{empty}} "
             + "{{> partial}}",
         $("partials", $("partial", "partial: {{empty}}"),
-            "data", $("empty", false)),
-        "template: true partial: true");
+            "hash", $("empty", false)),
+        "template: false partial: false");
   }
 
 }

handlebars/src/test/java/com/github/jknack/handlebars/issues/Issue655.java

@@ -0,0 +1,51 @@
+package com.github.jknack.handlebars.issues;
+
+import com.github.jknack.handlebars.v4Test;
+import org.junit.Test;
+
+public class Issue655 extends v4Test {
+
+  @Test
+  public void inlinePartialsLeak() throws Exception {
+    shouldCompileTo("main has partials:<br>\n"
+            + "-------------<br>\n"
+            + "{{>inherit1}}\n"
+            + "-------------<br>\n"
+            + "{{>inherit2}}",
+        $("hash", $,
+            "partials",
+            $(
+                "base",
+                "text from base partial<br>\n"
+                    + "{{#>inlinePartial}}{{/inlinePartial}}<br>\n"
+                    + "{{#>inlinePartial2}}{{/inlinePartial2}}<br>",
+                "inherit1", "inherit1<br>\n"
+                    + "{{#>base}}\n"
+                    + "    {{#*inline \"inlinePartial\"}}\n"
+                    + "        inline partial defined by inherit1, called from base\n"
+                    + "    {{/inline}}\n"
+                    + "    {{#*inline \"inlinePartial2\"}}\n"
+                    + "        {{>some-other-template}}\n"
+                    + "    {{/inline}}\n"
+                    + "{{/base}}",
+                "inherit2", "inherit2<br>\n"
+                    + "{{#>base}}\n"
+                    + "{{/base}}",
+                "some-other-template", "template called from second inline partial of inherit 1")
+        ),
+        "main has partials:<br>\n"
+            + "-------------<br>\n"
+            + "inherit1<br>\n"
+            + "text from base partial<br>\n"
+            + "\n"
+            + "        inline partial defined by inherit1, called from base\n"
+            + "    <br>\n"
+            + "\n"
+            + "        template called from second inline partial of inherit 1\n"
+            + "    <br>\n"
+            + "-------------<br>\n"
+            + "inherit2<br>\n"
+            + "text from base partial<br>\n"
+            + "<br>\n<br>");
+  }
+}