Skip to content

Commit 3703851

Browse files
committed
Use isResolvableObject instead of contains before trying to build
hashcode 1. using `contains` can cause a StackOverflowError 2. If the depth grows very deep, running hashCode can be quite slow, using isResolvableObject will break out before going into too much depth and we won't use hashCode In cases where we can't use the hashCode of such objects, we just aren't able to check for changes to them
1 parent 0e5510e commit 3703851

4 files changed

Lines changed: 29 additions & 4 deletions

File tree

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

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -123,8 +123,7 @@ private static Map<String, String> getInitiallyResolvedAsStrings(
123123
? entrySet
124124
: interpreter.getContext().getCombinedScope().entrySet()).stream()
125125
.filter(entry -> initiallyResolvedHashes.containsKey(entry.getKey()))
126-
.filter(entry ->
127-
EagerExpressionResolver.isResolvableObject(entry.getValue(), 4, 400) // TODO make this configurable
126+
.filter(entry -> isResolvableForContextReverting(entry.getValue()) // TODO make this configurable
128127
);
129128
entryStream.forEach(entry ->
130129
cacheRevertibleObject(
@@ -392,15 +391,19 @@ private static Object getObjectOrHashCode(Object o) {
392391
o = ((LazyExpression) o).get();
393392
}
394393

395-
if (o instanceof PyList && !((PyList) o).toList().contains(o)) {
394+
if (o instanceof PyList && isResolvableForContextReverting(o)) {
396395
return o.hashCode();
397396
}
398-
if (o instanceof PyMap && !((PyMap) o).toMap().containsValue(o)) {
397+
if (o instanceof PyMap && isResolvableForContextReverting(o)) {
399398
return o.hashCode() + ((PyMap) o).keySet().hashCode();
400399
}
401400
return o;
402401
}
403402

403+
private static boolean isResolvableForContextReverting(Object o) {
404+
return EagerExpressionResolver.isResolvableObject(o, 4, 400);
405+
}
406+
404407
public static class EagerChildContextConfig {
405408

406409
private final boolean takeNewValue;

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1740,4 +1740,11 @@ public void itHandlesModifiedIncludePathSecondPass() {
17401740
"handles-modified-include-path/test.expected"
17411741
);
17421742
}
1743+
1744+
@Test
1745+
public void itDoesNotStackOverflowTryingToBuildHashcode() {
1746+
expectedTemplateInterpreter.assertExpectedOutput(
1747+
"does-not-stack-overflow-trying-to-build-hashcode/test"
1748+
);
1749+
}
17431750
}
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
{% for i in deferred %}
2+
hey
3+
{% endfor %}
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
{% set l1000_1 = [] %}
2+
{% set l1000_2 = [] %}
3+
{% set l1000_3 = [] %}
4+
5+
{% do l1000_1.append(l1000_2) %}
6+
{% do l1000_2.append(l1000_3) %}
7+
{% do l1000_3.append(l1000_1) %}
8+
9+
10+
{% for i in deferred %}
11+
{{ 'hey' }}
12+
{% endfor %}

0 commit comments

Comments
 (0)