Repository: PebbleTemplates/pebble
Pull request #443 (2019-05-27 23:23:19)

More comprehensive fix for illegal reflective access under jdk 9+
While looking into #441 I noticed that the same problem occurs for the private `List` implementation returned by `Arrays::asList` (the `size()` method can't be called, see `LogicTest::testListSize`). That's a general problem for unexported / unopened collection types. The fix is to search these well known public classes for members first. This approach also avoids copying potentially large collections.

~Adding `--illegal-access=deny` to unit tests will fail the build for these errors to avoid regressions in the future.~ Doesn't work, see https://github.com/PebbleTemplates/pebble/pull/443#issuecomment-496643233.

ebussieres: @zimmi Did you run the performance benchmark to see if there's a difference ? https://github.com/PebbleTemplates/pebble-performance-test
zimmi: @ebussieres Nope, will do that now.
zimmi: Running on `openjdk 12.0.1 2019-04-16`

#### master
    Benchmark          Mode  Cnt      Score     Error  Units
    Pebble.benchmark  thrpt   50  24635.207 ± 180.448  ops/s

#### this PR
    Benchmark          Mode  Cnt      Score     Error  Units
    Pebble.benchmark  thrpt   50  24335.089 ± 253.823  ops/s

ebussieres: Can you update .travis.yml file to test with multiple jdk ?
zimmi: The failures are because https://github.com/PebbleTemplates/pebble/pull/445 is not yet merged. Do you want me to add those changes in here and delete the other PR?
ebussieres: Yes please merge the two PR together. 

Thanks
zimmi: It is done.

Patch:
diff --git a/.travis.yml b/.travis.yml
index 281fbe01..0c67e816 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -6,6 +6,8 @@ addons:
 language: java
 jdk:
     - oraclejdk8
+    - openjdk11
+    - openjdk12
 
 after_success:
   - export VERSION=$(echo "cat //*[local-name()='project']/*[local-name()='version']/text()" | xmllint --nocdata --shell pom.xml | sed '1d;$d')
diff --git a/docs/src/orchid/resources/wiki/guide/spring-integration.md b/docs/src/orchid/resources/wiki/guide/spring-integration.md
index 32eed897..33b1b03d 100644
--- a/docs/src/orchid/resources/wiki/guide/spring-integration.md
+++ b/docs/src/orchid/resources/wiki/guide/spring-integration.md
@@ -4,7 +4,9 @@
 # Integration with Spring
 
 ## Example
