How to solve FindBugs DP_DO_INSIDE_DO_PRIVILEGED

Question

When reading and scanning an old codes, I saw these lines of code :

public static void replaceNull(Object obj)
{
    if (obj == null)
    {
        return;
    }

    Field[] fields = obj.getClass().getDeclaredFields();
    if (fields != null)
    {
        for (Field field : fields)
        {
            field.setAccessible(true);
            Class<?> fieldType = field.getType();
            try
            {
                if (field.get(obj) == null)
                {
                    setDefaultValue(obj, field, fieldType);
                }
            } catch (IllegalArgumentException e)
            {
                logger. error("failed replacing null :"+ e.getMessage(),e);
            } catch (IllegalAccessException e)
            {
                logger. error("failed replacing null :"+ e.getMessage(),e);
            }

        }
    }
}

   private static void setDefaultValue(Object obj, Field field, Class<?> fieldType) throws IllegalAccessException
{
    if (fieldType == String.class)
    {
        field.set(obj, CommonConstants.BLANK);

    } else if (fieldType == Date.class)
    {
        field.set(obj, new Date());
    } else if (fieldType == Long.class)
    {
        field.setLong(obj, 0L);
    } else if (fieldType == Integer.class)
    {
        field.setInt(obj, 0);
    } else if (fieldType == BigDecimal.class)
    {
        field.set(obj, new BigDecimal("0.0"));
    }
}

From the flow of the program, it seems that the writer want to create a default values for all of the data member of the object if the value is null.

Upon scanning using FindBugs, the findbugs reported "DP_DO_INSIDE_DO_PRIVILEGED" with this description on setAccessible(true):

Bad practice - Method invoked that should be only be invoked inside a doPrivileged block Plugin: findbugs Key: DP_DO_INSIDE_DO_PRIVILEGED This code invokes a method that requires a security permission check. If this code will be granted security permissions, but might be invoked by code that does not have security permissions, then the invocation needs to occur inside a doPrivileged block.

My question why is this bad? And how should I solve it?

Gastroenteritis

field.setAccessible(true); requires suppressAccessChecks permission. You can "solve" the case via AccessController.doPrivileged(...) wrapping the entire code within a PrivilegedAction. Alternatively ignore this warning from FindBugs. Not all bugs/warnings are valid in any context. Also the entire function might be already wrapped but usually FindBugs won't manage to detect that. – Inelegance

Answer 1

From the Javadocs of field#setAccessible(boolean):

First, if there is a security manager, its checkPermission method is called with a ReflectPermission("suppressAccessChecks") permission.

Without a SecurityManager installed, the program will run fine. However, imagine that your code is written as a shared library, and it happens to be used by some module that has setup a security manager in place. In this case, field.setAccessible(true) may be denied permission even though this and other operations in your code are considered as trusted code. That's why FindBugs raises this warning.

In order to guarantee that field.setAccessible(true) will always be granted permission regardless of the permissions of the caller code, you can wrap the statement inside a AccessController.doPrivileged (you'd have to make field final):

AccessController.doPrivileged(new PrivilegedAction() {
    @Override
    public Object run() {
        field.setAccessible(true);
        return null;
    }
});

Decentralization

Answer 2

Adding to the accepted answer above, Using Java 1.7+ lambda expressions, the same can be achieved with:

AccessController.doPrivileged((PrivilegedAction) () -> {
    field.setAccessible(true);
    return null;
});

Outburst

Answer 3

Since both the SecurityManager and AccessController were deprecated in Java 17 and remain without replacements, disabling this bug detection should be considered safe. Refer to the official documentation for more details.

There is also a SpotBugs issue regarding this: https://github.com/spotbugs/spotbugs/issues/1515

Deprecated, for removal: This API element is subject to removal in a future version. This class is only useful in conjunction with the Security Manager, which is deprecated and subject to removal in a future release. Consequently, this class is also deprecated and subject to removal. There is no replacement for the Security Manager or this class.

Withrow