Skip to content

Commit 1598751

Browse files
jasmith-hsjasmith-hs
committed
Don't naively swallow DeferredValueExceptions when partially reconstructing EL expressions.
These are generally unexpected, if they happen it is due to a value not being reconstructible and should result in a Deferred Node
1 parent 4a8c8df commit 1598751

9 files changed

Lines changed: 220 additions & 54 deletions

File tree

‎src/main/java/com/hubspot/jinjava/el/ext/eager/EvalResultHolder.java‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,23 @@ static String reconstructNode(
120120
}
121121
try {
122122
return EagerExpressionResolver.getValueAsJinjavaStringSafe(evalResult);
123-
} catch (DeferredValueException ignored) {}
123+
} catch (DeferredValueException e) {
124+
if (astNode instanceof AstIdentifier) {
125+
String name = ((AstIdentifier) astNode).getName();
126+
if (
127+
(
128+
(JinjavaInterpreter) context
129+
.getELResolver()
130+
.getValue(context, null, ExtendedParser.INTERPRETER)
131+
).getContext()
132+
.getMetaContextVariables()
133+
.contains(name)
134+
) {
135+
return name;
136+
}
137+
throw e;
138+
}
139+
}
124140
}
125141
return astNode.getPartiallyResolved(
126142
bindings,

‎src/main/java/com/hubspot/jinjava/interpret/ReconstructiblePartiallyDeferredValue.java‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
package com.hubspot.jinjava.interpret;
2+
3+
import com.hubspot.jinjava.objects.serialization.PyishSerializable;
4+
import java.io.IOException;
5+
6+
public interface ReconstructiblePartiallyDeferredValue
7+
extends PartiallyDeferredValue, PyishSerializable {
8+
/**
9+
* This method should not throw any RuntimeExceptions
10+
*/
11+
@Override
12+
<T extends Appendable & CharSequence> T appendPyishString(T appendable)
13+
throws IOException;
14+
}

‎src/main/java/com/hubspot/jinjava/objects/serialization/PyishObjectMapper.java‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import com.fasterxml.jackson.databind.SerializerProvider;
1111
import com.fasterxml.jackson.databind.module.SimpleModule;
1212
import com.google.common.annotations.Beta;
13+
import com.hubspot.jinjava.interpret.DeferredValueException;
1314
import com.hubspot.jinjava.interpret.JinjavaInterpreter;
1415
import com.hubspot.jinjava.interpret.OutputTooBigException;
1516
import com.hubspot.jinjava.util.WhitespaceUtils;
@@ -69,10 +70,21 @@ private static String getAsPyishString(Object val, boolean forOutput) {
6970
return getAsPyishStringOrThrow(val, forOutput);
7071
} catch (IOException e) {
7172
handleLengthLimitingException(e);
73+
handleDeferredValueException(e);
7274
return Objects.toString(val, "");
7375
}
7476
}
7577

