Merge pull request #1112 from HubSpot/hashcode-stack-overflow

Use a ThreadLocal semaphore to prevent hashCode recursion
HubSpot · Aug 24, 2023 · 15f33ad · 15f33ad
2 parents b964022 + 9607209
commit 15f33ad
Show file tree

Hide file tree

Showing 4 changed files with 127 additions and 6 deletions.
diff --git a/src/main/java/com/hubspot/jinjava/objects/collections/PyList.java b/src/main/java/com/hubspot/jinjava/objects/collections/PyList.java
@@ -9,6 +9,7 @@
 import java.util.Objects;
 
 public class PyList extends ForwardingList<Object> implements PyWrapper {
+  private boolean computingHashCode = false;
   private final List<Object> list;
 
   public PyList(List<Object> list) {
@@ -99,4 +100,21 @@ IndexOutOfRangeException createOutOfRangeException(int index) {
       String.format("Index %d is out of range for list of size %d", index, list.size())
     );
   }
+
+  /**
+   * This is not thread-safe
+   * @return hashCode, preventing recursion
+   */
+  @Override
+  public int hashCode() {
+    if (computingHashCode) {
+      return Objects.hashCode(null);
+    }
+    try {
+      computingHashCode = true;
+      return super.hashCode();
+    } finally {
+      computingHashCode = false;
+    }
+  }
 }
diff --git a/src/main/java/com/hubspot/jinjava/objects/collections/PyMap.java b/src/main/java/com/hubspot/jinjava/objects/collections/PyMap.java
@@ -3,9 +3,12 @@
 import com.google.common.collect.ForwardingMap;
 import com.hubspot.jinjava.objects.PyWrapper;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
 
 public class PyMap extends ForwardingMap<String, Object> implements PyWrapper {
+  private boolean computingHashCode = false;
+
   private final Map<String, Object> map;
 
   public PyMap(Map<String, Object> map) {
@@ -59,14 +62,20 @@ public void putAll(Map<? extends String, ? extends Object> m) {
     super.putAll(m);
   }
 
+  /**
+   * This is not thread-safe
+   * @return hashCode, preventing recursion
+   */
   @Override
   public int hashCode() {
-    int h = 0;
-    for (Entry<String, Object> entry : map.entrySet()) {
-      if (entry.getValue() != map && entry.getValue() != this) {
-        h += entry.hashCode();
-      }
+    if (computingHashCode) {
+      return Objects.hashCode(null);
+    }
+    try {
+      computingHashCode = true;
+      return super.hashCode();
+    } finally {
+      computingHashCode = false;
     }
-    return h;
   }
 }
diff --git a/src/test/java/com/hubspot/jinjava/objects/collections/PyListTest.java b/src/test/java/com/hubspot/jinjava/objects/collections/PyListTest.java
@@ -6,6 +6,7 @@
 import com.hubspot.jinjava.interpret.IndexOutOfRangeException;
 import com.hubspot.jinjava.interpret.RenderResult;
 import com.hubspot.jinjava.interpret.TemplateError;
+import java.util.ArrayList;
 import java.util.Collections;
 import org.junit.Test;
 
@@ -261,4 +262,38 @@ public void itDisallowsAppendingSelf() {
       )
       .isEqualTo("[1, 2]");
   }
+
+  @Test
+  public void itComputesHashCodeWhenListContainsItself() {
+    PyList list1 = new PyList(new ArrayList<>());
+    PyList list2 = new PyList(new ArrayList<>());
+    list1.add(list2);
+    int initialHashCode = list1.hashCode();
+    list2.add(list1);
+    int hashCodeWithInfiniteRecursion = list1.hashCode();
+    assertThat(initialHashCode).isNotEqualTo(hashCodeWithInfiniteRecursion);
+    assertThat(list1.hashCode())
+      .isEqualTo(hashCodeWithInfiniteRecursion)
+      .describedAs("Hash code should be consistent on multiple calls");
+    assertThat(list2.hashCode())
+      .isEqualTo(list1.hashCode())
+      .describedAs(
+        "The two lists are currently the same as they are both a list1 of a single infinitely recurring list"
+      );
+    list1.add(123456);
+    assertThat(list2.hashCode())
+      .isNotEqualTo(list1.hashCode())
+      .describedAs(
+        "The two lists are no longer the same as list1 has 2 elements while list2 has one"
+      );
+    PyList copy = list1.copy();
+    assertThat(copy.hashCode())
+      .isNotEqualTo(list1.hashCode())
+      .describedAs(
+        "copy is not the same as list1 because it is a list of a list of recursion, whereas list1 is a list of recursion"
+      );
+    assertThat(list1.copy().hashCode())
+      .isEqualTo(copy.hashCode())
+      .describedAs("All copies should have the same hash code");
+  }
 }
diff --git a/src/test/java/com/hubspot/jinjava/objects/collections/PyMapTest.java b/src/test/java/com/hubspot/jinjava/objects/collections/PyMapTest.java
@@ -3,13 +3,15 @@
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
+import com.google.common.collect.ImmutableList;
 import com.hubspot.jinjava.BaseJinjavaTest;
 import com.hubspot.jinjava.Jinjava;
 import com.hubspot.jinjava.JinjavaConfig;
 import com.hubspot.jinjava.LegacyOverrides;
 import com.hubspot.jinjava.interpret.IndexOutOfRangeException;
 import com.hubspot.jinjava.interpret.RenderResult;
 import com.hubspot.jinjava.interpret.TemplateError;
+import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
 import org.junit.Test;
@@ -367,4 +369,61 @@ public void itUpdatesKeysWithVariableName() {
       )
       .isEqualTo("value2");
   }
+
+  @Test
+  public void itComputesHashCodeWhenContainedWithinItself() {
+    PyMap map = new PyMap(new HashMap<>());
+    map.put("map1key1", "value1");
+
+    PyMap map2 = new PyMap(new HashMap<>());
+    map2.put("map2key1", map);
+
+    map.put("map1key2", map2);
+
+    assertThat(map.hashCode()).isNotEqualTo(0);
+  }
+
+  @Test
+  public void itComputesHashCodeWhenContainedWithinItselfWithFurtherEntries() {
+    PyMap map = new PyMap(new HashMap<>());
+    map.put("map1key1", "value1");
+
+    PyMap map2 = new PyMap(new HashMap<>());
+    map2.put("map2key1", map);
+
+    map.put("map1key2", map2);
+
+    int originalHashCode = map.hashCode();
+    map2.put("newKey", "newValue");
+    int newHashCode = map.hashCode();
+    assertThat(originalHashCode).isNotEqualTo(newHashCode);
+  }
+
+  @Test
+  public void itComputesHashCodeWhenContainedWithinItselfInsideList() {
+    PyMap map = new PyMap(new HashMap<>());
+    map.put("map1key1", "value1");
+
+    PyMap map2 = new PyMap(new HashMap<>());
+    map2.put("map2key1", map);
+
+    map.put("map1key2", new PyList(ImmutableList.of((map2))));
+
+    assertThat(map.hashCode()).isNotEqualTo(0);
+  }
+
+  @Test
+  public void itComputesHashCodeWithNullKeysAndValues() {
+    PyMap map = new PyMap(new HashMap<>());
+    map.put(null, "value1");
+
+    PyMap map2 = new PyMap(new HashMap<>());
+    map2.put("map2key1", map);
+
+    PyList list = new PyList(new ArrayList<>());
+    list.add(null);
+    map.put("map1key2", new PyList(list));
+
+    assertThat(map.hashCode()).isNotEqualTo(0);
+  }
 }