Martin Buchholz wrote:
If I had known when I started how hard it would be,
I would never have started writing one...
That is the case with many endeavors ;-)
Where are annotations tests supposed to go?
They seem under-test-covered.
I've put it into a new directory, test/java/lang/annotation
Recently I moved over the existing closed regression tests for
annotations to test/java/lang/annotation:
http://mail.openjdk.java.net/pipermail/jdk6-dev/2008-October/000231.html
so I'll find a home for your tests in there. The newly-opened tests
will be in the b13 source drop.
Thanks,
-Joe
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;
}
