Repository: PebbleTemplates/pebble
Pull request #400 (2018-09-06 00:24:15)

Adjust AndExpression and OrExpression logic (#398)
And: Evaluate right expression only if left expression is true
Or: Evaluate right expression only if left expression is false


Patch:
diff --git a/pebble/CHANGELOG.md b/pebble/CHANGELOG.md
index 58bccfeb..5dc19b71 100644
--- a/pebble/CHANGELOG.md
+++ b/pebble/CHANGELOG.md
@@ -1,5 +1,8 @@
-## v3.0.4 (xxxx-xx-xx)
+## v3.0.4 (2018-09-09)
 - Make NOT operator more tolerant to argument type (#394)
+- Make ternary if operator more tolerant to argument type (#399) 
+- Adjust AndExpression and OrExpression logic (#398)
+- Add JSON escaping as part of the default escaping strategies (#395)
 
 ## v3.0.3 (2018-08-26)
 - Support more expressions for `if` operator (#387)
diff --git a/pebble/src/main/java/com/mitchellbosecke/pebble/node/expression/AndExpression.java b/pebble/src/main/java/com/mitchellbosecke/pebble/node/expression/AndExpression.java
index 2c61f730..d3ee7b1a 100644
--- a/pebble/src/main/java/com/mitchellbosecke/pebble/node/expression/AndExpression.java
+++ b/pebble/src/main/java/com/mitchellbosecke/pebble/node/expression/AndExpression.java
@@ -17,23 +17,28 @@ public class AndExpression extends BinaryExpression<Boolean> {
   @SuppressWarnings("unchecked")
   @Override
   public Boolean evaluate(PebbleTemplateImpl self, EvaluationContextImpl context) {
-    Boolean left = ((Expression<Boolean>) this.getLeftExpression()).evaluate(self, context);
-    Boolean right = ((Expression<Boolean>) this.getRightExpression()).evaluate(self, context);
-    if (context.isStrictVariables()) {
-      if (left == null || right == null) {
+    Expression<Boolean> leftExpression = (Expression<Boolean>) this.getLeftExpression();
+    boolean left = this.evaluateExpression(self, context, leftExpression);
+    if (left) {
+      Expression<Boolean> rightExpression = (Expression<Boolean>) this.getRightExpression();
+      return this.evaluateExpression(self, context, rightExpression);
+    }
+    return false;
+  }
+
+  private boolean evaluateExpression(PebbleTemplateImpl self, EvaluationContextImpl context,
+      Expression<Boolean> expression) {
+    Boolean evaluatedExpression = expression.evaluate(self, context);
+
+    if (evaluatedExpression == null) {
+      if (context.isStrictVariables()) {
         throw new PebbleException(null,
             "null value used in and operator and strict variables is set to true",
             this.getLineNumber(),
             self.getName());
       }
-    } else {
-      if (left == null) {
-        left = false;
-      }
-      if (right == null) {
-        right = false;
-      }
+      return false;
     }
-    return left && right;
+    return evaluatedExpression;
   }
 }
diff --git a/pebble/src/main/java/com/mitchellbosecke/pebble/node/expression/OrExpression.java b/pebble/src/main/java/com/mitchellbosecke/pebble/node/expression/OrExpression.java
index 70137cf0..eab37bd3 100644
--- a/pebble/src/main/java/com/mitchellbosecke/pebble/node/expression/OrExpression.java
+++ b/pebble/src/main/java/com/mitchellbosecke/pebble/node/expression/OrExpression.java
@@ -17,23 +17,28 @@ public class OrExpression extends BinaryExpression<Boolean> {
   @SuppressWarnings("unchecked")
   @Override
   public Boolean evaluate(PebbleTemplateImpl self, EvaluationContextImpl context) {
-    Boolean left = ((Expression<Boolean>) this.getLeftExpression()).evaluate(self, context);
-    Boolean right = ((Expression<Boolean>) this.getRightExpression()).evaluate(self, context);
-    if (context.isStrictVariables()) {
-      if (left == null || right == null) {
+    Expression<Boolean> leftExpression = (Expression<Boolean>) this.getLeftExpression();
+    boolean left = this.evaluateExpression(self, context, leftExpression);
+    if (!left) {
+      Expression<Boolean> rightExpression = (Expression<Boolean>) this.getRightExpression();
+      return this.evaluateExpression(self, context, rightExpression);
+    }
+    return true;
+  }
+
+  private boolean evaluateExpression(PebbleTemplateImpl self, EvaluationContextImpl context,
+      Expression<Boolean> expression) {
+    Boolean evaluatedExpression = expression.evaluate(self, context);
+
+    if (evaluatedExpression == null) {
+      if (context.isStrictVariables()) {
         throw new PebbleException(null,
-            "null value used in or operator and strict variables is set to true", this
-            .getLineNumber(),
+            "null value used in and operator and strict variables is set to true",
+            this.getLineNumber(),
             self.getName());
       }
-    } else {
-      if (left == null) {
-        left = false;
-      }
-      if (right == null) {
-        right = false;
-      }
+      return false;
     }
-    return left || right;
+    return evaluatedExpression;
   }
 }


Test patch:
diff --git a/pebble/src/test/java/com/mitchellbosecke/pebble/LogicTest.java b/pebble/src/test/java/com/mitchellbosecke/pebble/LogicTest.java
index 6ec4307f..2fab45bc 100644
--- a/pebble/src/test/java/com/mitchellbosecke/pebble/LogicTest.java
+++ b/pebble/src/test/java/com/mitchellbosecke/pebble/LogicTest.java
@@ -471,12 +471,9 @@ public class LogicTest {
     context.put("a", false);
     context.put("b", null);
 
-    try {
-      writer = new StringWriter();
-      andTemplate.evaluate(writer, context);
-      fail("Exception not thrown");
-    } catch (PebbleException e) {
-    }
+    writer = new StringWriter();
+    andTemplate.evaluate(writer, context);
+    assertEquals("no", writer.toString());
 
     try {
       writer = new StringWriter();
diff --git a/pebble/src/test/java/com/mitchellbosecke/pebble/StrictModeTest.java b/pebble/src/test/java/com/mitchellbosecke/pebble/StrictModeTest.java
index 2089b103..5721c759 100644
--- a/pebble/src/test/java/com/mitchellbosecke/pebble/StrictModeTest.java
+++ b/pebble/src/test/java/com/mitchellbosecke/pebble/StrictModeTest.java
@@ -1,7 +1,10 @@
 package com.mitchellbosecke.pebble;
 
+import static org.junit.Assert.assertEquals;
+
 import com.mitchellbosecke.pebble.error.PebbleException;
 import com.mitchellbosecke.pebble.error.RootAttributeNotFoundException;
+import com.mitchellbosecke.pebble.loader.StringLoader;
 import com.mitchellbosecke.pebble.template.PebbleTemplate;
 import java.io.IOException;
 import java.io.StringWriter;
@@ -67,4 +70,45 @@ public class StrictModeTest {
     }
   }
 
+  @Test
+  public void whenStrictVariableEnabledWithAndExpressionAndLeftOperandFalse_thenDontEvaluateRightExpression()
+      throws PebbleException, IOException {
+    PebbleEngine engine = new PebbleEngine
+        .Builder()
+        .loader(new StringLoader())
+        .strictVariables(true)
+        .autoEscaping(false)
+        .build();
+
+    PebbleTemplate template = engine.getTemplate("{%- set a = null -%}\n" +
+        "{{- a is not null and a.toLowerCase() == \"abc\" -}}\n" +
+        "{%- if a is not null and a.toLowerCase() == \"abc\" -%}\n" +
+        "Do something" +
+        "{%- endif -%}");
+
+    Writer writer = new StringWriter();
+    template.evaluate(writer);
+    assertEquals("false", writer.toString());
+  }
+
+  @Test
+  public void whenStrictVariableEnabledWithOrExpressionAndLeftOperandTrue_thenDontEvaluateRightExpression()
+      throws PebbleException, IOException {
+    PebbleEngine engine = new PebbleEngine
+        .Builder()
+        .loader(new StringLoader())
+        .strictVariables(true)
+        .autoEscaping(false)
+        .build();
+
+    PebbleTemplate template = engine.getTemplate("{%- set a = null -%}\n" +
+        "{{- a is null or a.toLowerCase() == \"abc\" -}}\n" +
+        "{%- if a is null or a.toLowerCase() == \"abc\" -%}\n" +
+        "Do something" +
+        "{%- endif -%}");
+
+    Writer writer = new StringWriter();
+    template.evaluate(writer);
+    assertEquals("trueDo something", writer.toString());
+  }
 }