From f58968c0a2f31603ee22305405bf9e595f35c579 Mon Sep 17 00:00:00 2001 From: Brett Meyer Date: Fri, 10 Jan 2014 15:59:00 -0500 Subject: [PATCH] HHH-5289 remove unnecessary security checks in property accessors --- .../BeanValidationIntegrator.java | 4 +-- .../internal/util/ReflectHelper.java | 8 ++--- .../property/BasicPropertyAccessor.java | 6 ++-- .../property/DirectPropertyAccessor.java | 4 +-- .../javassist/JavassistLazyInitializer.java | 4 +-- .../component/ComponentTuplizerFactory.java | 13 +++----- .../tuple/entity/EntityTuplizerFactory.java | 13 +++----- .../internal/jpa/CallbackProcessorImpl.java | 6 ++-- .../internal/jpa/LegacyCallbackProcessor.java | 2 +- .../internal/metamodel/MetadataContext.java | 6 ++-- .../testing/junit4/TestClassMetadata.java | 32 ++++++++----------- 11 files changed, 37 insertions(+), 61 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/cfg/beanvalidation/BeanValidationIntegrator.java b/hibernate-core/src/main/java/org/hibernate/cfg/beanvalidation/BeanValidationIntegrator.java index e30cd765c2..5ca1b6b5ca 100644 --- a/hibernate-core/src/main/java/org/hibernate/cfg/beanvalidation/BeanValidationIntegrator.java +++ b/hibernate-core/src/main/java/org/hibernate/cfg/beanvalidation/BeanValidationIntegrator.java @@ -69,9 +69,7 @@ public class BeanValidationIntegrator implements Integrator { final Class activatorClass = BeanValidationIntegrator.class.getClassLoader().loadClass( ACTIVATOR_CLASS_NAME ); try { final Method validateMethod = activatorClass.getMethod( VALIDATE_SUPPLIED_FACTORY_METHOD_NAME, Object.class ); - if ( ! validateMethod.isAccessible() ) { - validateMethod.setAccessible( true ); - } + validateMethod.setAccessible( true ); try { validateMethod.invoke( null, object ); } diff --git a/hibernate-core/src/main/java/org/hibernate/internal/util/ReflectHelper.java b/hibernate-core/src/main/java/org/hibernate/internal/util/ReflectHelper.java index a1b289b540..58f8b9d3e7 100644 --- a/hibernate-core/src/main/java/org/hibernate/internal/util/ReflectHelper.java +++ b/hibernate-core/src/main/java/org/hibernate/internal/util/ReflectHelper.java @@ -303,9 +303,7 @@ public final class ReflectHelper { try { Constructor constructor = clazz.getDeclaredConstructor( NO_PARAM_SIGNATURE ); - if ( !isPublic( clazz, constructor ) ) { - constructor.setAccessible( true ); - } + constructor.setAccessible( true ); return constructor; } catch ( NoSuchMethodException nme ) { @@ -363,9 +361,7 @@ public final class ReflectHelper { } } if ( found ) { - if ( !isPublic( clazz, constructor ) ) { - constructor.setAccessible( true ); - } + constructor.setAccessible( true ); return constructor; } } diff --git a/hibernate-core/src/main/java/org/hibernate/property/BasicPropertyAccessor.java b/hibernate-core/src/main/java/org/hibernate/property/BasicPropertyAccessor.java index ea835a736f..31db69bdf7 100644 --- a/hibernate-core/src/main/java/org/hibernate/property/BasicPropertyAccessor.java +++ b/hibernate-core/src/main/java/org/hibernate/property/BasicPropertyAccessor.java @@ -260,7 +260,7 @@ public class BasicPropertyAccessor implements PropertyAccessor { Method method = setterMethod(theClass, propertyName); if (method!=null) { - if ( !ReflectHelper.isPublic(theClass, method) ) method.setAccessible(true); + method.setAccessible(true); return new BasicSetter(theClass, method, propertyName); } else { @@ -325,9 +325,7 @@ public class BasicPropertyAccessor implements PropertyAccessor { Method method = getterMethod(theClass, propertyName); if (method!=null) { - if ( !ReflectHelper.isPublic( theClass, method ) ) { - method.setAccessible(true); - } + method.setAccessible(true); return new BasicGetter(theClass, method, propertyName); } else { diff --git a/hibernate-core/src/main/java/org/hibernate/property/DirectPropertyAccessor.java b/hibernate-core/src/main/java/org/hibernate/property/DirectPropertyAccessor.java index 1858960b27..89a358a996 100644 --- a/hibernate-core/src/main/java/org/hibernate/property/DirectPropertyAccessor.java +++ b/hibernate-core/src/main/java/org/hibernate/property/DirectPropertyAccessor.java @@ -157,7 +157,7 @@ public class DirectPropertyAccessor implements PropertyAccessor { catch (NoSuchFieldException nsfe) { field = getField( clazz, clazz.getSuperclass(), name ); } - if ( !ReflectHelper.isPublic(clazz, field) ) field.setAccessible(true); + field.setAccessible(true); return field; } @@ -172,7 +172,7 @@ public class DirectPropertyAccessor implements PropertyAccessor { catch (NoSuchFieldException nsfe) { field = getField( root, clazz.getSuperclass(), name ); } - if ( !ReflectHelper.isPublic(clazz, field) ) field.setAccessible(true); + field.setAccessible(true); return field; } diff --git a/hibernate-core/src/main/java/org/hibernate/proxy/pojo/javassist/JavassistLazyInitializer.java b/hibernate-core/src/main/java/org/hibernate/proxy/pojo/javassist/JavassistLazyInitializer.java index b539e71789..a60646e136 100644 --- a/hibernate-core/src/main/java/org/hibernate/proxy/pojo/javassist/JavassistLazyInitializer.java +++ b/hibernate-core/src/main/java/org/hibernate/proxy/pojo/javassist/JavassistLazyInitializer.java @@ -192,9 +192,7 @@ public class JavassistLazyInitializer extends BasicLazyInitializer implements Me returnValue = thisMethod.invoke( target, args ); } else { - if ( !thisMethod.isAccessible() ) { - thisMethod.setAccessible( true ); - } + thisMethod.setAccessible( true ); returnValue = thisMethod.invoke( target, args ); } diff --git a/hibernate-core/src/main/java/org/hibernate/tuple/component/ComponentTuplizerFactory.java b/hibernate-core/src/main/java/org/hibernate/tuple/component/ComponentTuplizerFactory.java index c912708b3a..bd92250177 100644 --- a/hibernate-core/src/main/java/org/hibernate/tuple/component/ComponentTuplizerFactory.java +++ b/hibernate-core/src/main/java/org/hibernate/tuple/component/ComponentTuplizerFactory.java @@ -135,14 +135,11 @@ public class ComponentTuplizerFactory implements Serializable { Constructor constructor = null; try { constructor = clazz.getDeclaredConstructor( COMPONENT_TUP_CTOR_SIG ); - if ( ! ReflectHelper.isPublic( constructor ) ) { - try { - // found a constructor, but it was not publicly accessible so try to request accessibility - constructor.setAccessible( true ); - } - catch ( SecurityException e ) { - constructor = null; - } + try { + constructor.setAccessible( true ); + } + catch ( SecurityException e ) { + constructor = null; } } catch ( NoSuchMethodException ignore ) { diff --git a/hibernate-core/src/main/java/org/hibernate/tuple/entity/EntityTuplizerFactory.java b/hibernate-core/src/main/java/org/hibernate/tuple/entity/EntityTuplizerFactory.java index 0cf17f85a6..8be52bd0c8 100644 --- a/hibernate-core/src/main/java/org/hibernate/tuple/entity/EntityTuplizerFactory.java +++ b/hibernate-core/src/main/java/org/hibernate/tuple/entity/EntityTuplizerFactory.java @@ -227,14 +227,11 @@ public class EntityTuplizerFactory implements Serializable { Constructor constructor = null; try { constructor = clazz.getDeclaredConstructor( constructorArgs ); - if ( ! ReflectHelper.isPublic( constructor ) ) { - try { - // found a constructor, but it was not publicly accessible so try to request accessibility - constructor.setAccessible( true ); - } - catch ( SecurityException e ) { - constructor = null; - } + try { + constructor.setAccessible( true ); + } + catch ( SecurityException e ) { + constructor = null; } } catch ( NoSuchMethodException ignore ) { diff --git a/hibernate-entitymanager/src/main/java/org/hibernate/jpa/event/internal/jpa/CallbackProcessorImpl.java b/hibernate-entitymanager/src/main/java/org/hibernate/jpa/event/internal/jpa/CallbackProcessorImpl.java index 6090370205..d7bcda53f5 100644 --- a/hibernate-entitymanager/src/main/java/org/hibernate/jpa/event/internal/jpa/CallbackProcessorImpl.java +++ b/hibernate-entitymanager/src/main/java/org/hibernate/jpa/event/internal/jpa/CallbackProcessorImpl.java @@ -132,9 +132,7 @@ public class CallbackProcessorImpl implements CallbackProcessor { if (argType != Object.class && argType != entityClass) { continue; } - if (!method.isAccessible()) { - method.setAccessible( true ); - } + method.setAccessible( true ); return new ListenerCallback( listenerInstance, method ); } @@ -151,7 +149,7 @@ public class CallbackProcessorImpl implements CallbackProcessor { for (Method method : callbackClass.getDeclaredMethods()) { if (!method.getName().equals(methodName)) continue; if (method.getParameterTypes().length != 0) continue; - if (!method.isAccessible()) method.setAccessible(true); + method.setAccessible(true); return new EntityCallback(method); } return null; diff --git a/hibernate-entitymanager/src/main/java/org/hibernate/jpa/event/internal/jpa/LegacyCallbackProcessor.java b/hibernate-entitymanager/src/main/java/org/hibernate/jpa/event/internal/jpa/LegacyCallbackProcessor.java index a72c155f8e..ce507d1a87 100644 --- a/hibernate-entitymanager/src/main/java/org/hibernate/jpa/event/internal/jpa/LegacyCallbackProcessor.java +++ b/hibernate-entitymanager/src/main/java/org/hibernate/jpa/event/internal/jpa/LegacyCallbackProcessor.java @@ -104,7 +104,7 @@ public class LegacyCallbackProcessor implements CallbackProcessor { .getName() + " - " + xMethod ); } - if (!method.isAccessible()) method.setAccessible(true); + method.setAccessible(true); log.debugf("Adding %s as %s callback for entity %s", methodName, annotation.getSimpleName(), diff --git a/hibernate-entitymanager/src/main/java/org/hibernate/jpa/internal/metamodel/MetadataContext.java b/hibernate-entitymanager/src/main/java/org/hibernate/jpa/internal/metamodel/MetadataContext.java index f2a87370e1..eb0e2c59e4 100755 --- a/hibernate-entitymanager/src/main/java/org/hibernate/jpa/internal/metamodel/MetadataContext.java +++ b/hibernate-entitymanager/src/main/java/org/hibernate/jpa/internal/metamodel/MetadataContext.java @@ -428,10 +428,8 @@ class MetadataContext { ? metamodelClass.getField( name ) : metamodelClass.getDeclaredField( name ); try { - if ( ! field.isAccessible() ) { - // should be public anyway, but to be sure... - field.setAccessible( true ); - } + // should be public anyway, but to be sure... + field.setAccessible( true ); field.set( null, attribute ); } catch ( IllegalAccessException e ) { diff --git a/hibernate-testing/src/main/java/org/hibernate/testing/junit4/TestClassMetadata.java b/hibernate-testing/src/main/java/org/hibernate/testing/junit4/TestClassMetadata.java index aea68ca3f7..ad3d424056 100644 --- a/hibernate-testing/src/main/java/org/hibernate/testing/junit4/TestClassMetadata.java +++ b/hibernate-testing/src/main/java/org/hibernate/testing/junit4/TestClassMetadata.java @@ -83,13 +83,11 @@ public class TestClassMetadata { } private void ensureAccessibility(Method method) { - if ( !method.isAccessible() ) { - try { - method.setAccessible( true ); - } - catch (Exception ignored) { - // ignore for now - } + try { + method.setAccessible( true ); + } + catch (Exception ignored) { + // ignore for now } } @@ -141,17 +139,15 @@ public class TestClassMetadata { ) ); } - if ( !method.isAccessible() ) { - try { - method.setAccessible( true ); - } - catch (Exception e) { - errors.add( - new InvalidMethodForAnnotationException( - type.buildTypeMarker() + " attached to inaccessible method and unable to make accessible" - ) - ); - } + try { + method.setAccessible( true ); + } + catch (Exception e) { + errors.add( + new InvalidMethodForAnnotationException( + type.buildTypeMarker() + " attached to inaccessible method and unable to make accessible" + ) + ); } }