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

[Peter Levart]

On 03/28/2015 11:24 AM, Joel Borggrén-Franck wrote:
> Hi,
>
>> On 27 Feb 2015, at 11:06, Peter Levart <peter.levart at gmail.com> wrote:
>> On 02/27/2015 10:27 AM, Joel Borggrén-Franck wrote:
>>>> On 26 feb 2015, at 22:35, Peter Levart <peter.levart at gmail.com>
>>>>   wrote:
>>>> On 02/26/2015 10:27 PM, Peter Levart wrote:
>>>>
>>>>> The m.setAccessible(true) for the methods is needed to access methods of non-public annotations, right? This call could be moved to AnnotationType constructor as there it will be performed only once per Method object.
>>>>>
>>>> ...which will have the added benefit in that it will guarantee that only one MethodAccessor object per Method will ever be constructed instead of two…
>>>>
>>>>
>>> I don’t see this. setAccessible sets override in AccessibleObject, I don’t see a new MethodAccessor being generated here.
>> My fault! I was mistakenly thinking of Field objects in the context of setAccessible(boolean). If you use a Field object in two modes it will create and retain two different FieldAccessors (because they are different implementations in case field is final).
>>
>>> But I agree with you, and setting it as accessible in the AnnotationType constructor should arguably be more secure since then we know it isn’t shared since we just got our copy fresh from getDeclaredMethods().
>> That's a good point from maintainability perspective, yes, if someone some time decides to "optimize" the AnnotationType. I think it would be nice if AnnotationType documents that members() method returns Method objects that are pre-conditioned with setAccessible(true) and that users should not change this flag.
>>
> I don’t want to do this in AnnotationType without a bigger overhaul that may be slightly incompatible and therefor should be 9 only. I do want to backport this fix to 8 however, so here is an alternative solution that should be safe and correct at the cost of extra allocation in the path for custom implementations of annotations (that should be rare).
>
> New webrev:
>
> http://cr.openjdk.java.net/~jfranck/8073056/webrev.02/
>
> cheers
> /Joel

Hi Joel,

If you really must add new methods to Method, ReflectAccess, 
LangReflectAccess and ReflectionFactory then you could expose a method 
called "invokeUnchecked" that would skip access checks, so you don't 
have to copy the Method object just to get a one-time access to the 
possibly inaccessible method. But this is good as it is, since the code 
path is normally not used - it's there only for consistency's sake, right?


Regards, Peter