DATACMNS-1376 - Fix illegal access warning in DefaultMethodInvokingMethodInterceptor on Java 9 and higher.

We now attempt to use private MethodHandles lookup as the first mechanism to resolve a MethodHandle for default interface methods and fall back to reflection-based Lookup construction if private lookup is not available. Reflective availability is checked lazily to prevent illegal access on enum constant construction. This approach prevents an illegal access which was logged by attempting a reflection-based lookup first.

We also introduced a FALLBACK mechanism to split encapsulated access from a fallback mechanism.

Author: mp911de

src/main/java/org/springframework/data/projection/DefaultMethodInvokingMethodInterceptor.java

@@ -21,13 +21,13 @@
 import java.lang.invoke.MethodType;
 import java.lang.reflect.Constructor;
 import java.lang.reflect.Method;
-import java.util.Arrays;
+import java.lang.reflect.Modifier;
 import java.util.Map;
-import java.util.Optional;
 
 import org.aopalliance.intercept.MethodInterceptor;
 import org.aopalliance.intercept.MethodInvocation;
 import org.springframework.aop.ProxyMethodInvocation;
+import org.springframework.data.util.Lazy;
 import org.springframework.lang.Nullable;
 import org.springframework.util.ConcurrentReferenceHashMap;
 import org.springframework.util.ConcurrentReferenceHashMap.ReferenceType;
