Skip to content

Commit

Permalink
Cleaning up #877 fix for 2.8.3, by deferring access-forcing at a late…
Browse files Browse the repository at this point in the history 
…r point
  • Loading branch information
@cowtowncoder
cowtowncoder committed Sep 8, 2016
1 parent e9ade49 commit 8dbf55f
Show file tree
Hide file tree
Showing 14 changed files with 160 additions and 89 deletions.
95 changes: 71 additions & 24 deletions src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializerBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,26 +20,22 @@ public class BeanDeserializerBuilder
{
/*
/**********************************************************
/* General information about POJO
/* Configuration
/**********************************************************
*/

/**
* Introspected information about POJO for deserializer to handle
*/
final protected BeanDescription _beanDesc;
final protected DeserializationConfig _config;

/**
* Whether default setting for properties without any view annotations
* is to include (true) or exclude (false).
/*
/**********************************************************
/* General information about POJO
/**********************************************************
*/
final protected boolean _defaultViewInclusion;

/**
* Flag that indicates whether default settings suggest use of case-insensitive
* property comparison or not.
* Introspected information about POJO for deserializer to handle
*/
final protected boolean _caseInsensitivePropertyComparison;
final protected BeanDescription _beanDesc;

/*
/**********************************************************
Expand Down Expand Up @@ -114,8 +110,7 @@ public BeanDeserializerBuilder(BeanDescription beanDesc,
DeserializationConfig config)
{
_beanDesc = beanDesc;
_defaultViewInclusion = config.isEnabled(MapperFeature.DEFAULT_VIEW_INCLUSION);
_caseInsensitivePropertyComparison = config.isEnabled(MapperFeature.ACCEPT_CASE_INSENSITIVE_PROPERTIES);
_config = config;
}

/**
Expand All @@ -125,8 +120,7 @@ public BeanDeserializerBuilder(BeanDescription beanDesc,
protected BeanDeserializerBuilder(BeanDeserializerBuilder src)
{
_beanDesc = src._beanDesc;
_defaultViewInclusion = src._defaultViewInclusion;
_caseInsensitivePropertyComparison = src._caseInsensitivePropertyComparison;
_config = src._config;

// let's make copy of properties
_properties.putAll(src._properties);
Expand Down Expand Up @@ -205,6 +199,11 @@ public void addInjectable(PropertyName propName, JavaType propType,
if (_injectables == null) {
_injectables = new ArrayList<ValueInjector>();
}
boolean fixAccess = _config.canOverrideAccessModifiers();
boolean forceAccess = fixAccess && _config.isEnabled(MapperFeature.OVERRIDE_PUBLIC_ACCESS_MODIFIERS);
if (fixAccess) {
member.fixAccess(forceAccess);
}
_injectables.add(new ValueInjector(propName, propType,
contextAnnotations, member, valueId));
}
Expand Down Expand Up @@ -314,7 +313,7 @@ public AnnotatedMethod getBuildMethod() {
public JsonPOJOBuilder.Value getBuilderConfig() {
return _builderConfig;
}

/*
/**********************************************************
/* Build method(s)
Expand All @@ -328,14 +327,16 @@ public JsonPOJOBuilder.Value getBuilderConfig() {
public JsonDeserializer<?> build()
{
Collection<SettableBeanProperty> props = _properties.values();
BeanPropertyMap propertyMap = BeanPropertyMap.construct(props, _caseInsensitivePropertyComparison);
_fixAccess(props);

BeanPropertyMap propertyMap = BeanPropertyMap.construct(props,
_config.isEnabled(MapperFeature.ACCEPT_CASE_INSENSITIVE_PROPERTIES));
propertyMap.assignIndexes();

// view processing must be enabled if:
// (a) fields are not included by default (when deserializing with view), OR
// (b) one of properties has view(s) to included in defined
boolean anyViews = !_defaultViewInclusion;

boolean anyViews = !_config.isEnabled(MapperFeature.DEFAULT_VIEW_INCLUSION);
if (!anyViews) {
for (SettableBeanProperty prop : props) {
if (prop.hasViews()) {
Expand Down Expand Up @@ -382,8 +383,10 @@ public JsonDeserializer<?> buildBuilderBased(JavaType valueType,
if (_buildMethod == null) {
// as per [databind#777], allow empty name
if (!expBuildMethodName.isEmpty()) {
throw new IllegalArgumentException("Builder class "+_beanDesc.getBeanClass().getName()
+" does not have build method (name: '"+expBuildMethodName+"')");
throw new IllegalArgumentException(String.format(
"Builder class %s does not have build method (name: '%s')",
_beanDesc.getBeanClass().getName(),
expBuildMethodName));
}
} else {
// also: type of the method must be compatible
Expand All @@ -399,10 +402,12 @@ public JsonDeserializer<?> buildBuilderBased(JavaType valueType,
}
// And if so, we can try building the deserializer
Collection<SettableBeanProperty> props = _properties.values();
BeanPropertyMap propertyMap = BeanPropertyMap.construct(props, _caseInsensitivePropertyComparison);
_fixAccess(props);
BeanPropertyMap propertyMap = BeanPropertyMap.construct(props,
_config.isEnabled(MapperFeature.ACCEPT_CASE_INSENSITIVE_PROPERTIES));
propertyMap.assignIndexes();

boolean anyViews = !_defaultViewInclusion;
boolean anyViews = !_config.isEnabled(MapperFeature.DEFAULT_VIEW_INCLUSION);

if (!anyViews) {
for (SettableBeanProperty prop : props) {
Expand All @@ -427,4 +432,46 @@ public JsonDeserializer<?> buildBuilderBased(JavaType valueType,
_beanDesc, propertyMap, _backRefProperties, _ignorableProps, _ignoreAllUnknown,
anyViews);
}

/*
/**********************************************************
/* Internal helper method(s)
/**********************************************************
*/

private void _fixAccess(Collection<SettableBeanProperty> mainProps)
{
/* 07-Sep-2016, tatu: Ideally we should be able to avoid forcing
* access to properties that are likely ignored, but due to
* renaming it seems this is not a safe thing to do (there was
* at least one failing test). May need to dig deeper in future;
* for now let's just play it safe.
*/
/*
Set<String> ignorable = _ignorableProps;
if (ignorable == null) {
ignorable = Collections.emptySet();
}
*/
for (SettableBeanProperty prop : mainProps) {
/*
// first: no point forcing access on to-be-ignored properties
if (ignorable.contains(prop.getName())) {
continue;
}
*/
prop.fixAccess(_config);
}
if (_backRefProperties != null) {
for (SettableBeanProperty prop : _backRefProperties.values()) {
prop.fixAccess(_config);
}
}
if (_anySetter != null) {
_anySetter.fixAccess(_config);
}
if (_buildMethod != null) {
_buildMethod.fixAccess(_config.isEnabled(MapperFeature.OVERRIDE_PUBLIC_ACCESS_MODIFIERS));
}
}
}
31 changes: 2 additions & 29 deletions src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializerFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,6 @@ public JsonDeserializer<Object> buildBeanDeserializer(DeserializationContext ctx
addInjectables(ctxt, beanDesc, builder);

final DeserializationConfig config = ctxt.getConfig();
// [JACKSON-440]: update builder now that all information is in?
if (_factoryConfig.hasDeserializerModifiers()) {
for (BeanDeserializerModifier mod : _factoryConfig.deserializerModifiers()) {
builder = mod.updateBuilder(config, beanDesc, builder);
Expand Down Expand Up @@ -640,8 +639,8 @@ protected void addReferenceProperties(DeserializationContext ctxt,
}
SimpleBeanPropertyDefinition propDef = SimpleBeanPropertyDefinition.construct(
ctxt.getConfig(), m);
builder.addBackReferenceProperty(name, constructSettableProperty(
ctxt, beanDesc, propDef, type));
builder.addBackReferenceProperty(name, constructSettableProperty(ctxt,
beanDesc, propDef, type));
}
}
}
Expand All @@ -656,13 +655,8 @@ protected void addInjectables(DeserializationContext ctxt,
{
Map<Object, AnnotatedMember> raw = beanDesc.findInjectables();
if (raw != null) {
boolean fixAccess = ctxt.canOverrideAccessModifiers();
boolean forceAccess = fixAccess && ctxt.isEnabled(MapperFeature.OVERRIDE_PUBLIC_ACCESS_MODIFIERS);
for (Map.Entry<Object, AnnotatedMember> entry : raw.entrySet()) {
AnnotatedMember m = entry.getValue();
if (fixAccess) {
m.fixAccess(forceAccess); // to ensure we can call it
}
builder.addInjectable(PropertyName.construct(m.getName()),
m.getType(),
beanDesc.getClassAnnotations(), m, entry.getKey());
Expand All @@ -683,10 +677,6 @@ protected SettableAnyProperty constructAnySetter(DeserializationContext ctxt,
BeanDescription beanDesc, AnnotatedMember mutator)
throws JsonMappingException
{
if (ctxt.canOverrideAccessModifiers()) {
mutator.fixAccess(ctxt.isEnabled(MapperFeature.OVERRIDE_PUBLIC_ACCESS_MODIFIERS)); // to ensure we can call it
}

//find the java type based on the annotated setter method or setter field
JavaType type = null;
if (mutator instanceof AnnotatedMethod) {
Expand Down Expand Up @@ -729,19 +719,6 @@ protected SettableBeanProperty constructSettableProperty(DeserializationContext
{
// need to ensure method is callable (for non-public)
AnnotatedMember mutator = propDef.getNonConstructorMutator();

if (ctxt.canOverrideAccessModifiers()) {
// [databind#877]: explicitly prevent forced access to `cause` of `Throwable`;
// never needed and attempts may cause problems on some platforms.
// !!! NOTE: should be handled better for 2.8 and later
if ((mutator instanceof AnnotatedField)
&& "cause".equals(mutator.getName())
&& Throwable.class.isAssignableFrom(propType0.getRawClass())) {
;
} else {
mutator.fixAccess(ctxt.isEnabled(MapperFeature.OVERRIDE_PUBLIC_ACCESS_MODIFIERS));
}
}
JavaType type = resolveMemberAndTypeAnnotations(ctxt, mutator, propType0);
// Does the Method specify the deserializer to use? If so, let's use it.
TypeDeserializer typeDeser = type.getTypeHandler();
Expand Down Expand Up @@ -782,10 +759,6 @@ protected SettableBeanProperty constructSetterlessProperty(DeserializationContex
throws JsonMappingException
{
final AnnotatedMethod getter = propDef.getGetter();
// need to ensure it is callable now:
if (ctxt.canOverrideAccessModifiers()) {
getter.fixAccess(ctxt.isEnabled(MapperFeature.OVERRIDE_PUBLIC_ACCESS_MODIFIERS));
}
JavaType type = resolveMemberAndTypeAnnotations(ctxt, getter, getter.getType());
TypeDeserializer typeDeser = type.getTypeHandler();
SettableBeanProperty prop = new SetterlessProperty(propDef, type, typeDeser,
Expand Down
23 changes: 14 additions & 9 deletions src/main/java/com/fasterxml/jackson/databind/deser/CreatorProperty.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import java.lang.annotation.Annotation;

import com.fasterxml.jackson.core.JsonParser;
import com.fasterxml.jackson.core.JsonProcessingException;

import com.fasterxml.jackson.databind.*;
import com.fasterxml.jackson.databind.introspect.AnnotatedMember;
import com.fasterxml.jackson.databind.introspect.AnnotatedParameter;
Expand Down Expand Up @@ -114,6 +114,13 @@ public CreatorProperty withValueDeserializer(JsonDeserializer<?> deser) {
return new CreatorProperty(this, deser);
}

@Override
public void fixAccess(DeserializationConfig config) {
if (_fallbackSetter != null) {
_fallbackSetter.fixAccess(config);
}
}

/**
* NOTE: one exception to immutability, due to problems with CreatorProperty instances
* being shared between Bean, separate PropertyBasedCreator
Expand Down Expand Up @@ -173,19 +180,17 @@ public <A extends Annotation> A getAnnotation(Class<A> acls) {
*/

@Override
public void deserializeAndSet(JsonParser jp, DeserializationContext ctxt,
Object instance)
throws IOException, JsonProcessingException
public void deserializeAndSet(JsonParser p, DeserializationContext ctxt,
Object instance) throws IOException
{
set(instance, deserialize(jp, ctxt));
set(instance, deserialize(p, ctxt));
}

@Override
public Object deserializeSetAndReturn(JsonParser jp,
DeserializationContext ctxt, Object instance)
throws IOException, JsonProcessingException
public Object deserializeSetAndReturn(JsonParser p,
DeserializationContext ctxt, Object instance) throws IOException
{
return setAndReturn(instance, deserialize(jp, ctxt));
return setAndReturn(instance, deserialize(p, ctxt));
}

@Override
Expand Down
15 changes: 10 additions & 5 deletions src/main/java/com/fasterxml/jackson/databind/deser/SettableAnyProperty.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,11 +59,6 @@ public SettableAnyProperty(BeanProperty property, AnnotatedMember setter, JavaTy
_valueTypeDeserializer = typeDeser;
_setterIsField = setter instanceof AnnotatedField;
}

public SettableAnyProperty withValueDeserializer(JsonDeserializer<Object> deser) {
return new SettableAnyProperty(_property, _setter, _type,
deser, _valueTypeDeserializer);
}

/**
* Constructor used for JDK Serialization when reading persisted object
Expand All @@ -78,6 +73,16 @@ protected SettableAnyProperty(SettableAnyProperty src)
_setterIsField = src._setterIsField;
}

public SettableAnyProperty withValueDeserializer(JsonDeserializer<Object> deser) {
return new SettableAnyProperty(_property, _setter, _type,
deser, _valueTypeDeserializer);
}

public void fixAccess(DeserializationConfig config) {
_setter.fixAccess(
config.isEnabled(MapperFeature.OVERRIDE_PUBLIC_ACCESS_MODIFIERS));
}

/*
/**********************************************************
/* JDK serialization handling
Expand Down
19 changes: 14 additions & 5 deletions src/main/java/com/fasterxml/jackson/databind/deser/SettableBeanProperty.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,10 @@
import java.lang.annotation.Annotation;

import com.fasterxml.jackson.core.*;

import com.fasterxml.jackson.databind.*;
import com.fasterxml.jackson.databind.deser.impl.FailingDeserializer;
import com.fasterxml.jackson.databind.introspect.AnnotatedMember;
import com.fasterxml.jackson.databind.introspect.BeanPropertyDefinition;
import com.fasterxml.jackson.databind.introspect.ConcreteBeanPropertyBase;
import com.fasterxml.jackson.databind.introspect.ObjectIdInfo;
import com.fasterxml.jackson.databind.introspect.*;
import com.fasterxml.jackson.databind.jsonFormatVisitors.JsonObjectFormatVisitor;
import com.fasterxml.jackson.databind.jsontype.TypeDeserializer;
import com.fasterxml.jackson.databind.util.Annotations;
Expand Down Expand Up @@ -309,7 +307,18 @@ public void assignIndex(int index) {
}
_propertyIndex = index;
}


/**
* Method called to ensure that the mutator has proper access rights to
* be called, as per configuration. Overridden by implementations that
* have mutators that require access, fields and setters.
*
* @since 2.8.3
*/
public void fixAccess(DeserializationConfig config) {
;
}

/*
/**********************************************************
/* BeanProperty impl
Expand Down
9 changes: 8 additions & 1 deletion src/main/java/com/fasterxml/jackson/databind/deser/impl/FieldProperty.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import com.fasterxml.jackson.databind.introspect.BeanPropertyDefinition;
import com.fasterxml.jackson.databind.jsontype.TypeDeserializer;
import com.fasterxml.jackson.databind.util.Annotations;
import com.fasterxml.jackson.databind.util.ClassUtil;

/**
* This concrete sub-class implements property that is set
Expand Down Expand Up @@ -74,7 +75,13 @@ public FieldProperty withName(PropertyName newName) {
public FieldProperty withValueDeserializer(JsonDeserializer<?> deser) {
return new FieldProperty(this, deser);
}


@Override
public void fixAccess(DeserializationConfig config) {
ClassUtil.checkAndFixAccess(_field,
config.isEnabled(MapperFeature.OVERRIDE_PUBLIC_ACCESS_MODIFIERS));
}

/*
/**********************************************************
/* BeanProperty impl
Expand Down
5 changes: 5 additions & 0 deletions src/main/java/com/fasterxml/jackson/databind/deser/impl/InnerClassProperty.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,11 @@ public InnerClassProperty withValueDeserializer(JsonDeserializer<?> deser) {
return new InnerClassProperty(this, deser);
}

@Override
public void fixAccess(DeserializationConfig config) {
_delegate.fixAccess(config);
}

// // // BeanProperty impl

@Override
Expand Down
Loading

0 comments on commit 8dbf55f

Please sign in to comment.