andreacavalli/netty5
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Reflective setAccessible(true) will produce scary warnings on the console when using java9+, dont do it

Browse Source 
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].
This commit is contained in:
Norman Maurer 2018-01-29 15:30:21 +01:00
parent bda6cb01c6
commit e72c197aa3
5 changed files with 26 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
codec-http2/src/test/java/io/netty/handler/codec/http2/Http2FrameCodecTest.java
View File

@ -719,7 +719,7 @@ public class Http2FrameCodecTest {
UpgradeEvent.class.getDeclaredConstructor(CharSequence.class, FullHttpRequest.class); UpgradeEvent.class.getDeclaredConstructor(CharSequence.class, FullHttpRequest.class);
// Check if we could make it accessible which may fail on java9. // 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( HttpServerUpgradeHandler.UpgradeEvent upgradeEvent = constructor.newInstance(
"HTTP/2", new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET, "/")); "HTTP/2", new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET, "/"));
@ -757,7 +757,7 @@ public class Http2FrameCodecTest {
UpgradeEvent.class.getDeclaredConstructor(CharSequence.class, FullHttpRequest.class); UpgradeEvent.class.getDeclaredConstructor(CharSequence.class, FullHttpRequest.class);
// Check if we could make it accessible which may fail on java9. // 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", "*"); String longString = new String(new char[70000]).replace("\0", "*");
DefaultFullHttpRequest request = DefaultFullHttpRequest request =

19
common/src/main/java/io/netty/util/internal/PlatformDependent0.java
View File

@ -46,6 +46,8 @@ final class PlatformDependent0 {
private static final Throwable UNSAFE_UNAVAILABILITY_CAUSE; private static final Throwable UNSAFE_UNAVAILABILITY_CAUSE;
private static final Object INTERNAL_UNSAFE; private static final Object INTERNAL_UNSAFE;
private static final boolean IS_EXPLICIT_TRY_REFLECTION_SET_ACCESSIBLE = explicitTryReflectionSetAccessible0();
static final Unsafe UNSAFE; static final Unsafe UNSAFE;
// constants borrowed from murmur3 // constants borrowed from murmur3
@ -84,7 +86,9 @@ final class PlatformDependent0 {
public Object run() { public Object run() {
try { try {
final Field unsafeField = Unsafe.class.getDeclaredField("theUnsafe"); 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) { if (cause != null) {
return cause; return cause;
} }
@ -218,7 +222,7 @@ final class PlatformDependent0 {
try { try {
final Constructor<?> constructor = final Constructor<?> constructor =
direct.getClass().getDeclaredConstructor(long.class, int.class); direct.getClass().getDeclaredConstructor(long.class, int.class);
Throwable cause = ReflectionUtil.trySetAccessible(constructor); Throwable cause = ReflectionUtil.trySetAccessible(constructor, true);
if (cause != null) { if (cause != null) {
return cause; return cause;
} }
@ -267,7 +271,7 @@ final class PlatformDependent0 {
Class<?> bitsClass = Class<?> bitsClass =
Class.forName("java.nio.Bits", false, getSystemClassLoader()); Class.forName("java.nio.Bits", false, getSystemClassLoader());
Method unalignedMethod = bitsClass.getDeclaredMethod("unaligned"); Method unalignedMethod = bitsClass.getDeclaredMethod("unaligned");
Throwable cause = ReflectionUtil.trySetAccessible(unalignedMethod); Throwable cause = ReflectionUtil.trySetAccessible(unalignedMethod, true);
if (cause != null) { if (cause != null) {
return cause; return cause;
} }
@ -797,6 +801,15 @@ final class PlatformDependent0 {
return android; 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() { static int javaVersion() {
return JAVA_VERSION; return JAVA_VERSION;
} }

5
common/src/main/java/io/netty/util/internal/ReflectionUtil.java
View File

@ -26,7 +26,10 @@ public final class ReflectionUtil {
* {@link java.lang.reflect.InaccessibleObjectException} and return it. * {@link java.lang.reflect.InaccessibleObjectException} and return it.
* The caller must check if it returns {@code null} and if not handle the returned exception. * 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 { try {
object.setAccessible(true); object.setAccessible(true);
return null; return null;

4
common/src/test/java/io/netty/util/internal/logging/Log4J2LoggerTest.java
View File

@ -67,7 +67,7 @@ public class Log4J2LoggerTest extends AbstractInternalLoggerTest<Logger> {
try { try {
Field field = clazz.getDeclaredField(fieldName); Field field = clazz.getDeclaredField(fieldName);
if (!field.isAccessible()) { if (!field.isAccessible()) {
Assume.assumeThat(ReflectionUtil.trySetAccessible(field), CoreMatchers.nullValue()); Assume.assumeThat(ReflectionUtil.trySetAccessible(field, true), CoreMatchers.nullValue());
} }
return (T) field.get(AbstractInternalLogger.class); return (T) field.get(AbstractInternalLogger.class);
} catch (ReflectiveOperationException e) { } catch (ReflectiveOperationException e) {
@ -87,7 +87,7 @@ public class Log4J2LoggerTest extends AbstractInternalLoggerTest<Logger> {
Method method = mockLog.getClass().getDeclaredMethod("setLevel", Level.class); Method method = mockLog.getClass().getDeclaredMethod("setLevel", Level.class);
if (!method.isAccessible()) { if (!method.isAccessible()) {
Assume.assumeThat(ReflectionUtil.trySetAccessible(method), CoreMatchers.nullValue()); Assume.assumeThat(ReflectionUtil.trySetAccessible(method, true), CoreMatchers.nullValue());
} }
method.invoke(mockLog, targetLevel); method.invoke(mockLog, targetLevel);
} }

4
transport/src/main/java/io/netty/channel/nio/NioEventLoop.java
View File

@ -214,11 +214,11 @@ public final class NioEventLoop extends SingleThreadEventLoop {
Field selectedKeysField = selectorImplClass.getDeclaredField("selectedKeys"); Field selectedKeysField = selectorImplClass.getDeclaredField("selectedKeys");
Field publicSelectedKeysField = selectorImplClass.getDeclaredField("publicSelectedKeys"); Field publicSelectedKeysField = selectorImplClass.getDeclaredField("publicSelectedKeys");
Throwable cause = ReflectionUtil.trySetAccessible(selectedKeysField); Throwable cause = ReflectionUtil.trySetAccessible(selectedKeysField, true);
if (cause != null) { if (cause != null) {
return cause; return cause;
} }
cause = ReflectionUtil.trySetAccessible(publicSelectedKeysField); cause = ReflectionUtil.trySetAccessible(publicSelectedKeysField, true);
if (cause != null) { if (cause != null) {
return cause; return cause;
} }