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

[Mandy Chung]

On 2/26/15 1:27 PM, Peter Levart wrote:
>
> On 02/26/2015 05:34 PM, Mandy Chung wrote:
>>> 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 did a quick search on my corpus and find only one reference to
>> sun.reflect.annotation.AnnotationType.  Looks like external 
>> implementation
>> doesn't get pass this. 
>
> Now I'm missing something. Why would a custom non-Proxy based 
> annotation not pass the above code?

I had assumed that AnnotationType.getInstance also expects the 
implementation of the annotation type is 
sun.reflect.annotation.AnnotationType.  I don't know enough about this 
area and that's just my observation. Hope Joel will say more details.
> I don't see anything in AnnotationType implementation that would be 
> dependent on annotation implementation to be a Proxy. AnnotationType 
> only deals with the annotation type, which is an interface, not with 
> implementation.
>
> The m.setAccessible(true) for the methods is needed to access methods 
> of non-public annotations, right? 

I think so.
> This call could be moved to AnnotationType constructor as there it 
> will be performed only once per Method object.
>
If the specification supports other implementation, it seems to me that 
calling setAccessible(true) should be wrapped with doPrivileged unless 
the specification states the "suppressAccessCheck" permission is 
required; otherwise, SecurityException will be thrown.  It'd be good to 
clarify what that code is intended for.

Mandy