From 0d39693c94c6bc8e7371d6e53e47e6f4dadceb9b Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Fri, 17 Aug 2018 10:51:39 +0200 Subject: [PATCH] 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. Original pull request: #307. --- ...efaultMethodInvokingMethodInterceptor.java | 160 +++++++++++------- ...hodInvokingMethodInterceptorUnitTests.java | 54 ++++++ 2 files changed, 155 insertions(+), 59 deletions(-) create mode 100644 src/test/java/org/springframework/data/projection/DefaultMethodInvokingMethodInterceptorUnitTests.java diff --git a/src/main/java/org/springframework/data/projection/DefaultMethodInvokingMethodInterceptor.java b/src/main/java/org/springframework/data/projection/DefaultMethodInvokingMethodInterceptor.java index 483f73ff5..d2aad992c 100644 --- a/src/main/java/org/springframework/data/projection/DefaultMethodInvokingMethodInterceptor.java +++ b/src/main/java/org/springframework/data/projection/DefaultMethodInvokingMethodInterceptor.java @@ -21,13 +21,13 @@ import java.lang.invoke.MethodHandles.Lookup; 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,37 +85,6 @@ public class DefaultMethodInvokingMethodInterceptor implements MethodInterceptor */ enum MethodHandleLookup { - /** - * 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 = getLookupConstructor(); - - /* - * (non-Javadoc) - * @see org.springframework.data.projection.DefaultMethodInvokingMethodInterceptor.MethodHandleLookup#lookup(java.lang.reflect.Method) - */ - @Override - MethodHandle lookup(Method method) throws ReflectiveOperationException { - - Constructor constructor = this.constructor - .orElseThrow(() -> new IllegalStateException("Could not obtain MethodHandles.lookup constructor")); - - return constructor.newInstance(method.getDeclaringClass()).unreflectSpecial(method, method.getDeclaringClass()); - } - - /* - * (non-Javadoc) - * @see org.springframework.data.projection.DefaultMethodInvokingMethodInterceptor.MethodHandleLookup#isAvailable() - */ - @Override - boolean isAvailable() { - return constructor.isPresent(); - } - }, - /** * Encapsulated {@link MethodHandle} lookup working on Java 9. */ @@ -131,10 +100,82 @@ public class DefaultMethodInvokingMethodInterceptor implements MethodInterceptor @Override MethodHandle lookup(Method method) throws ReflectiveOperationException { - MethodType methodType = MethodType.methodType(method.getReturnType(), method.getParameterTypes()); + if (privateLookupIn == null) { + throw new IllegalStateException("Could not obtain MethodHandles.privateLookupIn!"); + } - return getLookup(method.getDeclaringClass(), privateLookupIn).findSpecial(method.getDeclaringClass(), - method.getName(), methodType, method.getDeclaringClass()); + 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 Lazy> constructor = Lazy.of(MethodHandleLookup::getLookupConstructor); + + /* + * (non-Javadoc) + * @see org.springframework.data.projection.DefaultMethodInvokingMethodInterceptor.MethodHandleLookup#lookup(java.lang.reflect.Method) + */ + @Override + MethodHandle lookup(Method method) throws ReflectiveOperationException { + + if (!isAvailable()) { + throw new IllegalStateException("Could not obtain MethodHandles.lookup constructor!"); + } + + Constructor constructor = this.constructor.get(); + + return constructor.newInstance(method.getDeclaringClass()).unreflectSpecial(method, method.getDeclaringClass()); + } + + /* + * (non-Javadoc) + * @see org.springframework.data.projection.DefaultMethodInvokingMethodInterceptor.MethodHandleLookup#isAvailable() + */ + @Override + boolean isAvailable() { + return constructor.orElse(null) != null; + } + }, + + /** + * Fallback {@link MethodHandle} lookup using {@link MethodHandles#lookup() public lookup}. + * + * @since 2.1 + */ + FALLBACK { + + /* + * (non-Javadoc) + * @see org.springframework.data.projection.DefaultMethodInvokingMethodInterceptor.MethodHandleLookup#lookup(java.lang.reflect.Method) + */ + @Override + MethodHandle lookup(Method method) throws ReflectiveOperationException { + return doLookup(method, MethodHandles.lookup()); } /* @@ -145,23 +186,20 @@ public class DefaultMethodInvokingMethodInterceptor implements MethodInterceptor boolean isAvailable() { return true; } - - private Lookup getLookup(Class declaringClass, @Nullable Method privateLookupIn) { - - if (privateLookupIn == null) { - return MethodHandles.lookup(); - } - - Lookup lookup = MethodHandles.lookup(); - - try { - return (Lookup) privateLookupIn.invoke(MethodHandles.class, declaringClass, lookup); - } catch (ReflectiveOperationException e) { - return lookup; - } - } }; + private static MethodHandle doLookup(Method method, Lookup lookup) + throws NoSuchMethodException, IllegalAccessException { + + MethodType methodType = MethodType.methodType(method.getReturnType(), method.getParameterTypes()); + + 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 @@ public class DefaultMethodInvokingMethodInterceptor implements MethodInterceptor */ 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> getLookupConstructor() { + @Nullable + private static Constructor getLookupConstructor() { try { Constructor 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); diff --git a/src/test/java/org/springframework/data/projection/DefaultMethodInvokingMethodInterceptorUnitTests.java b/src/test/java/org/springframework/data/projection/DefaultMethodInvokingMethodInterceptorUnitTests.java new file mode 100644 index 000000000..70d5335f6 --- /dev/null +++ b/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(); + } +}