Repository: PebbleTemplates/pebble
Pull request #338 (2018-05-04 19:27:04)

Makes logic with allowGetClass and strictVariables well-defined
`allowGetClass` flag was always returning null or throwing ClassAccessException, even when it should have been allowed. This PR fixes that bug, and tests that it conforms to the following truth table (with the prior results also shown):

| AllowGetClass | Strict Variables | Access Type | Expected Result | Prior Result |
| ------------- | ---------------- | ----------- | --------------- | ------------ |
| true          | false            | property    | allowed         | null         |
| true          | false            | method      | allowed         | null         |
| true          | true             | property    | allowed         | throw        |
| true          | true             | method      | allowed         | throw        |
| false         | false            | property    | null            | throw        |
| false         | false            | method      | null            | throw        |
| false         | true             | property    | throw           | throw        |
| false         | true             | method      | throw           | throw        |




Patch:
diff --git a/src/main/java/com/mitchellbosecke/pebble/node/expression/GetAttributeExpression.java b/src/main/java/com/mitchellbosecke/pebble/node/expression/GetAttributeExpression.java
index 5e4f1dfb..e850eb7e 100644
--- a/src/main/java/com/mitchellbosecke/pebble/node/expression/GetAttributeExpression.java
+++ b/src/main/java/com/mitchellbosecke/pebble/node/expression/GetAttributeExpression.java
@@ -196,13 +196,9 @@ public class GetAttributeExpression implements Expression<Object> {
                 }
 
             } else {
-                if (attributeName.equals("class") || attributeName.equals("getClass")) {
-                    throw new ClassAccessException(this.lineNumber, this.filename);
-                } else {
-                    throw new AttributeNotFoundException(null, String.format(
-                            "Attribute [%s] of [%s] does not exist or can not be accessed and strict variables is set to true.",
-                            attributeName, object.getClass().getName()), attributeName, this.lineNumber, this.filename);
-                }
+                throw new AttributeNotFoundException(null, String.format(
+                        "Attribute [%s] of [%s] does not exist or can not be accessed and strict variables is set to true.",
+                        attributeName, object.getClass().getName()), attributeName, this.lineNumber, this.filename);
             }
         }
         return result;
