Fix #997

FasterXML · Nov 5, 2015 · 52ae85f · 52ae85f
1 parent ff192c1
commit 52ae85f
Show file tree

Hide file tree

Showing 16 changed files with 119 additions and 49 deletions.
diff --git a/release-notes/VERSION b/release-notes/VERSION
@@ -19,6 +19,7 @@ Project: jackson-databind
  (contributed by Jesse W)
 #957: Merge `datatype-jdk7` stuff in (java.nio.file.Path handling)
 #959: Schema generation: consider active view, discard non-included properties
+#997: Add `MapperFeature.OVERRIDE_PUBLIC_ACCESS_MODIFIERS`
 - Make `JsonValueFormat` (self-)serializable, deserializable, to/from valid external
   value (as per JSON Schema spec)
 

diff --git a/src/main/java/com/fasterxml/jackson/databind/MapperFeature.java b/src/main/java/com/fasterxml/jackson/databind/MapperFeature.java
@@ -148,15 +148,16 @@ public enum MapperFeature implements ConfigFeature
      * modifier settings can be overridden when accessing
      * properties. If enabled, method
      * {@link java.lang.reflect.AccessibleObject#setAccessible}
-     * may be called to enable access to otherwise unaccessible
-     * objects.
+     * may be called to enable access to otherwise unaccessible objects.
      *<p>
-     * Note that this setting usually has significant performance implications,
+     * Note that this setting may have significant performance implications,
      * since access override helps remove costly access checks on each
      * and every Reflection access. If you are considering disabling
      * this feature, be sure to verify performance consequences if usage
      * is performance sensitive.
-     * Especially on standard JavaSE platforms difference is significant.
+     * Also note that performance effects vary between Java platforms
+     * (JavaSE vs Android, for example), as well as JDK versions: older
+     * versions seemed to have more significant performance difference.
      *<p>
      * Conversely, on some platforms, it may be necessary to disable this feature
      * as platform does not allow such calls. For example, when developing
@@ -167,6 +168,24 @@ public enum MapperFeature implements ConfigFeature
      */
     CAN_OVERRIDE_ACCESS_MODIFIERS(true),
 
+    /**
+     * Feature that determines that forces call to
+     * {@link java.lang.reflect.AccessibleObject#setAccessible} even for
+     * <code>public</code> accessors -- that is, even if no such call is
+     * needed from functionality perspective -- if call is allowed
+     * (that is, {@link #CAN_OVERRIDE_ACCESS_MODIFIERS} is set to true).
+     * The main reason to enable this feature is possible performance
+     * improvement as JDK does not have to perform access checks; these
+     * checks are otherwise made for all accessors, including public ones,
+     * and may result in slower Reflection calls. Exact impact (if any)
+     * depends on Java platform (Java SE, Android) as well as JDK version.
+     *<p>
+     * Feature is enabled by default, for legacy reasons.
+     *
+     * @since 2.7
+     */
+    OVERRIDE_PUBLIC_ACCESS_MODIFIERS(true),
+
     /**
      * Feature that determines whether member mutators (fields and
      * setters) may be "pulled in" even if they are not visible,

diff --git a/src/main/java/com/fasterxml/jackson/databind/deser/BasicDeserializerFactory.java b/src/main/java/com/fasterxml/jackson/databind/deser/BasicDeserializerFactory.java
@@ -293,8 +293,7 @@ protected ValueInstantiator _constructDefaultValueInstantiator(DeserializationCo
             BeanDescription beanDesc)
         throws JsonMappingException
     {
-        boolean fixAccess = ctxt.canOverrideAccessModifiers();
-        CreatorCollector creators =  new CreatorCollector(beanDesc, fixAccess);
+        CreatorCollector creators =  new CreatorCollector(beanDesc, ctxt.getConfig());
         AnnotationIntrospector intr = ctxt.getAnnotationIntrospector();
 
         // need to construct suitable visibility checker:
@@ -1439,9 +1438,10 @@ private KeyDeserializer _createEnumKeyDeserializer(DeserializationContext ctxt,
         }
 
         EnumResolver enumRes = constructEnumResolver(enumClass, config, beanDesc.findJsonValueMethod());
-        // [JACKSON-193] May have @JsonCreator for static factory method:
+        // May have @JsonCreator for static factory method:
+        final AnnotationIntrospector ai = config.getAnnotationIntrospector();
         for (AnnotatedMethod factory : beanDesc.getFactoryMethods()) {
-            if (config.getAnnotationIntrospector().hasCreatorAnnotation(factory)) {
+            if (ai.hasCreatorAnnotation(factory)) {
                 int argCount = factory.getParameterCount();
                 if (argCount == 1) {
                     Class<?> returnType = factory.getRawReturnType();
@@ -1452,7 +1452,8 @@ private KeyDeserializer _createEnumKeyDeserializer(DeserializationContext ctxt,
                             throw new IllegalArgumentException("Parameter #0 type for factory method ("+factory+") not suitable, must be java.lang.String");
                         }
                         if (config.canOverrideAccessModifiers()) {
-                            ClassUtil.checkAndFixAccess(factory.getMember());
+                            ClassUtil.checkAndFixAccess(factory.getMember(),
+                                    ctxt.isEnabled(MapperFeature.OVERRIDE_PUBLIC_ACCESS_MODIFIERS));
                         }
                         return StdKeyDeserializers.constructEnumKeyDeserializer(enumRes, factory);
                     }
@@ -1914,7 +1915,7 @@ protected EnumResolver constructEnumResolver(Class<?> enumClass,
         if (jsonValueMethod != null) {
             Method accessor = jsonValueMethod.getAnnotated();
             if (config.canOverrideAccessModifiers()) {
-                ClassUtil.checkAndFixAccess(accessor);
+                ClassUtil.checkAndFixAccess(accessor, config.isEnabled(MapperFeature.OVERRIDE_PUBLIC_ACCESS_MODIFIERS));
             }
             return EnumResolver.constructUnsafeUsingMethod(enumClass, accessor);
         }

diff --git a/src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializerBase.java b/src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializerBase.java
@@ -760,8 +760,8 @@ protected SettableBeanProperty _resolveInnerClassValuedProperty(DeserializationC
                     for (Constructor<?> ctor : valueClass.getConstructors()) {
                         Class<?>[] paramTypes = ctor.getParameterTypes();
                         if (paramTypes.length == 1 && paramTypes[0] == enclosing) {
-                            if (ctxt.getConfig().canOverrideAccessModifiers()) {
-                                ClassUtil.checkAndFixAccess(ctor);
+                            if (ctxt.canOverrideAccessModifiers()) {
+                                ClassUtil.checkAndFixAccess(ctor, ctxt.isEnabled(MapperFeature.OVERRIDE_PUBLIC_ACCESS_MODIFIERS));
                             }
                             return new InnerClassProperty(prop, ctor);
                         }

diff --git a/src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializerFactory.java b/src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializerFactory.java
@@ -292,7 +292,7 @@ protected JsonDeserializer<Object> buildBuilderBasedDeserializer(
         AnnotatedMethod buildMethod = builderDesc.findMethod(buildMethodName, null);
         if (buildMethod != null) { // note: can't yet throw error; may be given build method
             if (config.canOverrideAccessModifiers()) {
-            	ClassUtil.checkAndFixAccess(buildMethod.getMember());
+            	ClassUtil.checkAndFixAccess(buildMethod.getMember(), config.isEnabled(MapperFeature.OVERRIDE_PUBLIC_ACCESS_MODIFIERS));
             }
         }
         builder.setPOJOBuilder(buildMethod, builderConfig);
@@ -645,10 +645,11 @@ 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(); // to ensure we can call it
+                    m.fixAccess(forceAccess); // to ensure we can call it
                 }
                 builder.addInjectable(PropertyName.construct(m.getName()),
                         m.getType(),
@@ -667,7 +668,7 @@ protected SettableAnyProperty constructAnySetter(DeserializationContext ctxt,
         throws JsonMappingException
     {
         if (ctxt.canOverrideAccessModifiers()) {
-            setter.fixAccess(); // to ensure we can call it
+            setter.fixAccess(ctxt.isEnabled(MapperFeature.OVERRIDE_PUBLIC_ACCESS_MODIFIERS)); // to ensure we can call it
         }
         // we know it's a 2-arg method, second arg is the value
         JavaType type = setter.getParameterType(1);
@@ -709,7 +710,7 @@ protected SettableBeanProperty constructSettableProperty(DeserializationContext
         // need to ensure method is callable (for non-public)
         AnnotatedMember mutator = propDef.getNonConstructorMutator();
         if (ctxt.canOverrideAccessModifiers()) {
-            mutator.fixAccess();
+            mutator.fixAccess(ctxt.isEnabled(MapperFeature.OVERRIDE_PUBLIC_ACCESS_MODIFIERS));
         }
         // note: this works since we know there's exactly one argument for methods
         BeanProperty.Std property = new BeanProperty.Std(propDef.getFullName(),
@@ -761,7 +762,7 @@ protected SettableBeanProperty constructSetterlessProperty(DeserializationContex
         final AnnotatedMethod getter = propDef.getGetter();
         // need to ensure it is callable now:
         if (ctxt.canOverrideAccessModifiers()) {
-            getter.fixAccess();
+            getter.fixAccess(ctxt.isEnabled(MapperFeature.OVERRIDE_PUBLIC_ACCESS_MODIFIERS));
         }
         JavaType type = getter.getType();
         // First: does the Method specify the deserializer to use? If so, let's use it.

diff --git a/src/main/java/com/fasterxml/jackson/databind/deser/impl/CreatorCollector.java b/src/main/java/com/fasterxml/jackson/databind/deser/impl/CreatorCollector.java
@@ -5,6 +5,7 @@
 import java.util.*;
 
 import com.fasterxml.jackson.databind.*;
+import com.fasterxml.jackson.databind.cfg.MapperConfig;
 import com.fasterxml.jackson.databind.deser.CreatorProperty;
 import com.fasterxml.jackson.databind.deser.SettableBeanProperty;
 import com.fasterxml.jackson.databind.deser.ValueInstantiator;
@@ -39,6 +40,11 @@ public class CreatorCollector
 
     final protected boolean _canFixAccess;
 
+    /**
+     * @since 2.7
+     */
+    final protected boolean _forceAccess;
+
     /**
      * Set of creators we have collected so far
      * 
@@ -69,10 +75,11 @@ public class CreatorCollector
     /**********************************************************
      */
 
-    public CreatorCollector(BeanDescription beanDesc, boolean canFixAccess)
+    public CreatorCollector(BeanDescription beanDesc, MapperConfig<?> config)
     {
         _beanDesc = beanDesc;
-        _canFixAccess = canFixAccess;
+        _canFixAccess = config.canOverrideAccessModifiers();
+        _forceAccess = config.isEnabled(MapperFeature.OVERRIDE_PUBLIC_ACCESS_MODIFIERS);
     }
 
     public ValueInstantiator constructValueInstantiator(DeserializationConfig config)
@@ -272,7 +279,7 @@ public boolean hasPropertyBasedCreator() {
     private <T extends AnnotatedMember> T _fixAccess(T member)
     {
         if (member != null && _canFixAccess) {
-            ClassUtil.checkAndFixAccess((Member) member.getAnnotated());
+            ClassUtil.checkAndFixAccess((Member) member.getAnnotated(), _forceAccess);
         }
         return member;
     }

diff --git a/src/main/java/com/fasterxml/jackson/databind/deser/std/EnumDeserializer.java b/src/main/java/com/fasterxml/jackson/databind/deser/std/EnumDeserializer.java
@@ -53,7 +53,8 @@ public static JsonDeserializer<?> deserializerForCreator(DeserializationConfig c
         // note: caller has verified there's just one arg; but we must verify its type
         Class<?> paramClass = factory.getRawParameterType(0);
         if (config.canOverrideAccessModifiers()) {
-            ClassUtil.checkAndFixAccess(factory.getMember());
+            ClassUtil.checkAndFixAccess(factory.getMember(),
+                    config.isEnabled(MapperFeature.OVERRIDE_PUBLIC_ACCESS_MODIFIERS));
         }
         return new FactoryBasedDeserializer(enumClass, factory, paramClass);
     }

diff --git a/src/main/java/com/fasterxml/jackson/databind/deser/std/StdKeyDeserializers.java b/src/main/java/com/fasterxml/jackson/databind/deser/std/StdKeyDeserializers.java
@@ -56,7 +56,7 @@ public static KeyDeserializer findStringBasedKeyDeserializer(DeserializationConf
         Constructor<?> ctor = beanDesc.findSingleArgConstructor(String.class);
         if (ctor != null) {
             if (config.canOverrideAccessModifiers()) {
-                ClassUtil.checkAndFixAccess(ctor);
+                ClassUtil.checkAndFixAccess(ctor, config.isEnabled(MapperFeature.OVERRIDE_PUBLIC_ACCESS_MODIFIERS));
             }
             return new StdKeyDeserializer.StringCtorKeyDeserializer(ctor);
         }
@@ -66,7 +66,7 @@ public static KeyDeserializer findStringBasedKeyDeserializer(DeserializationConf
         Method m = beanDesc.findFactoryMethod(String.class);
         if (m != null){
             if (config.canOverrideAccessModifiers()) {
-                ClassUtil.checkAndFixAccess(m);
+                ClassUtil.checkAndFixAccess(m, config.isEnabled(MapperFeature.OVERRIDE_PUBLIC_ACCESS_MODIFIERS));
             }
             return new StdKeyDeserializer.StringFactoryKeyDeserializer(m);
         }

diff --git a/src/main/java/com/fasterxml/jackson/databind/introspect/AnnotatedConstructor.java b/src/main/java/com/fasterxml/jackson/databind/introspect/AnnotatedConstructor.java
@@ -186,7 +186,7 @@ Object readResolve() {
             Constructor<?> ctor = clazz.getDeclaredConstructor(_serialization.args);
             // 06-Oct-2012, tatu: Has "lost" its security override, must force back
             if (!ctor.isAccessible()) {
-                ClassUtil.checkAndFixAccess(ctor);
+                ClassUtil.checkAndFixAccess(ctor, false);
             }
             return new AnnotatedConstructor(null, ctor, null, null);
         } catch (Exception e) {

diff --git a/src/main/java/com/fasterxml/jackson/databind/introspect/AnnotatedField.java b/src/main/java/com/fasterxml/jackson/databind/introspect/AnnotatedField.java
@@ -164,7 +164,7 @@ Object readResolve() {
             Field f = clazz.getDeclaredField(_serialization.name);
             // 06-Oct-2012, tatu: Has "lost" its security override, may need to force back
             if (!f.isAccessible()) {
-                ClassUtil.checkAndFixAccess(f);
+                ClassUtil.checkAndFixAccess(f, false);
             }
             return new AnnotatedField(null, f, null);
         } catch (Exception e) {

diff --git a/src/main/java/com/fasterxml/jackson/databind/introspect/AnnotatedMember.java b/src/main/java/com/fasterxml/jackson/databind/introspect/AnnotatedMember.java
@@ -21,10 +21,10 @@ public abstract class AnnotatedMember
     // 19-Dec-2014, tatu: Similarly, assumed NOT to be needed in cases where
     //    owning object (ObjectMapper or relatives) is being JDK-serialized
     /**
-     * Class that was resolved to produce this member instance; either class that declared
-     * the member, or one of its subtypes that inherited it.
-     * 
-     * @since 2.5
+     * Context object needed for resolving generic type associated with this
+     * member (method parameter or return value, or field type).
+     *
+     * @since 2.7
      */
     protected final transient TypeResolutionContext _typeContext;
 
@@ -111,14 +111,30 @@ public final boolean addOrOverride(Annotation a) {
     public final boolean addIfNotPresent(Annotation a) {
         return _annotations.addIfNotPresent(a);
     }
-    
+
     /**
      * Method that can be called to modify access rights, by calling
      * {@link java.lang.reflect.AccessibleObject#setAccessible} on
      * the underlying annotated element.
+     *<p>
+     * Note that caller should verify that
+     * {@link com.fasterxml.jackson.databind.MapperFeature#CAN_OVERRIDE_ACCESS_MODIFIERS}
+     * is enabled before calling this method; as well as pass
+     * <code>force</code> flag appropriately.
+     * 
+     * @since 2.7
+     */
+    public final void fixAccess(boolean force) {
+        ClassUtil.checkAndFixAccess(getMember(), force);
+    }
+
+    /**
+     * @deprecated Since 2.7 use {@link #fixAccess(boolean)} instead
      */
+    @Deprecated
     public final void fixAccess() {
-        ClassUtil.checkAndFixAccess(getMember());
+//        fixAccess(false);
+        fixAccess(true);
     }
 
     /**

diff --git a/src/main/java/com/fasterxml/jackson/databind/introspect/AnnotatedMethod.java b/src/main/java/com/fasterxml/jackson/databind/introspect/AnnotatedMethod.java
@@ -255,7 +255,7 @@ Object readResolve() {
                     _serialization.args);
             // 06-Oct-2012, tatu: Has "lost" its security override, may need to force back
             if (!m.isAccessible()) {
-                ClassUtil.checkAndFixAccess(m);
+                ClassUtil.checkAndFixAccess(m, false);
             }
             return new AnnotatedMethod(null, m, null, null);
         } catch (Exception e) {

diff --git a/src/main/java/com/fasterxml/jackson/databind/introspect/BasicBeanDescription.java b/src/main/java/com/fasterxml/jackson/databind/introspect/BasicBeanDescription.java
@@ -305,7 +305,7 @@ public Object instantiateBean(boolean fixAccess)
             return null;
         }
         if (fixAccess) {
-            ac.fixAccess();
+            ac.fixAccess(_config.isEnabled(MapperFeature.OVERRIDE_PUBLIC_ACCESS_MODIFIERS));
         }
         try {
             return ac.getAnnotated().newInstance();
@@ -319,7 +319,7 @@ public Object instantiateBean(boolean fixAccess)
             throw new IllegalArgumentException("Failed to instantiate bean of type "+_classInfo.getAnnotated().getName()+": ("+t.getClass().getName()+") "+t.getMessage(), t);
         }
     }
-    
+
     /*
     /**********************************************************
     /* Simple accessors, extended

diff --git a/src/main/java/com/fasterxml/jackson/databind/ser/BasicSerializerFactory.java b/src/main/java/com/fasterxml/jackson/databind/ser/BasicSerializerFactory.java
@@ -239,7 +239,7 @@ public JsonSerializer<Object> createKeySerializer(SerializationConfig config,
                                 rawType, true);
                         Method m = am.getAnnotated();
                         if (config.canOverrideAccessModifiers()) {
-                            ClassUtil.checkAndFixAccess(m);
+                            ClassUtil.checkAndFixAccess(m, config.isEnabled(MapperFeature.OVERRIDE_PUBLIC_ACCESS_MODIFIERS));
                         }
                         ser = new JsonValueSerializer(m, delegate);
                     } else {
@@ -357,7 +357,7 @@ protected final JsonSerializer<?> findSerializerByAnnotations(SerializerProvider
         if (valueMethod != null) {
             Method m = valueMethod.getAnnotated();
             if (prov.canOverrideAccessModifiers()) {
-                ClassUtil.checkAndFixAccess(m);
+                ClassUtil.checkAndFixAccess(m, prov.isEnabled(MapperFeature.OVERRIDE_PUBLIC_ACCESS_MODIFIERS));
             }
             JsonSerializer<Object> ser = findSerializerFromAnnotation(prov, valueMethod);
             return new JsonValueSerializer(m, ser);

diff --git a/src/main/java/com/fasterxml/jackson/databind/ser/BeanSerializerFactory.java b/src/main/java/com/fasterxml/jackson/databind/ser/BeanSerializerFactory.java
@@ -420,11 +420,11 @@ protected JsonSerializer<Object> constructBeanSerializer(SerializerProvider prov
 
         builder.setProperties(props);
         builder.setFilterId(findFilterId(config, beanDesc));
-        
+
         AnnotatedMember anyGetter = beanDesc.findAnyGetter();
         if (anyGetter != null) {
             if (config.canOverrideAccessModifiers()) {
-                anyGetter.fixAccess();
+                anyGetter.fixAccess(config.isEnabled(MapperFeature.OVERRIDE_PUBLIC_ACCESS_MODIFIERS));
             }
             JavaType type = anyGetter.getType();
             // copied from BasicSerializerFactory.buildMapSerializer():
@@ -585,19 +585,21 @@ protected List<BeanPropertyWriter> findBeanProperties(SerializerProvider prov,
         PropertyBuilder pb = constructPropertyBuilder(config, beanDesc);
 
         ArrayList<BeanPropertyWriter> result = new ArrayList<BeanPropertyWriter>(properties.size());
+        final boolean fixAccess = config.canOverrideAccessModifiers();
+        final boolean forceAccess = fixAccess && config.isEnabled(MapperFeature.OVERRIDE_PUBLIC_ACCESS_MODIFIERS);
         for (BeanPropertyDefinition property : properties) {
             final AnnotatedMember accessor = property.getAccessor();
             // Type id? Requires special handling:
             if (property.isTypeId()) {
                 if (accessor != null) { // only add if we can access... but otherwise?
-                    if (config.canOverrideAccessModifiers()) {
-                        accessor.fixAccess();
+                    if (fixAccess) {
+                        accessor.fixAccess(forceAccess);
                     }
                     builder.setTypeId(accessor);
                 }
                 continue;
             }
-            // [JACKSON-235]: suppress writing of back references
+            // suppress writing of back references
             AnnotationIntrospector.ReferenceProperty refType = property.findReferenceType();
             if (refType != null && refType.isBackReference()) {
                 continue;
@@ -776,7 +778,7 @@ protected BeanPropertyWriter _constructWriter(SerializerProvider prov,
     {
         final PropertyName name = propDef.getFullName();
         if (prov.canOverrideAccessModifiers()) {
-            accessor.fixAccess();
+            accessor.fixAccess(prov.isEnabled(MapperFeature.OVERRIDE_PUBLIC_ACCESS_MODIFIERS));
         }
         JavaType type = accessor.getType();
         BeanProperty.Std property = new BeanProperty.Std(name, type, propDef.getWrapperName(),