diff --git a/src/main/java/com/hubspot/jinjava/el/ExpressionResolver.java b/src/main/java/com/hubspot/jinjava/el/ExpressionResolver.java index aaab4a4c6..74221fd65 100644 --- a/src/main/java/com/hubspot/jinjava/el/ExpressionResolver.java +++ b/src/main/java/com/hubspot/jinjava/el/ExpressionResolver.java @@ -5,6 +5,7 @@ import com.google.common.collect.ImmutableMap; import com.hubspot.jinjava.Jinjava; import com.hubspot.jinjava.el.ext.NamedParameter; +import com.hubspot.jinjava.el.ext.eager.EagerExtendedSyntaxBuilder; import com.hubspot.jinjava.interpret.CollectionTooBigException; import com.hubspot.jinjava.interpret.DeferredValueException; import com.hubspot.jinjava.interpret.DisabledException; @@ -25,6 +26,7 @@ import com.hubspot.jinjava.objects.serialization.PyishObjectMapper; import com.hubspot.jinjava.util.WhitespaceUtils; import de.odysseus.el.tree.TreeBuilderException; +import de.odysseus.el.tree.impl.Builder.Feature; import java.util.Arrays; import java.util.List; import java.util.Objects; @@ -41,6 +43,7 @@ public class ExpressionResolver { private final JinjavaInterpreter interpreter; private final ExpressionFactory expressionFactory; + private final JinjavaInterpreterResolver interpreterResolver; private final ReturnTypeValidatingJinjavaInterpreterResolver resolver; private final JinjavaELContext elContext; private final ObjectUnwrapper objectUnwrapper; @@ -55,10 +58,11 @@ public ExpressionResolver(JinjavaInterpreter interpreter, Jinjava jinjava) { ? jinjava.getEagerExpressionFactory() : jinjava.getExpressionFactory(); + this.interpreterResolver = new JinjavaInterpreterResolver(interpreter); this.resolver = new ReturnTypeValidatingJinjavaInterpreterResolver( interpreter.getConfig().getReturnTypeValidator(), - new JinjavaInterpreterResolver(interpreter) + interpreterResolver ); this.elContext = new JinjavaELContext(interpreter, resolver); for (ELFunctionDefinition fn : jinjava.getGlobalContext().getAllFunctions()) { @@ -118,8 +122,17 @@ private Object resolveExpression(String expression, boolean addToResolvedExpress elExpression, Object.class ); + interpreterResolver.resetMethodInvoked(); Object result = valueExp.getValue(elContext); - if (result == null && interpreter.getConfig().isFailOnUnknownTokens()) { + boolean nullReturnedByMethod = + result == null && + interpreterResolver.wasMethodInvoked() && + expressionIsMethodInvocation(elExpression); + if ( + result == null && + interpreter.getConfig().isFailOnUnknownTokens() && + !nullReturnedByMethod + ) { throw new UnknownTokenException( expression, interpreter.getLineNumber(), @@ -227,6 +240,16 @@ private Object resolveExpression(String expression, boolean addToResolvedExpress return null; } + private boolean expressionIsMethodInvocation(String expression) { + ExtendedSyntaxBuilder builder = interpreter + .getConfig() + .getExecutionMode() + .useEagerParser() + ? new EagerExtendedSyntaxBuilder(Feature.METHOD_INVOCATIONS, Feature.VARARGS) + : new ExtendedSyntaxBuilder(Feature.METHOD_INVOCATIONS, Feature.VARARGS); + return builder.build(expression).getRoot().isMethodInvocation(); + } + private void handleELException(String expression, ELException e) { if (e.getCause() != null && e.getCause() instanceof DeferredValueException) { throw (DeferredValueException) e.getCause(); diff --git a/src/main/java/com/hubspot/jinjava/el/JinjavaInterpreterResolver.java b/src/main/java/com/hubspot/jinjava/el/JinjavaInterpreterResolver.java index c60372b99..388218957 100644 --- a/src/main/java/com/hubspot/jinjava/el/JinjavaInterpreterResolver.java +++ b/src/main/java/com/hubspot/jinjava/el/JinjavaInterpreterResolver.java @@ -74,6 +74,7 @@ public class JinjavaInterpreterResolver extends SimpleResolver { private final JinjavaInterpreter interpreter; private final ObjectUnwrapper objectUnwrapper; + private boolean methodInvoked; public JinjavaInterpreterResolver(JinjavaInterpreter interpreter) { super(interpreter.getConfig().getElResolver()); @@ -89,6 +90,7 @@ public Object invoke( Class[] paramTypes, Object[] params ) { + methodInvoked = false; try { Object methodProperty = getValue(context, base, method, false); if (methodProperty instanceof AbstractCallableMethod) { @@ -96,6 +98,7 @@ public Object invoke( ? "" : ((AbstractCallableMethod) methodProperty).evaluate(params); context.setPropertyResolved(true); + methodInvoked = true; return result; } } catch (IllegalArgumentException e) { @@ -103,7 +106,7 @@ public Object invoke( } try { - return interpreter.getContext().isValidationMode() + Object result = interpreter.getContext().isValidationMode() ? "" : super.invoke( context, @@ -112,11 +115,21 @@ public Object invoke( paramTypes, generateMethodParams(method, params) ); + methodInvoked = true; + return result; } catch (IllegalArgumentException e) { return null; } } + void resetMethodInvoked() { + methodInvoked = false; + } + + boolean wasMethodInvoked() { + return methodInvoked; + } + /** * {@inheritDoc} * @@ -124,6 +137,7 @@ public Object invoke( */ @Override public Object getValue(ELContext context, Object base, Object property) { + methodInvoked = false; return getValue(context, base, property, true); } diff --git a/src/test/java/com/hubspot/jinjava/tree/FailOnUnknownTokensTest.java b/src/test/java/com/hubspot/jinjava/tree/FailOnUnknownTokensTest.java index 13042155f..3b10127fa 100644 --- a/src/test/java/com/hubspot/jinjava/tree/FailOnUnknownTokensTest.java +++ b/src/test/java/com/hubspot/jinjava/tree/FailOnUnknownTokensTest.java @@ -8,6 +8,7 @@ import com.hubspot.jinjava.JinjavaConfig; import com.hubspot.jinjava.interpret.JinjavaInterpreter; import com.hubspot.jinjava.interpret.UnknownTokenException; +import com.hubspot.jinjava.mode.EagerExecutionMode; import java.util.HashMap; import java.util.Map; import org.junit.Before; @@ -47,6 +48,43 @@ public void itReplacesTokensWithDefaultValues() { .isEqualTo("mary has a lamb and eats apple"); } + @Test + public void itAllowsNullReturningDoExpressions() { + String template = "{% set object = {} %}{% do object.update({}) %}done"; + + assertThat(jinjava.render(template, new HashMap<>())).isEqualTo("done"); + } + + @Test + public void itAllowsNullReturningDoExpressionsInEagerMode() { + Jinjava eagerJinjava = new Jinjava( + BaseJinjavaTest + .newConfigBuilder() + .withFailOnUnknownTokens(true) + .withExecutionMode(EagerExecutionMode.instance()) + .build() + ); + String template = "{% set object = {} %}{% do object.update({}) %}done"; + + assertThat(eagerJinjava.render(template, new HashMap<>())).isEqualTo("done"); + } + + @Test + public void itRejectsUnknownMethodsInDoExpressions() { + String template = "{% set object = {} %}{% do object.missing() %}"; + + assertThatThrownBy(() -> jinjava.render(template, new HashMap<>())) + .hasMessageContaining("Cannot find method missing"); + } + + @Test + public void itRejectsUnknownPropertiesAfterNullReturningMethods() { + String template = "{% set object = {} %}{% do object.update({}).missing %}"; + + assertThatThrownBy(() -> jinjava.render(template, new HashMap<>())) + .hasMessageContaining("Unknown token found: object.update({}).missing"); + } + @Test public void itReplacesTokensInContextButThrowsExceptionForOthers() { final JinjavaConfig config = BaseJinjavaTest