From ab21ca98fd7814bd014e7d8e03de8640f2529352 Mon Sep 17 00:00:00 2001 From: Gunnar Morling Date: Wed, 23 Jul 2014 10:13:18 +0200 Subject: [PATCH] HV-912 Not exposing accessible-made members --- .../internal/engine/ValidatorImpl.java | 104 ++++++++++++++++-- .../resolver/DefaultTraversableResolver.java | 2 +- .../metadata/core/MetaConstraint.java | 16 --- .../metadata/raw/ConstrainedField.java | 36 ------ 4 files changed, 97 insertions(+), 61 deletions(-) diff --git a/engine/src/main/java/org/hibernate/validator/internal/engine/ValidatorImpl.java b/engine/src/main/java/org/hibernate/validator/internal/engine/ValidatorImpl.java index 09c4fdd8f2..ddf9e72bbb 100644 --- a/engine/src/main/java/org/hibernate/validator/internal/engine/ValidatorImpl.java +++ b/engine/src/main/java/org/hibernate/validator/internal/engine/ValidatorImpl.java @@ -17,16 +17,20 @@ package org.hibernate.validator.internal.engine; import java.lang.annotation.ElementType; +import java.lang.reflect.AccessibleObject; import java.lang.reflect.Field; import java.lang.reflect.Member; import java.lang.reflect.Method; import java.lang.reflect.Type; +import java.security.AccessController; +import java.security.PrivilegedAction; import java.util.Arrays; import java.util.Collections; import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.concurrent.ConcurrentMap; import javax.validation.ConstraintValidatorFactory; import javax.validation.ConstraintViolation; import javax.validation.MessageInterpolator; @@ -37,20 +41,24 @@ import javax.validation.metadata.BeanDescriptor; import org.hibernate.validator.internal.engine.groups.Group; +import org.hibernate.validator.internal.engine.groups.Sequence; import org.hibernate.validator.internal.engine.groups.ValidationOrder; import org.hibernate.validator.internal.engine.groups.ValidationOrderGenerator; -import org.hibernate.validator.internal.engine.groups.Sequence; import org.hibernate.validator.internal.engine.resolver.SingleThreadCachedTraversableResolver; import org.hibernate.validator.internal.metadata.BeanMetaDataManager; import org.hibernate.validator.internal.metadata.aggregated.BeanMetaData; import org.hibernate.validator.internal.metadata.aggregated.MethodMetaData; import org.hibernate.validator.internal.metadata.aggregated.ParameterMetaData; import org.hibernate.validator.internal.metadata.core.MetaConstraint; +import org.hibernate.validator.internal.util.ConcurrentReferenceHashMap; import org.hibernate.validator.internal.util.Contracts; import org.hibernate.validator.internal.util.ReflectionHelper; import org.hibernate.validator.internal.util.TypeHelper; import org.hibernate.validator.internal.util.logging.Log; import org.hibernate.validator.internal.util.logging.LoggerFactory; +import org.hibernate.validator.internal.util.privilegedactions.GetDeclaredField; +import org.hibernate.validator.internal.util.privilegedactions.GetDeclaredMethod; +import org.hibernate.validator.internal.util.privilegedactions.SetAccessibility; import org.hibernate.validator.method.MethodConstraintViolation; import org.hibernate.validator.method.MethodValidator; import org.hibernate.validator.method.metadata.TypeDescriptor; @@ -108,6 +116,11 @@ public class ValidatorImpl implements Validator, MethodValidator { */ private final boolean failFast; + /** + * Keeps an accessible version for each non-accessible member whose value needs to be accessed during validation. + */ + private final ConcurrentMap accessibleMembers; + public ValidatorImpl(ConstraintValidatorFactory constraintValidatorFactory, MessageInterpolator messageInterpolator, TraversableResolver traversableResolver, BeanMetaDataManager beanMetaDataManager, boolean failFast) { this.constraintValidatorFactory = constraintValidatorFactory; this.messageInterpolator = messageInterpolator; @@ -115,7 +128,13 @@ public ValidatorImpl(ConstraintValidatorFactory constraintValidatorFactory, Mess this.beanMetaDataManager = beanMetaDataManager; this.failFast = failFast; - validationOrderGenerator = new ValidationOrderGenerator(); + this.validationOrderGenerator = new ValidationOrderGenerator(); + + this.accessibleMembers = new ConcurrentReferenceHashMap( + 100, + ConcurrentReferenceHashMap.ReferenceType.SOFT, + ConcurrentReferenceHashMap.ReferenceType.SOFT + ); } public final Set> validate(T object, Class... groups) { @@ -428,7 +447,9 @@ private void validateConstraintsForNonDefaultGroup(ValidationContext boolean validateConstraint(ValidationContext validationContext, ValueContext valueContext, MetaConstraint metaConstraint) { + private boolean validateConstraint(ValidationContext validationContext, + ValueContext valueContext, + MetaConstraint metaConstraint) { boolean validationSuccessful = true; if ( metaConstraint.getElementType() != ElementType.TYPE ) { @@ -437,7 +458,7 @@ private boolean validateConstraint(ValidationContext validationC if ( isValidationRequired( validationContext, valueContext, metaConstraint ) ) { @SuppressWarnings("unchecked") - V valueToValidate = (V) metaConstraint.getValue( valueContext.getCurrentBean() ); + V valueToValidate = (V) getValue( metaConstraint.getLocation().getMember(), valueContext.getCurrentBean() ); valueContext.setCurrentValidatedValue( valueToValidate ); validationSuccessful = metaConstraint.validateConstraint( validationContext, valueContext ); } @@ -461,7 +482,7 @@ private void validateCascadedConstraints(ValidationContext valid valueContext.appendNode( newNode ); if ( isCascadeRequired( validationContext, valueContext, member ) ) { - Object value = ReflectionHelper.getValue( member, valueContext.getCurrentBean() ); + Object value = getValue( member, valueContext.getCurrentBean() ); if ( value != null ) { Type type = value.getClass(); Iterator iter = createIteratorForCascadedValue( type, value, valueContext ); @@ -752,7 +773,10 @@ private int validatePropertyForNonDefaultGroup(ValueContext valu if ( isValidationRequired( validationContext, valueContext, metaConstraint ) ) { if ( valueContext.getCurrentBean() != null ) { @SuppressWarnings("unchecked") - V valueToValidate = (V) metaConstraint.getValue( valueContext.getCurrentBean() ); + V valueToValidate = (V) getValue( + metaConstraint.getLocation().getMember(), + valueContext.getCurrentBean() + ); valueContext.setCurrentValidatedValue( valueToValidate ); } metaConstraint.validateConstraint( validationContext, valueContext ); @@ -814,7 +838,10 @@ && isValidationRequired( validationContext, valueContext, metaConstraint ) ) { if ( valueContext.getCurrentBean() != null ) { @SuppressWarnings("unchecked") - V valueToValidate = (V) metaConstraint.getValue( valueContext.getCurrentBean() ); + V valueToValidate = (V) getValue( + metaConstraint.getLocation().getMember(), + valueContext.getCurrentBean() + ); valueContext.setCurrentValidatedValue( valueToValidate ); } boolean tmp = metaConstraint.validateConstraint( validationContext, valueContext ); @@ -1157,7 +1184,7 @@ private ValueContext collectMetaConstraintsForPath(Class claz for ( Member m : cascadedMembers ) { if ( ReflectionHelper.getPropertyName( m ).equals( elem.getName() ) ) { Type type = ReflectionHelper.typeOf( m ); - newValue = newValue == null ? null : ReflectionHelper.getValue( m, newValue ); + newValue = newValue == null ? null : getValue( m, newValue ); if ( elem.isInIterable() ) { if ( newValue != null && elem.getIndex() != null ) { newValue = ReflectionHelper.getIndexedValue( newValue, elem.getIndex() ); @@ -1284,5 +1311,66 @@ private boolean isCascadeRequired(ValidationContext validationContext, Val private boolean shouldFailFast(ValidationContext context) { return context.isFailFastModeEnabled() && !context.getFailingConstraints().isEmpty(); } + + private Object getValue(Member member, Object object) { + if ( member == null ) { + return object; + } + + member = getAccessible( member ); + + if ( member instanceof Method ) { + return ReflectionHelper.getValue( (Method) member, object ); + } + else if ( member instanceof Field ) { + return ReflectionHelper.getValue( (Field) member, object ); + } + return null; + } + + /** + * Returns an accessible version of the given member. Will be the given member itself in case it is accessible, + * otherwise a copy which is set accessible. These copies are maintained in the + * {@link ValidatorImpl#accessibleMembers} cache. + */ + private Member getAccessible(Member original) { + if ( ( (AccessibleObject) original ).isAccessible() ) { + return original; + } + + Member member = accessibleMembers.get( original ); + + if ( member != null ) { + return member; + } + + Class clazz = original.getDeclaringClass(); + + if ( original instanceof Field ) { + member = run( GetDeclaredField.action( clazz, original.getName() ) ); + } + else { + member = run( GetDeclaredMethod.action( clazz, original.getName() ) ); + } + + run( SetAccessibility.action( member ) ); + + Member cached = accessibleMembers.putIfAbsent( original, member ); + if ( cached != null ) { + member = cached; + } + + return member; + } + + /** + * Runs the given privileged action, using a privileged block if required. + *

+ * NOTE: This must never be changed into a publicly available method to avoid execution of arbitrary + * privileged actions within HV's protection domain. + */ + private T run(PrivilegedAction action) { + return System.getSecurityManager() != null ? AccessController.doPrivileged( action ) : action.run(); + } } diff --git a/engine/src/main/java/org/hibernate/validator/internal/engine/resolver/DefaultTraversableResolver.java b/engine/src/main/java/org/hibernate/validator/internal/engine/resolver/DefaultTraversableResolver.java index e8bd592939..f223072950 100644 --- a/engine/src/main/java/org/hibernate/validator/internal/engine/resolver/DefaultTraversableResolver.java +++ b/engine/src/main/java/org/hibernate/validator/internal/engine/resolver/DefaultTraversableResolver.java @@ -99,7 +99,7 @@ private void detectJPA() { // unfortunately there are several incomplete implementations out there (see HV-374) try { Object persistence = run( NewInstance.action( persistenceClass, "persistence provider" ) ); - ReflectionHelper.getValue(persistenceUtilGetter, persistence ); + ReflectionHelper.getValue( persistenceUtilGetter, persistence ); } catch ( Exception e ) { log.debugf( diff --git a/engine/src/main/java/org/hibernate/validator/internal/metadata/core/MetaConstraint.java b/engine/src/main/java/org/hibernate/validator/internal/metadata/core/MetaConstraint.java index 3693f3f5c5..c6c481c0c2 100644 --- a/engine/src/main/java/org/hibernate/validator/internal/metadata/core/MetaConstraint.java +++ b/engine/src/main/java/org/hibernate/validator/internal/metadata/core/MetaConstraint.java @@ -26,7 +26,6 @@ import org.hibernate.validator.internal.engine.ValueContext; import org.hibernate.validator.internal.metadata.descriptor.ConstraintDescriptorImpl; import org.hibernate.validator.internal.metadata.location.ConstraintLocation; -import org.hibernate.validator.internal.util.ReflectionHelper; /** * Instances of this class abstract the constraint type (class, method or field constraint) and give access to @@ -94,21 +93,6 @@ protected final Type typeOfAnnotatedElement() { return location.typeOfAnnotatedElement(); } - /** - * @param o the object from which to retrieve the value. - * - * @return Returns the value for this constraint from the specified object. Depending on the type either the value itself - * is returned of method or field access is used to access the value. - */ - public Object getValue(Object o) { - if ( location.getMember() == null ) { - return o; - } - else { - return ReflectionHelper.getValue( location.getMember(), o ); - } - } - @Override public boolean equals(Object o) { if ( this == o ) { diff --git a/engine/src/main/java/org/hibernate/validator/internal/metadata/raw/ConstrainedField.java b/engine/src/main/java/org/hibernate/validator/internal/metadata/raw/ConstrainedField.java index aab2d39066..3a65c3ea6b 100644 --- a/engine/src/main/java/org/hibernate/validator/internal/metadata/raw/ConstrainedField.java +++ b/engine/src/main/java/org/hibernate/validator/internal/metadata/raw/ConstrainedField.java @@ -16,14 +16,10 @@ */ package org.hibernate.validator.internal.metadata.raw; -import java.lang.reflect.Member; -import java.security.AccessController; -import java.security.PrivilegedAction; import java.util.Set; import org.hibernate.validator.internal.metadata.core.MetaConstraint; import org.hibernate.validator.internal.metadata.location.BeanConstraintLocation; -import org.hibernate.validator.internal.util.privilegedactions.SetAccessibility; /** * Represents a field of a Java type and all its associated meta-data relevant @@ -48,41 +44,9 @@ public ConstrainedField(ConfigurationSource source, boolean isCascading) { super( source, ConstrainedElementKind.FIELD, location, constraints, isCascading ); - - Member member = location.getMember(); - if ( member != null && isConstrained() ) { - run( SetAccessibility.action( member ) ); - } } public BeanConstraintLocation getLocation() { return (BeanConstraintLocation) super.getLocation(); } - - @Override - public boolean equals(Object obj) { - if ( this == obj ) { - return true; - } - if ( !super.equals( obj ) ) { - return false; - } - if ( getClass() != obj.getClass() ) { - return false; - } - ConstrainedField other = (ConstrainedField) obj; - if ( getLocation().getMember() == null ) { - if ( other.getLocation().getMember() != null ) { - return false; - } - } - else if ( !getLocation().getMember().equals( other.getLocation().getMember() ) ) { - return false; - } - return true; - } - - private T run(PrivilegedAction action) { - return System.getSecurityManager() != null ? AccessController.doPrivileged( action ) : action.run(); - } }