RFR 8073056: Repeating annotations throws java.security.AccessControlException with a SecurityManager

[Paul Sandoz]

On Feb 26, 2015, at 9:30 AM, Joel Borggrén-Franck <joel.franck at oracle.com> wrote:
>> 182     // According to JLS the container must have an array-valued value
>> 183     // method. Get the AnnotationType, get the "value" method and invoke
>> 184     // it to get the content.
>> 190     Method m = annoType.members().get("value");
>> 212     m.setAccessible(true);
>> 
>> I am missing something here. I read the code and I think
>> annoType is of sun.reflect.annotation.AnnotationType type.
>> Does the old implementation still exist that is supported?
>> Which method is it in the old implementation?  If it's still
>> supported, as Class.getAnnotationsByType is not specified to
>> require any permission check nor throw any SecurityException,
>> and it seems that setAccessible(true) should be wrapped with
>> doPrivileged.
>> 
>> If it's not used in our implementation after your patch,
>> perhaps better to take m.setAccessible(true) out.
> 
> I realize now that this code will probably never have worked on other hypothetical implementations of Annotations. The problem is with the default method AnnotatedElement::getDeclaredAnnotationsByType. If someone has an external implementation of AnnotatedElement (and one of the lessons from adding these methods in 8 was that there are other implementations) then if that external implementation haven’t added getDeclaredAnnotationsByType they will call into our AnnotationSupport. This is all fine if that external representation of AnnotatedElement uses our representation of Annotations, with Proxies and all. But I suspect that the conditions that would end up taking the non-proxy code path in the patch, will already fail in the check:
> 
>             AnnotationType annoType = AnnotationType.getInstance(containerClass);
>             if (annoType == null)
>                 throw invalidContainerException(container, null);
> 
> So what is the best thing to do here if the annotation is not Proxy based and is coming from an implementation of the AnnotatedElement interface that we don’t control and that is missing the methods added in 8? I think throwing here might be the best option, especially since we will probably already have failed in the AnnotationType.getInstance check.
> 

From what you say it seems like throwing a USO might be the best option with an "unsupported annotation container" message. Although i presume being a Proxy is not entirely sufficient to identify that the container comes from an external implementation?

Paul.