-There is a fully working example project located on [github](https://github.com/PebbleTemplates/pebble-example-spring)
+There is the spring petclinic example which has been migrated to [pebble](https://github.com/PebbleTemplates/spring-petclinic) 
+
+There is also a fully working example project located on [github](https://github.com/PebbleTemplates/pebble-example-spring)
 which can be used as a reference. It is a very simple and bare-bones project designed to only portray the basics.
 To build the project, simply run `mvn install` and then deploy the resulting war file to a an application container.
 
diff --git a/docs/src/orchid/resources/wiki/summary.md b/docs/src/orchid/resources/wiki/summary.md
index 8ae526c0..d672e975 100644
--- a/docs/src/orchid/resources/wiki/summary.md
+++ b/docs/src/orchid/resources/wiki/summary.md
@@ -3,6 +3,7 @@
 - [Spring Integration](guide/spring-integration.md)
 - [Spring Boot Integration](guide/spring-boot-integration.md)
 - [Pebble Spring Example](https://github.com/PebbleTemplates/pebble-example-spring)
+- [Spring petclinic](https://github.com/PebbleTemplates/spring-petclinic)
 - [Basic Usage](guide/basic-usage.md)
 - [Escaping](guide/escaping.md)
 - [Extending Pebble](guide/extending-pebble.md)
diff --git a/pebble-spring/pebble-spring-boot-starter/pom.xml b/pebble-spring/pebble-spring-boot-starter/pom.xml
index 8abf3c7f..a32ab6ab 100644
--- a/pebble-spring/pebble-spring-boot-starter/pom.xml
+++ b/pebble-spring/pebble-spring-boot-starter/pom.xml
@@ -14,7 +14,7 @@
   <url>http://pebbletemplates.io</url>
 
   <properties>
-    <boot.version>1.3.0.RELEASE</boot.version>
+    <boot.version>1.5.21.RELEASE</boot.version>
   </properties>
 
   <dependencies>
diff --git a/pebble/src/main/java/com/mitchellbosecke/pebble/attributes/MemberCacheUtils.java b/pebble/src/main/java/com/mitchellbosecke/pebble/attributes/MemberCacheUtils.java
index ca39b609..b3719b31 100644
--- a/pebble/src/main/java/com/mitchellbosecke/pebble/attributes/MemberCacheUtils.java
+++ b/pebble/src/main/java/com/mitchellbosecke/pebble/attributes/MemberCacheUtils.java
@@ -8,7 +8,10 @@ import java.lang.reflect.Member;
 import java.lang.reflect.Method;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.List;
+import java.util.Map;
+import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 
 class MemberCacheUtils {
@@ -45,45 +48,61 @@ class MemberCacheUtils {
     // capitalize first letter of attribute for the following attempts
     String attributeCapitalized =
         Character.toUpperCase(attributeName.charAt(0)) + attributeName.substring(1);
-
-    // check get method
-    Member result = this
-        .findMethod(clazz, "get" + attributeCapitalized, parameterTypes, filename, lineNumber,
-            evaluationOptions);
-
-    // check is method
-    if (result == null) {
-      result = this
-          .findMethod(clazz, "is" + attributeCapitalized, parameterTypes, filename, lineNumber,
+    
+    // search well known super classes first to avoid illegal reflective access
+    List<Class<?>> agenda = Arrays.asList(
+        List.class,
+        Set.class,
+        Map.class,
+        Map.Entry.class,
+        Collection.class,
+        Iterable.class,
+        clazz);
+    
+    for (Class<?> type : agenda) {
+      if (!type.isAssignableFrom(clazz)) {
+        continue;
+      }
+      
+      // check get method
+      Member result = this
+          .findMethod(type, "get" + attributeCapitalized, parameterTypes, filename, lineNumber,
               evaluationOptions);
-    }
 
-    // check has method
-    if (result == null) {
-      result = this
-          .findMethod(clazz, "has" + attributeCapitalized, parameterTypes, filename, lineNumber,
-              evaluationOptions);
-    }
+      // check is method
+      if (result == null) {
+        result = this
+            .findMethod(type, "is" + attributeCapitalized, parameterTypes, filename, lineNumber,
+                evaluationOptions);
+      }
 
-    // check if attribute is a public method
-    if (result == null) {
-      result = this.findMethod(clazz, attributeName, parameterTypes, filename, lineNumber,
-          evaluationOptions);
-    }
+      // check has method
+      if (result == null) {
+        result = this
+            .findMethod(type, "has" + attributeCapitalized, parameterTypes, filename, lineNumber,
+                evaluationOptions);
+      }
 
-    // public field
-    if (result == null) {
-      try {
-        result = clazz.getField(attributeName);
-      } catch (NoSuchFieldException | SecurityException e) {
+      // check if attribute is a public method
+      if (result == null) {
+        result = this.findMethod(type, attributeName, parameterTypes, filename, lineNumber,
+            evaluationOptions);
       }
-    }
 
-    if (result != null) {
-      ((AccessibleObject) result).setAccessible(true);
+      // public field
+      if (result == null) {
+        try {
+          result = type.getField(attributeName);
+        } catch (NoSuchFieldException | SecurityException e) {
+        }
+      }
+      
+      if (result != null) {
+        ((AccessibleObject) result).setAccessible(true);
+        return result;
+      }
     }
-
-    return result;
+    return null;
   }
 
   /**
diff --git a/pebble/src/main/java/com/mitchellbosecke/pebble/node/ForNode.java b/pebble/src/main/java/com/mitchellbosecke/pebble/node/ForNode.java
index d4c596d5..6f156e71 100644
--- a/pebble/src/main/java/com/mitchellbosecke/pebble/node/ForNode.java
+++ b/pebble/src/main/java/com/mitchellbosecke/pebble/node/ForNode.java
@@ -16,12 +16,13 @@ import com.mitchellbosecke.pebble.node.fornode.LazyRevIndex;
 import com.mitchellbosecke.pebble.template.EvaluationContextImpl;
 import com.mitchellbosecke.pebble.template.PebbleTemplateImpl;
 import com.mitchellbosecke.pebble.template.ScopeChain;
-import com.mitchellbosecke.pebble.utils.PublicMapEntry;
-
 import java.io.IOException;
 import java.io.Writer;
 import java.lang.reflect.Array;
-import java.util.*;
+import java.util.Enumeration;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.Map;
 
 /**
  * Represents a "for" loop within the template.
@@ -150,7 +151,7 @@ public class ForNode extends AbstractRenderableNode {
       result = (Iterable<Object>) obj;
     } else if (obj instanceof Map) {
       // raw type
-      result = PublicMapEntry.fromEntrySet(((Map)obj).entrySet());
+      result = ((Map) obj).entrySet();
     } else if (obj.getClass().isArray()) {
       result = new ArrayIterable(obj);
     } else if (obj instanceof Enumeration) {
diff --git a/pebble/src/main/java/com/mitchellbosecke/pebble/utils/PublicMapEntry.java b/pebble/src/main/java/com/mitchellbosecke/pebble/utils/PublicMapEntry.java
deleted file mode 100644
index b02b754d..00000000
--- a/pebble/src/main/java/com/mitchellbosecke/pebble/utils/PublicMapEntry.java
+++ /dev/null
@@ -1,41 +0,0 @@
-package com.mitchellbosecke.pebble.utils;
-
-import java.util.LinkedHashSet;
-import java.util.Map;
-import java.util.Set;
-
-public class PublicMapEntry<K, V> implements Map.Entry<K, V> {
-
-    private K key;
-    private V value;
-
-    private PublicMapEntry(Map.Entry<K, V> source) {
-        this.key = source.getKey();
-        this.value = source.getValue();
-    }
-
-    public static <K, V> Set<Map.Entry<K, V>> fromEntrySet(Set<Map.Entry<K, V>> source) {
-        Set<Map.Entry<K, V>> ret = new LinkedHashSet<>();
-        for (Map.Entry sourceMe : source) {
-            ret.add(new PublicMapEntry(sourceMe));
-        }
-        return ret;
-    }
-
-    @Override
-    public K getKey() {
-        return this.key;
-    }
-
-    @Override
-    public V getValue() {
-        return this.value;
-    }
-
-    @Override
-    public V setValue(V value) {
-        V aux = this.value;
-        this.value = value;
-        return aux;
-    }
-}
\ No newline at end of file


Test patch:
diff --git a/pebble-spring/pebble-spring-boot-starter/src/test/java/com/mitchellbosecke/pebble/boot/ControllerTest.java b/pebble-spring/pebble-spring-boot-starter/src/test/java/com/mitchellbosecke/pebble/boot/ControllerTest.java
index 803a1a43..08792e1e 100644
--- a/pebble-spring/pebble-spring-boot-starter/src/test/java/com/mitchellbosecke/pebble/boot/ControllerTest.java
+++ b/pebble-spring/pebble-spring-boot-starter/src/test/java/com/mitchellbosecke/pebble/boot/ControllerTest.java
@@ -10,18 +10,17 @@ import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.springframework.beans.factory.annotation.Autowired;
-import org.springframework.boot.test.SpringApplicationConfiguration;
-import org.springframework.boot.test.WebIntegrationTest;
+import org.springframework.boot.test.context.SpringBootTest;
+import org.springframework.boot.test.context.SpringBootTest.WebEnvironment;
 import org.springframework.http.MediaType;
-import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
+import org.springframework.test.context.junit4.SpringRunner;
 import org.springframework.test.web.servlet.MockMvc;
 import org.springframework.test.web.servlet.MvcResult;
 import org.springframework.test.web.servlet.setup.MockMvcBuilders;
 import org.springframework.web.context.WebApplicationContext;
 
-@RunWith(SpringJUnit4ClassRunner.class)
-@SpringApplicationConfiguration(classes = Application.class)
-@WebIntegrationTest(randomPort = true)
+@RunWith(SpringRunner.class)
+@SpringBootTest(classes = Application.class, webEnvironment=WebEnvironment.RANDOM_PORT)
 public class ControllerTest {
 
   @Autowired
diff --git a/pebble-spring/pebble-spring-boot-starter/src/test/java/com/mitchellbosecke/pebble/boot/CoreTests.java b/pebble-spring/pebble-spring-boot-starter/src/test/java/com/mitchellbosecke/pebble/boot/CoreTests.java
index 2f046037..4f4ef0a2 100644
--- a/pebble-spring/pebble-spring-boot-starter/src/test/java/com/mitchellbosecke/pebble/boot/CoreTests.java
+++ b/pebble-spring/pebble-spring-boot-starter/src/test/java/com/mitchellbosecke/pebble/boot/CoreTests.java
@@ -6,11 +6,11 @@ import org.junit.Assert;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.springframework.beans.factory.annotation.Autowired;
-import org.springframework.boot.test.SpringApplicationConfiguration;
-import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
+import org.springframework.boot.test.context.SpringBootTest;
+import org.springframework.test.context.junit4.SpringRunner;
 
-@RunWith(SpringJUnit4ClassRunner.class)
-@SpringApplicationConfiguration(classes = NonWebApplication.class)
+@RunWith(SpringRunner.class)
+@SpringBootTest(classes = NonWebApplication.class)
 public class CoreTests {
 
   @Autowired
diff --git a/pebble-spring/pebble-spring-boot-starter/src/test/java/com/mitchellbosecke/pebble/boot/WebFilterHandler.java b/pebble-spring/pebble-spring-boot-starter/src/test/java/com/mitchellbosecke/pebble/boot/WebFilterHandler.java
index c07efa56..3d5bd9e9 100644
--- a/pebble-spring/pebble-spring-boot-starter/src/test/java/com/mitchellbosecke/pebble/boot/WebFilterHandler.java
+++ b/pebble-spring/pebble-spring-boot-starter/src/test/java/com/mitchellbosecke/pebble/boot/WebFilterHandler.java
@@ -8,7 +8,7 @@ import javax.servlet.annotation.WebFilter;
 import org.springframework.beans.factory.config.BeanDefinition;
 import org.springframework.beans.factory.support.BeanDefinitionBuilder;
 import org.springframework.beans.factory.support.BeanDefinitionRegistry;
-import org.springframework.boot.context.embedded.FilterRegistrationBean;
+import org.springframework.boot.web.servlet.FilterRegistrationBean;
 import org.springframework.util.StringUtils;
 
 class WebFilterHandler extends ServletComponentHandler {
diff --git a/pebble-spring/pebble-spring-boot-starter/src/test/java/com/mitchellbosecke/pebble/boot/WebListenerHandler.java b/pebble-spring/pebble-spring-boot-starter/src/test/java/com/mitchellbosecke/pebble/boot/WebListenerHandler.java
index 49565430..4658858e 100644
--- a/pebble-spring/pebble-spring-boot-starter/src/test/java/com/mitchellbosecke/pebble/boot/WebListenerHandler.java
+++ b/pebble-spring/pebble-spring-boot-starter/src/test/java/com/mitchellbosecke/pebble/boot/WebListenerHandler.java
@@ -5,7 +5,7 @@ import javax.servlet.annotation.WebListener;
 import org.springframework.beans.factory.config.BeanDefinition;
 import org.springframework.beans.factory.support.BeanDefinitionBuilder;
 import org.springframework.beans.factory.support.BeanDefinitionRegistry;
-import org.springframework.boot.context.embedded.ServletListenerRegistrationBean;
+import org.springframework.boot.web.servlet.ServletListenerRegistrationBean;
 
 class WebListenerHandler extends ServletComponentHandler {
 
diff --git a/pebble-spring/pebble-spring-boot-starter/src/test/java/com/mitchellbosecke/pebble/boot/WebServletHandler.java b/pebble-spring/pebble-spring-boot-starter/src/test/java/com/mitchellbosecke/pebble/boot/WebServletHandler.java
index 63891d7a..740c83bd 100644
--- a/pebble-spring/pebble-spring-boot-starter/src/test/java/com/mitchellbosecke/pebble/boot/WebServletHandler.java
+++ b/pebble-spring/pebble-spring-boot-starter/src/test/java/com/mitchellbosecke/pebble/boot/WebServletHandler.java
@@ -5,7 +5,7 @@ import javax.servlet.annotation.WebServlet;
 import org.springframework.beans.factory.config.BeanDefinition;
 import org.springframework.beans.factory.support.BeanDefinitionBuilder;
 import org.springframework.beans.factory.support.BeanDefinitionRegistry;
-import org.springframework.boot.context.embedded.ServletRegistrationBean;
+import org.springframework.boot.web.servlet.ServletRegistrationBean;
 import org.springframework.util.StringUtils;
 
 class WebServletHandler extends ServletComponentHandler {