diff --git a/src/main/java/com/hubspot/jinjava/el/ExpressionResolver.java b/src/main/java/com/hubspot/jinjava/el/ExpressionResolver.java index 4930747f5..198eff814 100644 --- a/src/main/java/com/hubspot/jinjava/el/ExpressionResolver.java +++ b/src/main/java/com/hubspot/jinjava/el/ExpressionResolver.java @@ -21,6 +21,7 @@ import com.hubspot.jinjava.interpret.TemplateSyntaxException; import com.hubspot.jinjava.interpret.UnknownTokenException; import com.hubspot.jinjava.interpret.errorcategory.BasicTemplateErrorCategory; +import com.hubspot.jinjava.interpret.errorcategory.DivideByZeroException; import com.hubspot.jinjava.lib.fn.ELFunctionDefinition; import com.hubspot.jinjava.util.WhitespaceUtils; import de.odysseus.el.tree.TreeBuilderException; @@ -102,6 +103,15 @@ public Object resolveExpression(String expression) { validateResult(result); return result; + } catch (DivideByZeroException e) { + // DivOperator lacks access to the interpreter, so couldn't generate an exception message that would + // be meaningful to the end-user. As there's no good way to get an interpreter down to DivOperator, + // generate a user-friendly TemplateError here. + TemplateError zeroDivisorError = TemplateError.fromException(e); + zeroDivisorError.setMessage(String.format("%s : %s", expression, e.getMessage())); + zeroDivisorError.setLineno(interpreter.getLineNumber()); + zeroDivisorError.setStartPosition(interpreter.getPosition()); + interpreter.addError(zeroDivisorError); } catch (PropertyNotFoundException e) { interpreter.addError( new TemplateError( diff --git a/src/main/java/com/hubspot/jinjava/el/ext/DivOperator.java b/src/main/java/com/hubspot/jinjava/el/ext/DivOperator.java new file mode 100644 index 000000000..cd68bcb16 --- /dev/null +++ b/src/main/java/com/hubspot/jinjava/el/ext/DivOperator.java @@ -0,0 +1,44 @@ +package com.hubspot.jinjava.el.ext; + +import com.hubspot.jinjava.interpret.errorcategory.DivideByZeroException; +import de.odysseus.el.misc.TypeConverter; +import de.odysseus.el.tree.impl.Parser.ExtensionHandler; +import de.odysseus.el.tree.impl.Parser.ExtensionPoint; +import de.odysseus.el.tree.impl.Scanner; +import de.odysseus.el.tree.impl.ast.AstBinary; +import de.odysseus.el.tree.impl.ast.AstBinary.SimpleOperator; +import de.odysseus.el.tree.impl.ast.AstNode; + +/** + * Created to allow for the detection and handling of divide-by-zero requests in EL expressions + * (see PR 473 @ https://github.com/HubSpot/jinjava/pull/473) + */ +public class DivOperator extends SimpleOperator { + public static final Scanner.ExtensionToken TOKEN = new Scanner.ExtensionToken("/"); + public static final DivOperator OP = new DivOperator(); + + @Override + protected Object apply(TypeConverter converter, Object a, Object b) { + if (a == null || b == null) { + throw new DivideByZeroException("Division operator argument may not be null"); + } + + Number numA = (Number) a; + Number numB = (Number) b; + if (numB.doubleValue() == 0.0) { + throw new DivideByZeroException("Division operator divisor may not be zero"); + } + + return numA.doubleValue() / numB.doubleValue(); + } + + public static final ExtensionHandler HANDLER = new ExtensionHandler( + ExtensionPoint.MUL + ) { + + @Override + public AstNode createAstNode(AstNode... children) { + return new AstBinary(children[0], children[1], OP); + } + }; +} diff --git a/src/main/java/com/hubspot/jinjava/el/ext/ExtendedParser.java b/src/main/java/com/hubspot/jinjava/el/ext/ExtendedParser.java index 3a3c51ef9..226da7835 100644 --- a/src/main/java/com/hubspot/jinjava/el/ext/ExtendedParser.java +++ b/src/main/java/com/hubspot/jinjava/el/ext/ExtendedParser.java @@ -65,6 +65,7 @@ public class ExtendedParser extends Parser { static final Scanner.ExtensionToken LITERAL_DICT_END = new Scanner.ExtensionToken("}"); static final Scanner.ExtensionToken TRUNC_DIV = TruncDivOperator.TOKEN; + static final Scanner.ExtensionToken REGULAR_DIV = DivOperator.TOKEN; static final Scanner.ExtensionToken POWER_OF = PowerOfOperator.TOKEN; static final Set VALID_SYMBOLS_FOR_EXP_TEST = Sets.newHashSet( @@ -87,6 +88,7 @@ public class ExtendedParser extends Parser { ExtendedScanner.addKeyToken(PYFALSE); ExtendedScanner.addKeyToken(TruncDivOperator.TOKEN); + ExtendedScanner.addKeyToken(DivOperator.TOKEN); ExtendedScanner.addKeyToken(PowerOfOperator.TOKEN); ExtendedScanner.addKeyToken(CollectionMembershipOperator.TOKEN); @@ -98,6 +100,7 @@ public ExtendedParser(Builder context, String input) { putExtensionHandler(NamedParameterOperator.TOKEN, NamedParameterOperator.HANDLER); putExtensionHandler(StringConcatOperator.TOKEN, StringConcatOperator.HANDLER); putExtensionHandler(TruncDivOperator.TOKEN, TruncDivOperator.HANDLER); + putExtensionHandler(DivOperator.TOKEN, DivOperator.HANDLER); putExtensionHandler(PowerOfOperator.TOKEN, PowerOfOperator.HANDLER); putExtensionHandler( diff --git a/src/main/java/com/hubspot/jinjava/el/ext/ExtendedScanner.java b/src/main/java/com/hubspot/jinjava/el/ext/ExtendedScanner.java index bf54da80f..8886b9b87 100644 --- a/src/main/java/com/hubspot/jinjava/el/ext/ExtendedScanner.java +++ b/src/main/java/com/hubspot/jinjava/el/ext/ExtendedScanner.java @@ -145,6 +145,10 @@ protected Token nextEval() throws ScanException { if (c1 == '~') { return StringConcatOperator.TOKEN; } + // Keep this comparison below that of all other tokens whose symbols begin with '/' + if (c1 == '/' && c2 != '/') { + return ExtendedParser.REGULAR_DIV; + } return super.nextEval(); } diff --git a/src/main/java/com/hubspot/jinjava/el/ext/TruncDivOperator.java b/src/main/java/com/hubspot/jinjava/el/ext/TruncDivOperator.java index d8b4fd146..d36305ecd 100644 --- a/src/main/java/com/hubspot/jinjava/el/ext/TruncDivOperator.java +++ b/src/main/java/com/hubspot/jinjava/el/ext/TruncDivOperator.java @@ -20,16 +20,30 @@ protected Object apply(TypeConverter converter, Object a, Object b) { boolean aNum = aInt || a instanceof Double || a instanceof Float; boolean bNum = bInt || b instanceof Double || b instanceof Float; + double bAsDouble = converter.convert(b, Double.class); + if (bAsDouble == 0.0) { + throw new IllegalArgumentException( + String.format( + "Divisor for // (truncated division) cannot be zero: '%s' (%s) and '%s' (%s)", + a, + a.getClass().getSimpleName(), + b, + b.getClass().getSimpleName() + ) + ); + } + if (aInt && bInt) { - Long d = converter.convert(a, Long.class); - Long e = converter.convert(b, Long.class); - return Math.floorDiv(d, e); + Long aAsLong = converter.convert(a, Long.class); + Long bAsLong = converter.convert(b, Long.class); + return Math.floorDiv(aAsLong, bAsLong); } + if (aNum && bNum) { - Double d = converter.convert(a, Double.class); - Double e = converter.convert(b, Double.class); - return Math.floor(d / e); + Double aAsDouble = converter.convert(a, Double.class); + return Math.floor(aAsDouble / bAsDouble); } + throw new IllegalArgumentException( String.format( "Unsupported operand type(s) for //: '%s' (%s) and '%s' (%s)", diff --git a/src/main/java/com/hubspot/jinjava/interpret/errorcategory/DivideByZeroException.java b/src/main/java/com/hubspot/jinjava/interpret/errorcategory/DivideByZeroException.java new file mode 100644 index 000000000..4d6180370 --- /dev/null +++ b/src/main/java/com/hubspot/jinjava/interpret/errorcategory/DivideByZeroException.java @@ -0,0 +1,13 @@ +package com.hubspot.jinjava.interpret.errorcategory; + +public class DivideByZeroException extends RuntimeException { + private final String errorMessage; + + public DivideByZeroException(String errorMessage) { + this.errorMessage = errorMessage; + } + + public String getMessage() { + return errorMessage; + } +} diff --git a/src/test/java/com/hubspot/jinjava/el/ext/DivTest.java b/src/test/java/com/hubspot/jinjava/el/ext/DivTest.java new file mode 100644 index 000000000..63f3ba95d --- /dev/null +++ b/src/test/java/com/hubspot/jinjava/el/ext/DivTest.java @@ -0,0 +1,46 @@ +package com.hubspot.jinjava.el.ext; + +import static org.junit.Assert.assertEquals; + +import com.hubspot.jinjava.Jinjava; +import com.hubspot.jinjava.interpret.RenderResult; +import com.hubspot.jinjava.interpret.TemplateError; +import com.hubspot.jinjava.interpret.TemplateError.ErrorReason; +import com.hubspot.jinjava.interpret.TemplateError.ErrorType; +import java.util.HashMap; +import org.junit.Before; +import org.junit.Test; + +public class DivTest { + private Jinjava jinja; + + @Before + public void setUp() { + jinja = new Jinjava(); + } + + @Test + public void itDividesWithNonZeroLongDivisor() { + assertEquals(jinja.render("{% set x = 10 / 2%}{{x}}", new HashMap<>()), "5.0"); + } + + @Test + public void itDividesWithNonZeroDoubleDivisor() { + assertEquals(jinja.render("{% set x = 10.0 / 2.0%}{{x}}", new HashMap<>()), "5.0"); + } + + @Test + public void itErrorsOutWithZeroDivisor() { + RenderResult result = jinja.renderForResult( + "{% set x = (10.0 + 2.0 - 3.0/1.5) / 0.0%}{{x}}", + new HashMap<>() + ); + + assertEquals(result.getErrors().size(), 1); + TemplateError error = result.getErrors().get(0); + assertEquals(error.getSeverity(), ErrorType.FATAL); + assertEquals(error.getReason(), ErrorReason.EXCEPTION); + assertEquals(error.getMessage().contains("10.0 + 2.0 - 3.0/1.5"), true); + assertEquals(error.getMessage().contains("divisor may not be zero"), true); + } +} diff --git a/src/test/java/com/hubspot/jinjava/el/ext/TruncDivTest.java b/src/test/java/com/hubspot/jinjava/el/ext/TruncDivTest.java index 5fe4553fd..c61b14b6e 100644 --- a/src/test/java/com/hubspot/jinjava/el/ext/TruncDivTest.java +++ b/src/test/java/com/hubspot/jinjava/el/ext/TruncDivTest.java @@ -6,6 +6,11 @@ import com.google.common.collect.Maps; import com.hubspot.jinjava.Jinjava; import com.hubspot.jinjava.interpret.FatalTemplateErrorsException; +import com.hubspot.jinjava.interpret.RenderResult; +import com.hubspot.jinjava.interpret.TemplateError; +import com.hubspot.jinjava.interpret.TemplateError.ErrorReason; +import com.hubspot.jinjava.interpret.TemplateError.ErrorType; +import java.util.HashMap; import java.util.Map; import org.junit.Before; import org.junit.Test; @@ -28,6 +33,7 @@ public void testTruncDivInteger() { context.put("divisor", 2); context.put("negativeDividend", -5); context.put("negativeDivisor", -2); + context.put("zeroDivisor", 0); String[][] testCases = { { "{% set x = dividend // divisor %}{{x}}", "2" }, @@ -50,6 +56,29 @@ public void testTruncDivInteger() { } } + /** + * Test the truncated division operator "//" with divisor equal to zero + */ + @Test + public void testTruncDivZeroDivisor() { + final String intTestCase = "{% set x = 10 // 0%}{{x}}"; + final String doubleTestCase = "{% set x = 10 // 0.0%}{{x}}"; + final String[] testCases = { intTestCase, doubleTestCase }; + + for (String testCase : testCases) { + RenderResult result = jinja.renderForResult(testCase, new HashMap<>()); + + assertEquals(result.getErrors().size(), 1); + TemplateError error = result.getErrors().get(0); + assertEquals(error.getSeverity(), ErrorType.FATAL); + assertEquals(error.getReason(), ErrorReason.EXCEPTION); + assertEquals( + error.getMessage().contains("Divisor for // (truncated division) cannot be zero"), + true + ); + } + } + /** * Test the truncated division operator "//" with fractional values */