@@ -85,13 +85,56 @@ private MethodHandle getMethodHandle(Method method) throws Exception {
 	 */
 	enum MethodHandleLookup {
 
+		/**
+		 * Encapsulated {@link MethodHandle} lookup working on Java 9.
+		 */
+		ENCAPSULATED {
+
+			private final @Nullable Method privateLookupIn = ReflectionUtils.findMethod(MethodHandles.class,
+					"privateLookupIn", Class.class, Lookup.class);
+
+			/*
+			 * (non-Javadoc)
+			 * @see org.springframework.data.projection.DefaultMethodInvokingMethodInterceptor.MethodHandleLookup#lookup(java.lang.reflect.Method)
+			 */
+			@Override
+			MethodHandle lookup(Method method) throws ReflectiveOperationException {
+
+				if (privateLookupIn == null) {
+					throw new IllegalStateException("Could not obtain MethodHandles.privateLookupIn!");
+				}
+
+				return doLookup(method, getLookup(method.getDeclaringClass(), privateLookupIn));
+			}
+
+			/*
+			 * (non-Javadoc)
+			 * @see org.springframework.data.projection.DefaultMethodInvokingMethodInterceptor.MethodHandleLookup#isAvailable()
+			 */
+			@Override
+			boolean isAvailable() {
+				return privateLookupIn != null;
+			}
+
+			private Lookup getLookup(Class<?> declaringClass, Method privateLookupIn) {
+
+				Lookup lookup = MethodHandles.lookup();
+
+				try {
+					return (Lookup) privateLookupIn.invoke(MethodHandles.class, declaringClass, lookup);
+				} catch (ReflectiveOperationException e) {
+					return lookup;
+				}
+			}
+		},
+
 		/**
 		 * Open (via reflection construction of {@link MethodHandles.Lookup}) method handle lookup. Works with Java 8 and
 		 * with Java 9 permitting illegal access.
 		 */
 		OPEN {
 
-			private final Optional<Constructor<Lookup>> constructor = getLookupConstructor();
+			private final Lazy<Constructor<Lookup>> constructor = Lazy.of(MethodHandleLookup::getLookupConstructor);
 
 			/*
 			 * (non-Javadoc)
@@ -100,8 +143,11 @@ enum MethodHandleLookup {
 			@Override
 			MethodHandle lookup(Method method) throws ReflectiveOperationException {
 
-				Constructor<Lookup> constructor = this.constructor
-						.orElseThrow(() -> new IllegalStateException("Could not obtain MethodHandles.lookup constructor"));
+				if (!isAvailable()) {
+					throw new IllegalStateException("Could not obtain MethodHandles.lookup constructor!");
+				}
+
+				Constructor<Lookup> constructor = this.constructor.get();
 
 				return constructor.newInstance(method.getDeclaringClass()).unreflectSpecial(method, method.getDeclaringClass());
 			}
@@ -112,29 +158,24 @@ MethodHandle lookup(Method method) throws ReflectiveOperationException {
 			 */
 			@Override
 			boolean isAvailable() {
-				return constructor.isPresent();
+				return constructor.orElse(null) != null;
 			}
 		},
 
 		/**
-		 * Encapsulated {@link MethodHandle} lookup working on Java 9.
+		 * Fallback {@link MethodHandle} lookup using {@link MethodHandles#lookup() public lookup}.
+		 *
+		 * @since 2.1
 		 */
-		ENCAPSULATED {
-
-			private final @Nullable Method privateLookupIn = ReflectionUtils.findMethod(MethodHandles.class,
-					"privateLookupIn", Class.class, Lookup.class);
+		FALLBACK {
 
 			/*
 			 * (non-Javadoc)
 			 * @see org.springframework.data.projection.DefaultMethodInvokingMethodInterceptor.MethodHandleLookup#lookup(java.lang.reflect.Method)
 			 */
 			@Override
 			MethodHandle lookup(Method method) throws ReflectiveOperationException {
-
-				MethodType methodType = MethodType.methodType(method.getReturnType(), method.getParameterTypes());
-
-				return getLookup(method.getDeclaringClass(), privateLookupIn).findSpecial(method.getDeclaringClass(),
-						method.getName(), methodType, method.getDeclaringClass());
+				return doLookup(method, MethodHandles.lookup());
 			}
 
 			/*
@@ -145,22 +186,19 @@ MethodHandle lookup(Method method) throws ReflectiveOperationException {
 			boolean isAvailable() {
 				return true;
 			}
+		};
 
-			private Lookup getLookup(Class<?> declaringClass, @Nullable Method privateLookupIn) {
+		private static MethodHandle doLookup(Method method, Lookup lookup)
+				throws NoSuchMethodException, IllegalAccessException {
 
-				if (privateLookupIn == null) {
-					return MethodHandles.lookup();
-				}
-
-				Lookup lookup = MethodHandles.lookup();
+			MethodType methodType = MethodType.methodType(method.getReturnType(), method.getParameterTypes());
 
-				try {
-					return (Lookup) privateLookupIn.invoke(MethodHandles.class, declaringClass, lookup);
-				} catch (ReflectiveOperationException e) {
-					return lookup;
-				}
+			if (Modifier.isStatic(method.getModifiers())) {
+				return lookup.findStatic(method.getDeclaringClass(), method.getName(), methodType);
 			}
-		};
+
+			return lookup.findSpecial(method.getDeclaringClass(), method.getName(), methodType, method.getDeclaringClass());
+		}
 
 		/**
 		 * Lookup a {@link MethodHandle} given {@link Method} to look up.
@@ -184,26 +222,30 @@ private Lookup getLookup(Class<?> declaringClass, @Nullable Method privateLookup
 		 */
 		public static MethodHandleLookup getMethodHandleLookup() {
 
-			return Arrays.stream(MethodHandleLookup.values()) //
-					.filter(it -> it.isAvailable()) //
-					.findFirst() //
-					.orElseThrow(() -> new IllegalStateException("No MethodHandleLookup available!"));
+			for (MethodHandleLookup it : MethodHandleLookup.values()) {
+
+				if (it.isAvailable()) {
+					return it;
+				}
+			}
+
+			throw new IllegalStateException("No MethodHandleLookup available!");
 		}
 
-		private static Optional<Constructor<Lookup>> getLookupConstructor() {
+		@Nullable
+		private static Constructor<Lookup> getLookupConstructor() {
 
 			try {
 
 				Constructor<Lookup> constructor = Lookup.class.getDeclaredConstructor(Class.class);
 				ReflectionUtils.makeAccessible(constructor);
 
-				return Optional.of(constructor);
-
+				return constructor;
 			} catch (Exception ex) {
 
 				// this is the signal that we are on Java 9 (encapsulated) and can't use the accessible constructor approach.
 				if (ex.getClass().getName().equals("java.lang.reflect.InaccessibleObjectException")) {
-					return Optional.empty();
+					return null;
 				}
 
 				throw new IllegalStateException(ex);

src/test/java/org/springframework/data/projection/DefaultMethodInvokingMethodInterceptorUnitTests.java

@@ -0,0 +1,54 @@
+/*
+ * Copyright 2018 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.springframework.data.projection;
+
+import static org.assertj.core.api.Assertions.*;
+import static org.junit.Assume.*;
+
+import org.junit.Test;
+import org.springframework.data.projection.DefaultMethodInvokingMethodInterceptor.MethodHandleLookup;
+import org.springframework.data.util.Version;
+
+/**
+ * Unit tests for {@link DefaultMethodInvokingMethodInterceptor}.
+ *
+ * @author Mark Paluch
+ */
+public class DefaultMethodInvokingMethodInterceptorUnitTests {
+
+	@Test // DATACMNS-1376
+	public void shouldApplyEncapsulatedLookupOnJava9AndHigher() {
+
+		Version version = Version.parse(System.getProperty("java.version"));
+
+		assumeTrue(version.isGreaterThanOrEqualTo(Version.parse("9.0")));
+
+		assertThat(MethodHandleLookup.getMethodHandleLookup()).isEqualTo(MethodHandleLookup.ENCAPSULATED);
+		assertThat(MethodHandleLookup.ENCAPSULATED.isAvailable()).isTrue();
+	}
+
+	@Test // DATACMNS-1376
+	public void shouldApplyOpenLookupOnJava8() {
+
+		Version version = Version.parse(System.getProperty("java.version"));
+
+		assumeTrue(version.isLessThan(Version.parse("1.8.9999")));
+
+		assertThat(MethodHandleLookup.getMethodHandleLookup()).isEqualTo(MethodHandleLookup.OPEN);
+		assertThat(MethodHandleLookup.OPEN.isAvailable()).isTrue();
+		assertThat(MethodHandleLookup.ENCAPSULATED.isAvailable()).isFalse();
+	}
+}