Merge pull request #1286 from HubSpot/use-is-resolvable-instead

fix(eager): Use isResolvableObject instead of contains before trying to build hashcode

Author: jasmith-hs

src/main/java/com/hubspot/jinjava/util/EagerContextWatcher.java

@@ -123,8 +123,7 @@ private static Map<String, String> getInitiallyResolvedAsStrings(
           ? entrySet
           : interpreter.getContext().getCombinedScope().entrySet()).stream()
         .filter(entry -> initiallyResolvedHashes.containsKey(entry.getKey()))
-        .filter(entry ->
-          EagerExpressionResolver.isResolvableObject(entry.getValue(), 4, 400) // TODO make this configurable
+        .filter(entry -> isResolvableForContextReverting(entry.getValue()) // TODO make this configurable
         );
     entryStream.forEach(entry ->
       cacheRevertibleObject(
@@ -392,15 +391,19 @@ private static Object getObjectOrHashCode(Object o) {
       o = ((LazyExpression) o).get();
     }
 
-    if (o instanceof PyList && !((PyList) o).toList().contains(o)) {
+    if (o instanceof PyList && isResolvableForContextReverting(o)) {
       return o.hashCode();
     }
-    if (o instanceof PyMap && !((PyMap) o).toMap().containsValue(o)) {
+    if (o instanceof PyMap && isResolvableForContextReverting(o)) {
       return o.hashCode() + ((PyMap) o).keySet().hashCode();
     }
     return o;
   }
 
+  private static boolean isResolvableForContextReverting(Object o) {
+    return EagerExpressionResolver.isResolvableObject(o, 4, 400);
+  }
+
   public static class EagerChildContextConfig {
 
     private final boolean takeNewValue;

src/test/java/com/hubspot/jinjava/EagerTest.java

@@ -1741,6 +1741,13 @@ public void itHandlesModifiedIncludePathSecondPass() {
     );
   }
 
+  @Test
+  public void itDoesNotStackOverflowTryingToBuildHashcode() {
+    expectedTemplateInterpreter.assertExpectedOutput(
+      "does-not-stack-overflow-trying-to-build-hashcode/test"
+    );
+  }
+
   @Test
   public void itHandlesDeferredValueInRenderFilter() {
     expectedTemplateInterpreter.assertExpectedOutput(

src/test/resources/eager/does-not-stack-overflow-trying-to-build-hashcode/test.expected.jinja

@@ -0,0 +1,3 @@
+{% for i in deferred %}
+hey
+{% endfor %}

src/test/resources/eager/does-not-stack-overflow-trying-to-build-hashcode/test.jinja

@@ -0,0 +1,12 @@
+{% set l1000_1 = [] %}
+{% set l1000_2 = [] %}
+{% set l1000_3 = [] %}
+
+{% do l1000_1.append(l1000_2) %}
+{% do l1000_2.append(l1000_3) %}
+{% do l1000_3.append(l1000_1) %}
+
+
+{% for i in deferred %}
+{{ 'hey' }}
+{% endfor %}