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