@@ -351,11 +347,8 @@ public class GetAttributeExpression implements Expression<Object> {
      * @return
      */
     private Method findMethod(Class<?> clazz, String name, Class<?>[] requiredTypes, boolean allowGetClass) {
-        if (name.equals("getClass")) {
-            if (!allowGetClass) {
-                throw new ClassAccessException(this.lineNumber, this.filename);
-            }
-            return null;
+        if (!allowGetClass && name.equals("getClass")) {
+            throw new ClassAccessException(this.lineNumber, this.filename);
         }
 
         Method result = null;


Test patch:
diff --git a/src/test/java/com/mitchellbosecke/pebble/GetAttributeTest.java b/src/test/java/com/mitchellbosecke/pebble/GetAttributeTest.java
index eab56aff..f0a1a1ab 100644
--- a/src/test/java/com/mitchellbosecke/pebble/GetAttributeTest.java
+++ b/src/test/java/com/mitchellbosecke/pebble/GetAttributeTest.java
@@ -15,7 +15,6 @@ import com.mitchellbosecke.pebble.error.RootAttributeNotFoundException;
 import com.mitchellbosecke.pebble.extension.DynamicAttributeProvider;
 import com.mitchellbosecke.pebble.loader.StringLoader;
 import com.mitchellbosecke.pebble.template.PebbleTemplate;
-
 import org.junit.Test;
 
 import java.io.IOException;
@@ -112,77 +111,143 @@ public class GetAttributeTest extends AbstractTest {
         assertEquals("hello Steve", writer.toString());
     }
 
+    /**
+     * Make sure we are properly accounting for getting the class object from an Object in all situations:
+     *
+     * | AllowGetClass | Strict Variables | Access Type | Result  |
+     * | ------------- | ---------------- | ----------- | ------- |
+     * | true          | false            | property    | allowed |
+     * | true          | false            | method      | allowed |
+     * | true          | true             | property    | allowed |
+     * | true          | true             | method      | allowed |
+     * | false         | false            | property    | throw   |
+     * | false         | false            | method      | throw   |
+     * | false         | true             | property    | throw   |
+     * | false         | true             | method      | throw   |
+     *
+     * @throws PebbleException
+     * @throws IOException
+     */
+
     @Test
-    public void testMethodAttributeWhenAccessingClassWithStrictVariableOffAndAllowGetClass() throws PebbleException, IOException {
+    public void testAccessingClass_AllowGetClassOn_StrictVariableOff_Property() throws PebbleException, IOException {
         PebbleEngine pebble = new PebbleEngine.Builder().loader(new StringLoader())
+                .allowGetClass(true)
                 .strictVariables(false)
+                .build();
+
+        PebbleTemplate template = pebble.getTemplate("hello [{{ object.class }}]");
+        Map<String, Object> context = new HashMap<>();
+        context.put("object", new SimpleObject());
+
+        Writer writer = new StringWriter();
+        template.evaluate(writer, context);
+        assertEquals("hello [" + SimpleObject.class.toString() + "]", writer.toString());
+    }
+
+    @Test
+    public void testAccessingClass_AllowGetClassOn_StrictVariableOff_Method() throws PebbleException, IOException {
+        PebbleEngine pebble = new PebbleEngine.Builder().loader(new StringLoader())
                 .allowGetClass(true)
+                .strictVariables(false)
                 .build();
 
-        PebbleTemplate template = pebble.getTemplate("hello {{ object.class }}");
+        PebbleTemplate template = pebble.getTemplate("hello [{{ object.getClass() }}]");
         Map<String, Object> context = new HashMap<>();
-        context.put("object", "some string");
+        context.put("object", new SimpleObject());
 
         Writer writer = new StringWriter();
         template.evaluate(writer, context);
-        assertEquals("hello ", writer.toString());
+        assertEquals("hello [" + SimpleObject.class.toString() + "]", writer.toString());
     }
 
-    @Test(expected = ClassAccessException.class)
-    public void testMethodAttributeWhenAccessingClassWithStrictVariableOnAndAllowGetClass() throws PebbleException, IOException {
+    @Test
+    public void testAccessingClass_AllowGetClassOn_StrictVariableOn_Property() throws PebbleException, IOException {
         PebbleEngine pebble = new PebbleEngine.Builder().loader(new StringLoader())
+                .allowGetClass(true)
                 .strictVariables(true)
+                .build();
+
+        PebbleTemplate template = pebble.getTemplate("hello [{{ object.class }}]");
+        Map<String, Object> context = new HashMap<>();
+        context.put("object", new SimpleObject());
+
+        Writer writer = new StringWriter();
+        template.evaluate(writer, context);
+        assertEquals("hello [" + SimpleObject.class.toString() + "]", writer.toString());
+    }
+
+    @Test
+    public void testAccessingClass_AllowGetClassOn_StrictVariableOn_Method() throws PebbleException, IOException {
+        PebbleEngine pebble = new PebbleEngine.Builder().loader(new StringLoader())
                 .allowGetClass(true)
+                .strictVariables(true)
                 .build();
 
-        PebbleTemplate template = pebble.getTemplate("hello {{ object.class }}");
+        PebbleTemplate template = pebble.getTemplate("hello [{{ object.getClass() }}]");
         Map<String, Object> context = new HashMap<>();
-        context.put("object", "some string");
+        context.put("object", new SimpleObject());
 
         Writer writer = new StringWriter();
         template.evaluate(writer, context);
+        assertEquals("hello [" + SimpleObject.class.toString() + "]", writer.toString());
     }
 
     @Test(expected = ClassAccessException.class)
-    public void testMethodAttributeWhenAccessingGetClassWithStrictVariableOnAndAllowGetClass() throws PebbleException, IOException {
+    public void testAccessingClass_AllowGetClassOff_StrictVariableOff_Property() throws PebbleException, IOException {
         PebbleEngine pebble = new PebbleEngine.Builder().loader(new StringLoader())
-                .strictVariables(true)
-                .allowGetClass(true)
+                .allowGetClass(false)
+                .strictVariables(false)
                 .build();
 
-        PebbleTemplate template = pebble.getTemplate("hello {{ object.getClass }}");
+        PebbleTemplate template = pebble.getTemplate("hello [{{ object.class }}]");
         Map<String, Object> context = new HashMap<>();
-        context.put("object", "some string");
+        context.put("object", new SimpleObject());
 
         Writer writer = new StringWriter();
         template.evaluate(writer, context);
     }
 
     @Test(expected = ClassAccessException.class)
-    public void testMethodAttributeWhenAccessingClassWithStrictVariableOffAndDontAllowGetClass() throws PebbleException, IOException {
+    public void testAccessingClass_AllowGetClassOff_StrictVariableOff_Method() throws PebbleException, IOException {
         PebbleEngine pebble = new PebbleEngine.Builder().loader(new StringLoader())
+                .allowGetClass(false)
                 .strictVariables(false)
+                .build();
+
+        PebbleTemplate template = pebble.getTemplate("hello [{{ object.getClass() }}]");
+        Map<String, Object> context = new HashMap<>();
+        context.put("object", new SimpleObject());
+
+        Writer writer = new StringWriter();
+        template.evaluate(writer, context);
+    }
+
+    @Test(expected = ClassAccessException.class)
+    public void testAccessingClass_AllowGetClassOff_StrictVariableOn_Property() throws PebbleException, IOException {
+        PebbleEngine pebble = new PebbleEngine.Builder().loader(new StringLoader())
                 .allowGetClass(false)
+                .strictVariables(true)
                 .build();
 
-        PebbleTemplate template = pebble.getTemplate("hello {{ object.class }}");
+        PebbleTemplate template = pebble.getTemplate("hello [{{ object.class }}]");
         Map<String, Object> context = new HashMap<>();
-        context.put("object", "some string");
+        context.put("object", new SimpleObject());
 
         Writer writer = new StringWriter();
         template.evaluate(writer, context);
     }
 
     @Test(expected = ClassAccessException.class)
-    public void testMethodAttributeWhenAccessingGetClassWithStrictVariableOffAndDontAllowGetClass() throws PebbleException, IOException {
+    public void testAccessingClass_AllowGetClassOff_StrictVariableOn_Method() throws PebbleException, IOException {
         PebbleEngine pebble = new PebbleEngine.Builder().loader(new StringLoader())
-                .strictVariables(false)
                 .allowGetClass(false)
+                .strictVariables(true)
                 .build();
 
-        PebbleTemplate template = pebble.getTemplate("hello {{ object.getClass }}");
+        PebbleTemplate template = pebble.getTemplate("hello [{{ object.getClass() }}]");
         Map<String, Object> context = new HashMap<>();
-        context.put("object", "some string");
+        context.put("object", new SimpleObject());
 
         Writer writer = new StringWriter();
         template.evaluate(writer, context);