From e72c197aa3a88e97c10793ee7870775ab8807f92 Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Mon, 29 Jan 2018 15:30:21 +0100 Subject: [PATCH] Reflective setAccessible(true) will produce scary warnings on the console when using java9+, dont do it Motivation: Reflective setAccessible(true) will produce scary warnings on the console when using java9+, while netty still works. That said users may feel uncomfortable with these warnings, we should not try to do it by default when using java9+. Modifications: Add io.netty.tryReflectionSetAccessible system property which controls if setAccessible(...) will be used. By default it will bet set to false when using java9+. Result: Fixes [#7254]. --- .../codec/http2/Http2FrameCodecTest.java | 4 ++-- .../util/internal/PlatformDependent0.java | 19 ++++++++++++++++--- .../netty/util/internal/ReflectionUtil.java | 5 ++++- .../internal/logging/Log4J2LoggerTest.java | 4 ++-- .../io/netty/channel/nio/NioEventLoop.java | 4 ++-- 5 files changed, 26 insertions(+), 10 deletions(-) diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2FrameCodecTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2FrameCodecTest.java index 9dc0da4c26..44676fc82e 100644 --- a/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2FrameCodecTest.java +++ b/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2FrameCodecTest.java @@ -719,7 +719,7 @@ public class Http2FrameCodecTest { UpgradeEvent.class.getDeclaredConstructor(CharSequence.class, FullHttpRequest.class); // Check if we could make it accessible which may fail on java9. - Assume.assumeTrue(ReflectionUtil.trySetAccessible(constructor) == null); + Assume.assumeTrue(ReflectionUtil.trySetAccessible(constructor, true) == null); HttpServerUpgradeHandler.UpgradeEvent upgradeEvent = constructor.newInstance( "HTTP/2", new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET, "/")); @@ -757,7 +757,7 @@ public class Http2FrameCodecTest { UpgradeEvent.class.getDeclaredConstructor(CharSequence.class, FullHttpRequest.class); // Check if we could make it accessible which may fail on java9. - Assume.assumeTrue(ReflectionUtil.trySetAccessible(constructor) == null); + Assume.assumeTrue(ReflectionUtil.trySetAccessible(constructor, true) == null); String longString = new String(new char[70000]).replace("\0", "*"); DefaultFullHttpRequest request = diff --git a/common/src/main/java/io/netty/util/internal/PlatformDependent0.java b/common/src/main/java/io/netty/util/internal/PlatformDependent0.java index 6457f5f7c2..f56a1a9ca3 100644 --- a/common/src/main/java/io/netty/util/internal/PlatformDependent0.java +++ b/common/src/main/java/io/netty/util/internal/PlatformDependent0.java @@ -46,6 +46,8 @@ final class PlatformDependent0 { private static final Throwable UNSAFE_UNAVAILABILITY_CAUSE; private static final Object INTERNAL_UNSAFE; + private static final boolean IS_EXPLICIT_TRY_REFLECTION_SET_ACCESSIBLE = explicitTryReflectionSetAccessible0(); + static final Unsafe UNSAFE; // constants borrowed from murmur3 @@ -84,7 +86,9 @@ final class PlatformDependent0 { public Object run() { try { final Field unsafeField = Unsafe.class.getDeclaredField("theUnsafe"); - Throwable cause = ReflectionUtil.trySetAccessible(unsafeField); + // We always want to try using Unsafe as the access still works on java9 as well and + // we need it for out native-transports and many optimizations. + Throwable cause = ReflectionUtil.trySetAccessible(unsafeField, false); if (cause != null) { return cause; } @@ -218,7 +222,7 @@ final class PlatformDependent0 { try { final Constructor constructor = direct.getClass().getDeclaredConstructor(long.class, int.class); - Throwable cause = ReflectionUtil.trySetAccessible(constructor); + Throwable cause = ReflectionUtil.trySetAccessible(constructor, true); if (cause != null) { return cause; } @@ -267,7 +271,7 @@ final class PlatformDependent0 { Class bitsClass = Class.forName("java.nio.Bits", false, getSystemClassLoader()); Method unalignedMethod = bitsClass.getDeclaredMethod("unaligned"); - Throwable cause = ReflectionUtil.trySetAccessible(unalignedMethod); + Throwable cause = ReflectionUtil.trySetAccessible(unalignedMethod, true); if (cause != null) { return cause; } @@ -797,6 +801,15 @@ final class PlatformDependent0 { return android; } + private static boolean explicitTryReflectionSetAccessible0() { + // we disable reflective access + return SystemPropertyUtil.getBoolean("io.netty.tryReflectionSetAccessible", javaVersion() < 9); + } + + static boolean isExplicitTryReflectionSetAccessible() { + return IS_EXPLICIT_TRY_REFLECTION_SET_ACCESSIBLE; + } + static int javaVersion() { return JAVA_VERSION; } diff --git a/common/src/main/java/io/netty/util/internal/ReflectionUtil.java b/common/src/main/java/io/netty/util/internal/ReflectionUtil.java index ef39b35215..8c898c0fdc 100644 --- a/common/src/main/java/io/netty/util/internal/ReflectionUtil.java +++ b/common/src/main/java/io/netty/util/internal/ReflectionUtil.java @@ -26,7 +26,10 @@ public final class ReflectionUtil { * {@link java.lang.reflect.InaccessibleObjectException} and return it. * The caller must check if it returns {@code null} and if not handle the returned exception. */ - public static Throwable trySetAccessible(AccessibleObject object) { + public static Throwable trySetAccessible(AccessibleObject object, boolean checkAccessible) { + if (checkAccessible && !PlatformDependent0.isExplicitTryReflectionSetAccessible()) { + return new UnsupportedOperationException("Reflective setAccessible(true) disabled"); + } try { object.setAccessible(true); return null; diff --git a/common/src/test/java/io/netty/util/internal/logging/Log4J2LoggerTest.java b/common/src/test/java/io/netty/util/internal/logging/Log4J2LoggerTest.java index 99c42ed845..b561993ebd 100644 --- a/common/src/test/java/io/netty/util/internal/logging/Log4J2LoggerTest.java +++ b/common/src/test/java/io/netty/util/internal/logging/Log4J2LoggerTest.java @@ -67,7 +67,7 @@ public class Log4J2LoggerTest extends AbstractInternalLoggerTest { try { Field field = clazz.getDeclaredField(fieldName); if (!field.isAccessible()) { - Assume.assumeThat(ReflectionUtil.trySetAccessible(field), CoreMatchers.nullValue()); + Assume.assumeThat(ReflectionUtil.trySetAccessible(field, true), CoreMatchers.nullValue()); } return (T) field.get(AbstractInternalLogger.class); } catch (ReflectiveOperationException e) { @@ -87,7 +87,7 @@ public class Log4J2LoggerTest extends AbstractInternalLoggerTest { Method method = mockLog.getClass().getDeclaredMethod("setLevel", Level.class); if (!method.isAccessible()) { - Assume.assumeThat(ReflectionUtil.trySetAccessible(method), CoreMatchers.nullValue()); + Assume.assumeThat(ReflectionUtil.trySetAccessible(method, true), CoreMatchers.nullValue()); } method.invoke(mockLog, targetLevel); } diff --git a/transport/src/main/java/io/netty/channel/nio/NioEventLoop.java b/transport/src/main/java/io/netty/channel/nio/NioEventLoop.java index 3da502ab9e..c4434cf448 100644 --- a/transport/src/main/java/io/netty/channel/nio/NioEventLoop.java +++ b/transport/src/main/java/io/netty/channel/nio/NioEventLoop.java @@ -214,11 +214,11 @@ public final class NioEventLoop extends SingleThreadEventLoop { Field selectedKeysField = selectorImplClass.getDeclaredField("selectedKeys"); Field publicSelectedKeysField = selectorImplClass.getDeclaredField("publicSelectedKeys"); - Throwable cause = ReflectionUtil.trySetAccessible(selectedKeysField); + Throwable cause = ReflectionUtil.trySetAccessible(selectedKeysField, true); if (cause != null) { return cause; } - cause = ReflectionUtil.trySetAccessible(publicSelectedKeysField); + cause = ReflectionUtil.trySetAccessible(publicSelectedKeysField, true); if (cause != null) { return cause; }