78+
private static void handleDeferredValueException(IOException e) {
79+
Throwable unwrapped = e;
80+
if (e instanceof JsonMappingException) {
81+
unwrapped = unwrapped.getCause();
82+
}
83+
if (unwrapped instanceof DeferredValueException) {
84+
throw (DeferredValueException) unwrapped;
85+
}
86+
}
87+
7688
public static void handleLengthLimitingException(IOException e) {
7789
Throwable unwrapped = e;
7890
if (e instanceof JsonMappingException) {

‎src/main/java/com/hubspot/jinjava/objects/serialization/PyishSerializable.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import com.fasterxml.jackson.databind.ObjectWriter;
88
import com.fasterxml.jackson.databind.SerializerProvider;
99
import com.google.common.annotations.Beta;
10+
import com.hubspot.jinjava.interpret.DeferredValueException;
1011
import com.hubspot.jinjava.objects.PyWrapper;
1112
import com.hubspot.jinjava.util.LengthLimitingStringBuilder;
1213
import java.io.IOException;
@@ -32,7 +33,7 @@ public interface PyishSerializable extends PyWrapper {
3233
@SuppressWarnings("unchecked")
3334
default <T extends Appendable & CharSequence> T appendPyishString(T appendable)
3435
throws IOException {
35-
return (T) appendable.append(SELF_WRITER.writeValueAsString(this));
36+
return (T) appendable.append(writeValueAsString(this));
3637
}
3738

3839
/**
@@ -72,6 +73,9 @@ static String writeValueAsString(Object value) {
7273
try {
7374
return SELF_WRITER.writeValueAsString(value);
7475
} catch (JsonProcessingException e) {
76+
if (e.getCause() instanceof DeferredValueException) {
77+
throw (DeferredValueException) e.getCause();
78+
}
7579
return '\'' + Objects.toString(value) + '\'';
7680
}
7781
}

‎src/main/java/com/hubspot/jinjava/util/EagerExpressionResolver.java‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -86,9 +86,7 @@ public static EagerExpressionResult resolveExpression(
8686
} catch (DeferredParsingException e) {
8787
deferredWords.addAll(findDeferredWords(e.getDeferredEvalResult(), interpreter));
8888
result = e.getDeferredEvalResult().trim();
89-
} catch (DeferredValueException e) {
90-
deferredWords.addAll(findDeferredWords(expression, interpreter));
91-
result = expression;
89+
// Throw base-class DeferredValueExceptions because only DeferredParsingExceptions are expected when parsing EL expressions
9290
} catch (TemplateSyntaxException e) {
9391
result = Collections.singletonList(null);
9492
fullyResolved = true;

‎src/test/java/com/hubspot/jinjava/el/ext/eager/EagerAstDotTest.java‎

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,7 @@
1212
import com.hubspot.jinjava.interpret.JinjavaInterpreter;
1313
import com.hubspot.jinjava.interpret.PartiallyDeferredValue;
1414
import com.hubspot.jinjava.mode.EagerExecutionMode;
15-
import com.hubspot.jinjava.objects.serialization.PyishSerializable;
1615
import com.hubspot.jinjava.random.RandomNumberGeneratorStrategy;
17-
import java.io.IOException;
1816
import org.junit.Before;
1917
import org.junit.Test;
2018

@@ -62,13 +60,14 @@ public void itResolvedNestedDeferredMapWithDot() {
6260
}
6361

6462
@Test
65-
public void itDefersWhenNestedDeferredMapDotThrowsDeferredValueException() {
63+
public void itDefersNodeWhenNestedDeferredMapDotThrowsDeferredValueException() {
6664
interpreter.getContext().put("foo_map", ImmutableMap.of("bar", new Foo()));
6765
assertThat(interpreter.render("{{ foo_map.bar.deferred }}"))
68-
.isEqualTo("{{ foo.deferred }}");
66+
.isEqualTo("{{ foo_map.bar.deferred }}");
67+
assertThat(interpreter.getContext().getDeferredNodes()).isNotEmpty();
6968
}
7069

71-
public static class Foo implements PartiallyDeferredValue, PyishSerializable {
70+
public static class Foo implements PartiallyDeferredValue {
7271

7372
public String getDeferred() {
7473
throw new DeferredValueException("foo.deferred is deferred");
@@ -82,11 +81,10 @@ public String getResolved() {
8281
public Object getOriginalValue() {
8382
return null;
8483
}
85-
86-
@Override
87-
public <T extends Appendable & CharSequence> T appendPyishString(T appendable)
88-
throws IOException {
89-
return (T) appendable.append("foo");
90-
}
84+
// @Override
85+
// public <T extends Appendable & CharSequence> T appendPyishString(T appendable)
86+
// throws IOException {
87+
// return (T) appendable.append("foo");
88+
// }
9189
}
9290
}

‎src/test/java/com/hubspot/jinjava/el/ext/eager/EagerAstMethodTest.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,7 @@ public void itPreservesAstTuple() {
225225
@Test
226226
public void itPreservesUnresolvable() {
227227
interpreter.getContext().put("foo_object", new Foo());
228+
interpreter.getContext().getMetaContextVariables().add("foo_object");
228229
try {
229230
interpreter.resolveELExpression("foo_object.deferred|upper", -1);
230231
fail("Should throw DeferredParsingException");

‎src/test/java/com/hubspot/jinjava/interpret/PartiallyDeferredValueTest.java‎

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
package com.hubspot.jinjava.interpret;
2+
3+
import static org.assertj.core.api.Assertions.assertThat;
4+
5+
import com.hubspot.jinjava.BaseInterpretingTest;
6+
import com.hubspot.jinjava.JinjavaConfig;
7+
import com.hubspot.jinjava.LegacyOverrides;
8+
import com.hubspot.jinjava.mode.EagerExecutionMode;
9+
import com.hubspot.jinjava.objects.collections.PyMap;
10+
import com.hubspot.jinjava.objects.serialization.PyishSerializable;
11+
import com.hubspot.jinjava.random.RandomNumberGeneratorStrategy;
12+
import java.io.IOException;
13+
import java.util.Map;
14+
import java.util.Set;
15+
import org.junit.Before;
16+
import org.junit.Test;
17+
18+
public class PartiallyDeferredValueTest extends BaseInterpretingTest {
19+
20+
@Before
21+
public void setup() {
22+
JinjavaConfig config = JinjavaConfig
23+
.newBuilder()
24+
.withRandomNumberGeneratorStrategy(RandomNumberGeneratorStrategy.DEFERRED)
25+
.withExecutionMode(EagerExecutionMode.instance())
26+
.withNestedInterpretationEnabled(true)
27+
.withLegacyOverrides(
28+
LegacyOverrides.newBuilder().withUsePyishObjectMapper(true).build()
29+
)
30+
.withMaxMacroRecursionDepth(5)
31+
.withEnableRecursiveMacroCalls(true)
32+
.build();
33+
JinjavaInterpreter parentInterpreter = new JinjavaInterpreter(
34+
jinjava,
35+
new Context(),
36+
config
37+
);
38+
interpreter = new JinjavaInterpreter(parentInterpreter);
39+
40+
interpreter.getContext().put("deferred", DeferredValue.instance());
41+
}
42+
43+
@Test
44+
public void itDefersNodeWhenCannotSerializePartiallyDeferredValue() {
45+
interpreter.getContext().put("foo", new BadSerialization());
46+
assertThat(interpreter.render("{% set bar = foo %}{{ bar.resolved }}"))
47+
.isEqualTo("resolved");
48+
assertThat(interpreter.render("{% set bar = foo %}{{ bar.deferred }}"))
49+
.isEqualTo("{{ bar.deferred }}");
50+
assertThat(interpreter.getContext().getDeferredNodes()).isNotEmpty();
51+
}
52+
53+
public static class BadSerialization implements PartiallyDeferredValue {
54+
55+
public String getDeferred() {
56+
throw new DeferredValueException("foo.deferred is deferred");
57+
}
58+
59+
public String getResolved() {
60+
return "resolved";
61+
}
62+
63+
@Override
64+
public Object getOriginalValue() {
65+
return null;
66+
}
67+
}
68+
69+
public static class BadEntrySet extends PyMap implements PartiallyDeferredValue {
70+
71+
public BadEntrySet(Map<String, Object> map) {
72+
super(map);
73+
}
74+
75+
@Override
76+
public Set<Entry<String, Object>> entrySet() {
77+
throw new DeferredValueException("entries are deferred");
78+
}
79+
80+
@Override
81+
public Object getOriginalValue() {
82+
return null;
83+
}
84+
}
85+
86+
public static class BadPyishSerializable
87+
implements ReconstructiblePartiallyDeferredValue {
88+
89+
public String getDeferred() {
90+
throw new DeferredValueException("foo.deferred is deferred");
91+
}
92+
93+
public String getResolved() {
94+
return "resolved";
95+
}
96+
97+
@Override
98+
public Object getOriginalValue() {
99+
return null;
100+
}
101+
102+
@Override
103+
public <T extends Appendable & CharSequence> T appendPyishString(T appendable)
104+
throws IOException {
105+
return (T) appendable.append("");
106+
}
107+
}
108+
109+
public static class GoodPyishSerializable
110+
implements PartiallyDeferredValue, PyishSerializable {
111+
112+
public String getDeferred() {
113+
throw new DeferredValueException("foo.deferred is deferred");
114+
}
115+
116+
public String getResolved() {
117+
return "resolved";
118+
}
119+
120+
@Override
121+
public Object getOriginalValue() {
122+
return null;
123+
}
124+
125+
@Override
126+
public <T extends Appendable & CharSequence> T appendPyishString(T appendable)
127+
throws IOException {
128+
return (T) appendable.append("good");
129+
}
130+
}
131+
}

0 commit comments

Comments
 (0)