If I had known when I started how hard it would be, I would never have started writing one...
Where are annotations tests supposed to go? They seem under-test-covered. I've put it into a new directory, test/java/lang/annotation Please review. The following fails with current openjdk, passes with proposed fix. /* * Copyright 2008 Sun Microsystems, Inc. All Rights Reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it * under the terms of the GNU General Public License version 2 only, as * published by the Free Software Foundation. * * This code is distributed in the hope that it will be useful, but WITHOUT * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License * version 2 for more details (a copy is included in the LICENSE file that * accompanied this code). * * You should have received a copy of the GNU General Public License version * 2 along with this work; if not, write to the Free Software Foundation, * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. * * Please contact Sun Microsystems, Inc., 4150 Network Circle, Santa Clara, * CA 95054 USA or visit www.sun.com if you need additional information or * have any questions. */ /* * @test * @bug 6761678 * @summary Check properties of Annotations returned from * getParameterAnnotations, including freedom from security * exceptions. * @author Martin Buchholz */ import java.lang.annotation.Annotation; import java.lang.annotation.ElementType; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; import java.lang.reflect.Method; import java.security.Permission; import java.security.Policy; import java.security.ProtectionDomain; @Retention(RetentionPolicy.RUNTIME) @Target({ ElementType.FIELD, ElementType.PARAMETER }) @interface Named { String value(); } public class ParameterAnnotations { // A security policy that differs from the default only in that it // allows a security manager to be uninstalled. static class MyPolicy extends Policy { final Policy defaultPolicy; MyPolicy(Policy defaultPolicy) { this.defaultPolicy = defaultPolicy; } public boolean implies(ProtectionDomain pd, Permission p) { return p.getName().equals("setSecurityManager") || defaultPolicy.implies(pd, p); } } public void nop(@Named("foo") Object foo, @Named("bar") Object bar) { } void test(String[] args) throws Throwable { // Test without a security manager test1(); // Test with a security manager Policy defaultPolicy = Policy.getPolicy(); Policy.setPolicy(new MyPolicy(defaultPolicy)); System.setSecurityManager(new SecurityManager()); try { test1(); } finally { System.setSecurityManager(null); Policy.setPolicy(defaultPolicy); } } void test1() throws Throwable { for (Method m : thisClass.getMethods()) { if (m.getName().equals("nop")) { Annotation[][] ann = m.getParameterAnnotations(); equal(ann.length, 2); Annotation foo = ann[0][0]; Annotation bar = ann[1][0]; equal(foo.toString(), "@Named(value=foo)"); equal(bar.toString(), "@Named(value=bar)"); check(foo.equals(foo)); check(! foo.equals(bar)); } } } //--------------------- Infrastructure --------------------------- volatile int passed = 0, failed = 0; void pass() {passed++;} void fail() {failed++; Thread.dumpStack();} void fail(String msg) {System.err.println(msg); fail();} void unexpected(Throwable t) {failed++; t.printStackTrace();} void check(boolean cond) {if (cond) pass(); else fail();} void equal(Object x, Object y) { if (x == null ? y == null : x.equals(y)) pass(); else fail(x + " not equal to " + y);} static Class<?> thisClass = new Object(){}.getClass().getEnclosingClass(); public static void main(String[] args) throws Throwable { try {thisClass.getMethod("instanceMain",String[].class) .invoke(thisClass.newInstance(), (Object) args);} catch (Throwable e) {throw e.getCause();}} public void instanceMain(String[] args) throws Throwable { try {test(args);} catch (Throwable t) {unexpected(t);} System.out.printf("%nPassed = %d, failed = %d%n%n", passed, failed); if (failed > 0) throw new AssertionError("Some tests failed");} } Martin On Mon, Oct 20, 2008 at 19:01, Joe Darcy <[EMAIL PROTECTED]> wrote: > PS Martin, I haven't written tests that setup and use a security manager, > etc., that should go along with this fix. Having a stand-alone regression > test along those lines would be helpful. > > Thanks, > > -Joe > > On 10/20/08 06:59 PM, Joe Darcy wrote: >> >> Hello. >> >> On 10/16/08 12:46 PM, Martin Buchholz wrote: >>> >>> Hi all, >>> >>> This is a bug report with fix. >>> Joe Darcy, please file a bug and review this change, >>> >> >> I've filed 6761678 "(ann) SecurityException in >> AnnotationInvocationHandler.getMemberMethods" for this issue. The problem >> seems similar to 6370080 "(ann) Method.getAnnotations() sometimes throw >> SecurityException: doPrivileged or javadoc missing?," which was fixed in JDK >> 6 and a JDK 5 update release. >> >> Martin, can you, Toby, and Josh review any other uses of reflection in the >> src/share/classes/sun/reflect/annotation package for similar problems so we >> can address any other such issues now? >> >> I've looked over the change and the use of getMemberMethods in equalsImpl >> and don't see a problem. However, I'd like the security team to give it a >> once over too; security-dev folk, please take a look at this. >> >> Thanks, >> >> -Joe >>> >>> and perhaps provide a small test case (it is impractical >>> to share the test we have at Google). >>> >>> Description: >>> >>> sun/reflect/annotation/AnnotationInvocationHandler.java.getMemberMethods >>> might throw if there is a security manager that does not allow >>> getDeclaredMethods. >>> >>> The author of this code (Josh Bloch) confirms that the intent was for the >>> doPrivileged block in this method to prevent security exceptions. >>> The methods cannot escape to untrusted code. >>> >>> Evaluation: >>> >>> Yes. Fix provided courtesy of Toby Reyelts and Josh Bloch at Google. >>> >>> # HG changeset patch >>> # User martin >>> # Date 1224185752 25200 >>> # Node ID 68730f05449cd4f39ce1cb82adc6c4e57f87554f >>> # Parent 214ebdcf7252d4862449fe0ae295e6c60a127315 >>> SecurityException in AnnotationInvocationHandler.getMemberMethods >>> Summary: Move call to getDeclaredMethods inside doPrivileged >>> Reviewed-by: >>> Contributed-by: [EMAIL PROTECTED] >>> >>> diff --git >>> a/src/share/classes/sun/reflect/annotation/AnnotationInvocationHandler.java >>> >>> b/src/share/classes/sun/reflect/annotation/AnnotationInvocationHandler.java >>> --- >>> a/src/share/classes/sun/reflect/annotation/AnnotationInvocationHandler.java >>> +++ >>> b/src/share/classes/sun/reflect/annotation/AnnotationInvocationHandler.java >>> @@ -272,14 +272,14 @@ >>> */ >>> private Method[] getMemberMethods() { >>> if (memberMethods == null) { >>> - final Method[] mm = type.getDeclaredMethods(); >>> - AccessController.doPrivileged(new PrivilegedAction<Void>() { >>> - public Void run() { >>> - AccessibleObject.setAccessible(mm, true); >>> - return null; >>> - } >>> - }); >>> - memberMethods = mm; >>> + memberMethods = AccessController.doPrivileged( >>> + new PrivilegedAction<Method[]>() { >>> + public Method[] run() { >>> + final Method[] mm = type.getDeclaredMethods(); >>> + AccessibleObject.setAccessible(mm, true); >>> + return mm; >>> + } >>> + }); >>> } >>> return memberMethods; >>> } >>> >> > >