Skip to content

Commit

Permalink
HV-912 Not exposing accessible-made members
Browse files Browse the repository at this point in the history
  • Loading branch information
gunnarmorling committed Jul 23, 2014
1 parent fd4eaed commit c9525ca
Show file tree
Hide file tree
Showing 9 changed files with 104 additions and 70 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,21 @@
package org.hibernate.validator.internal.engine;

import java.lang.annotation.ElementType;
import java.lang.reflect.AccessibleObject;
import java.lang.reflect.Constructor;
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;
Expand Down Expand Up @@ -58,12 +64,17 @@
import org.hibernate.validator.internal.metadata.facets.Cascadable;
import org.hibernate.validator.internal.metadata.facets.Validatable;
import org.hibernate.validator.internal.metadata.raw.ExecutableElement;
import org.hibernate.validator.internal.util.ConcurrentReferenceHashMap;
import org.hibernate.validator.internal.util.ConcurrentReferenceHashMap.ReferenceType;
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.TypeResolutionHelper;
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.spi.valuehandling.ValidatedValueUnwrapper;

import com.fasterxml.classmate.ResolvedType;
Expand Down Expand Up @@ -142,6 +153,11 @@ public class ValidatorImpl implements Validator, ExecutableValidator {
*/
private final List<ValidatedValueUnwrapper<?>> validatedValueHandlers;

/**
* Keeps an accessible version for each non-accessible member whose value needs to be accessed during validation.
*/
private final ConcurrentMap<Member, Member> accessibleMembers;

public ValidatorImpl(ConstraintValidatorFactory constraintValidatorFactory,
MessageInterpolator messageInterpolator,
TraversableResolver traversableResolver,
Expand All @@ -162,6 +178,12 @@ public ValidatorImpl(ConstraintValidatorFactory constraintValidatorFactory,
this.failFast = failFast;

validationOrderGenerator = new ValidationOrderGenerator();

this.accessibleMembers = new ConcurrentReferenceHashMap<Member, Member>(
100,
ReferenceType.SOFT,
ReferenceType.SOFT
);
}

@Override
Expand Down Expand Up @@ -519,7 +541,7 @@ private boolean validateConstraint(ValidationContext<?> validationContext,

if ( isValidationRequired( validationContext, valueContext, metaConstraint ) ) {
if ( valueContext.getCurrentBean() != null ) {
Object valueToValidate = metaConstraint.getValue( valueContext.getCurrentBean() );
Object valueToValidate = getValue( metaConstraint.getLocation().getMember(), valueContext.getCurrentBean() );
valueContext.setCurrentValidatedValue( valueToValidate );
}
validationSuccessful = metaConstraint.validateConstraint( validationContext, valueContext );
Expand Down Expand Up @@ -552,9 +574,8 @@ private void validateCascadedConstraints(ValidationContext<?> validationContext,
valueContext.getPropertyPath(),
elementType
) ) {
Object value = cascadable.getValue(
valueContext.getCurrentBean()
);

Object value = getValue( valueContext.getCurrentBean(), cascadable );

// Value can be wrapped (e.g. Optional<Address>). Try to unwrap it
ConstraintMetaData metaData = (ConstraintMetaData) cascadable;
Expand Down Expand Up @@ -1302,9 +1323,7 @@ else if ( !propertyIter.hasNext() ) {
else {
if ( property.isCascading() ) {
Type type = property.getType();
newValue = newValue == null ? null : property.getValue(
newValue
);
newValue = newValue == null ? null : getValue( newValue, property );
if ( elem.isIterable() ) {
if ( newValue != null && elem.getIndex() != null ) {
newValue = ReflectionHelper.getIndexedValue( newValue, elem.getIndex() );
Expand Down Expand Up @@ -1483,4 +1502,78 @@ private <T> void setValidatedValueHandlerToValueContextIfPresent(ValidationConte
valueContext.setValidatedValueHandler( handler );
}
}

private Object getValue(Object object, Cascadable cascadable) {
if ( cascadable instanceof PropertyMetaData ) {
Member member = getAccessible( ( (PropertyMetaData) cascadable ).getCascadingMember() );
return getValue( member, object );
}
else if ( cascadable instanceof ParameterMetaData ) {
return ( (Object[]) object )[( (ParameterMetaData) cascadable ).getIndex()];
}
else {
return object;
}
}

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.
* <p>
* <b>NOTE:</b> This must never be changed into a publicly available method to avoid execution of arbitrary
* privileged actions within HV's protection domain.
*/
private <T> T run(PrivilegedAction<T> action) {
return System.getSecurityManager() != null ? AccessController.doPrivileged( action ) : action.run();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,11 +88,6 @@ public ElementType getElementType() {
return ElementType.PARAMETER;
}

@Override
public Object getValue(Object parent) {
return ( (Object[]) parent )[index];
}

@Override
public ParameterDescriptor asDescriptor(boolean defaultGroupSequenceRedefined, List<Class<?>> defaultGroupSequence) {
return new ParameterDescriptorImpl(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import java.util.List;
import java.util.Map;
import java.util.Set;

import javax.validation.ElementKind;
import javax.validation.metadata.GroupConversionDescriptor;

Expand Down Expand Up @@ -94,9 +95,8 @@ private PropertyMetaData(String propertyName,
this.groupConversionHelper.validateGroupConversions( isCascading(), this.toString() );
}

@Override
public Object getValue(Object parent) {
return ReflectionHelper.getValue( cascadingMember, parent );
public Member getCascadingMember() {
return cascadingMember;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,11 +86,6 @@ public ElementType getElementType() {
return ElementType.METHOD;
}

@Override
public Object getValue(Object parent) {
return parent;
}

@Override
public ReturnValueDescriptor asDescriptor(boolean defaultGroupSequenceRedefined, List<Class<?>> defaultGroupSequence) {
return new ReturnValueDescriptorImpl(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
import org.hibernate.validator.internal.engine.constraintvalidation.ConstraintTree;
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
Expand Down Expand Up @@ -91,21 +90,6 @@ public ConstraintLocation getLocation() {
return location;
}

/**
* @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 ) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,15 +54,6 @@ public interface Cascadable {

ElementType getElementType();

/**
* Retrieves the value of this element from the given object.
*
* @param parent The object to retrieve the value from.
*
* @return This element's value.
*/
Object getValue(Object parent);

/**
* Returns the name of this cascadable element.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@

import java.lang.reflect.Constructor;
import java.lang.reflect.Method;
import java.security.AccessController;
import java.security.PrivilegedAction;
import java.util.Collections;
import java.util.List;
import java.util.Map;
Expand All @@ -31,7 +29,6 @@
import org.hibernate.validator.internal.metadata.location.ConstraintLocation;
import org.hibernate.validator.internal.util.logging.Log;
import org.hibernate.validator.internal.util.logging.LoggerFactory;
import org.hibernate.validator.internal.util.privilegedactions.SetAccessibility;

import static org.hibernate.validator.internal.util.CollectionHelper.newArrayList;
import static org.hibernate.validator.internal.util.CollectionHelper.newHashMap;
Expand Down Expand Up @@ -143,10 +140,6 @@ public ConstrainedExecutable(
this.crossParameterConstraints = crossParameterConstraints;
this.parameterMetaData = Collections.unmodifiableList( parameterMetaData );
this.hasParameterConstraints = hasParameterConstraints( parameterMetaData ) || !crossParameterConstraints.isEmpty();

if ( isConstrained() ) {
run( SetAccessibility.action( location.getMember() ) );
}
}

/**
Expand Down Expand Up @@ -353,8 +346,4 @@ else if ( !executable.equals( other.executable ) ) {
}
return true;
}

private <T> T run(PrivilegedAction<T> action) {
return System.getSecurityManager() != null ? AccessController.doPrivileged( action ) : action.run();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,11 @@
*/
package org.hibernate.validator.internal.metadata.raw;

import java.lang.reflect.Member;
import java.security.AccessController;
import java.security.PrivilegedAction;
import java.util.Map;
import java.util.Set;

import org.hibernate.validator.internal.metadata.core.MetaConstraint;
import org.hibernate.validator.internal.metadata.location.ConstraintLocation;
import org.hibernate.validator.internal.util.privilegedactions.SetAccessibility;

/**
* Represents a field of a Java type and all its associated meta-data relevant
Expand Down Expand Up @@ -53,11 +49,6 @@ public ConstrainedField(ConfigurationSource source,
boolean requiresUnwrapping) {

super( source, ConstrainedElementKind.FIELD, location, constraints, groupConversions, isCascading, requiresUnwrapping );

Member member = location.getMember();
if ( member != null && isConstrained() ) {
run( SetAccessibility.action( member ) );
}
}

@Override
Expand Down Expand Up @@ -90,8 +81,4 @@ else if ( !getLocation().getMember().equals( other.getLocation().getMember() ) )
}
return true;
}

private <T> T run(PrivilegedAction<T> action) {
return System.getSecurityManager() != null ? AccessController.doPrivileged( action ) : action.run();
}
}

0 comments on commit c9525ca

Please sign in to comment.