From 04d52fb829cf4d31f1f54d3ffad25d0970c6fb49 Mon Sep 17 00:00:00 2001 From: mchung Date: Wed, 18 Nov 2009 22:29:16 -0800 Subject: [PATCH] 6902678: com.sun.tracing.ProviderFactory.createProvider doesn't throw IllegalArgumentException Summary: doPrivileged for calls that have permission check instead of catching all exceptions Reviewed-by: kamg, dcubed --- .../com/sun/tracing/ProviderFactory.java | 46 +++--- .../sun/tracing/MultiplexProviderFactory.java | 9 +- .../sun/tracing/NullProviderFactory.java | 9 +- .../tracing/PrintStreamProviderFactory.java | 9 +- .../classes/sun/tracing/ProviderSkeleton.java | 10 +- .../tracing/dtrace/DTraceProviderFactory.java | 12 +- .../com/sun/tracing/BasicWithSecurityMgr.java | 149 ++++++++++++++++++ 7 files changed, 188 insertions(+), 56 deletions(-) create mode 100644 test/com/sun/tracing/BasicWithSecurityMgr.java diff --git a/src/share/classes/com/sun/tracing/ProviderFactory.java b/src/share/classes/com/sun/tracing/ProviderFactory.java index 768a6bebb..1a4f064ff 100644 --- a/src/share/classes/com/sun/tracing/ProviderFactory.java +++ b/src/share/classes/com/sun/tracing/ProviderFactory.java @@ -4,7 +4,10 @@ package com.sun.tracing; import java.util.HashSet; import java.io.PrintStream; import java.lang.reflect.Field; -import java.util.logging.Logger; +import java.security.AccessController; +import java.security.PrivilegedActionException; +import java.security.PrivilegedExceptionAction; +import sun.security.action.GetPropertyAction; import sun.tracing.NullProviderFactory; import sun.tracing.PrintStreamProviderFactory; @@ -52,23 +55,17 @@ public abstract class ProviderFactory { HashSet factories = new HashSet(); // Try to instantiate a DTraceProviderFactory - String prop = null; - try { prop = System.getProperty("com.sun.tracing.dtrace"); } - catch (java.security.AccessControlException e) { - Logger.getAnonymousLogger().fine( - "Cannot access property com.sun.tracing.dtrace"); - } + String prop = AccessController.doPrivileged( + new GetPropertyAction("com.sun.tracing.dtrace")); + if ( (prop == null || !prop.equals("disable")) && DTraceProviderFactory.isSupported() ) { factories.add(new DTraceProviderFactory()); } // Try to instantiate an output stream factory - try { prop = System.getProperty("sun.tracing.stream"); } - catch (java.security.AccessControlException e) { - Logger.getAnonymousLogger().fine( - "Cannot access property sun.tracing.stream"); - } + prop = AccessController.doPrivileged( + new GetPropertyAction("sun.tracing.stream")); if (prop != null) { for (String spec : prop.split(",")) { PrintStream ps = getPrintStreamFromSpec(spec); @@ -89,22 +86,29 @@ public abstract class ProviderFactory { } } - private static PrintStream getPrintStreamFromSpec(String spec) { + private static PrintStream getPrintStreamFromSpec(final String spec) { try { // spec is in the form of ., where is // a fully specified class name, and is a static member // in that class. The must be a 'PrintStream' or subtype // in order to be used. - int fieldpos = spec.lastIndexOf('.'); - Class cls = Class.forName(spec.substring(0, fieldpos)); - Field f = cls.getField(spec.substring(fieldpos + 1)); - Class fieldType = f.getType(); + final int fieldpos = spec.lastIndexOf('.'); + final Class cls = Class.forName(spec.substring(0, fieldpos)); + + Field f = AccessController.doPrivileged(new PrivilegedExceptionAction() { + public Field run() throws NoSuchFieldException { + return cls.getField(spec.substring(fieldpos + 1)); + } + }); + return (PrintStream)f.get(null); - } catch (Exception e) { - Logger.getAnonymousLogger().warning( - "Could not parse sun.tracing.stream property: " + e); + } catch (ClassNotFoundException e) { + throw new AssertionError(e); + } catch (IllegalAccessException e) { + throw new AssertionError(e); + } catch (PrivilegedActionException e) { + throw new AssertionError(e); } - return null; } } diff --git a/src/share/classes/sun/tracing/MultiplexProviderFactory.java b/src/share/classes/sun/tracing/MultiplexProviderFactory.java index 6ede31ec1..14c27c76d 100644 --- a/src/share/classes/sun/tracing/MultiplexProviderFactory.java +++ b/src/share/classes/sun/tracing/MultiplexProviderFactory.java @@ -30,7 +30,6 @@ import java.lang.reflect.InvocationTargetException; import java.util.HashMap; import java.util.HashSet; import java.util.Set; -import java.util.logging.Logger; import com.sun.tracing.ProviderFactory; import com.sun.tracing.Provider; @@ -65,13 +64,7 @@ public class MultiplexProviderFactory extends ProviderFactory { providers.add(factory.createProvider(cls)); } MultiplexProvider provider = new MultiplexProvider(cls, providers); - try { - provider.init(); - } catch (Exception e) { - // Probably a permission problem (can't get declared members) - Logger.getAnonymousLogger().warning( - "Could not initialize tracing provider: " + e.getMessage()); - } + provider.init(); return provider.newProxyInstance(); } } diff --git a/src/share/classes/sun/tracing/NullProviderFactory.java b/src/share/classes/sun/tracing/NullProviderFactory.java index f567acabb..5416fe35e 100644 --- a/src/share/classes/sun/tracing/NullProviderFactory.java +++ b/src/share/classes/sun/tracing/NullProviderFactory.java @@ -26,7 +26,6 @@ package sun.tracing; import java.lang.reflect.Method; -import java.util.logging.Logger; import com.sun.tracing.ProviderFactory; import com.sun.tracing.Provider; @@ -53,13 +52,7 @@ public class NullProviderFactory extends ProviderFactory { */ public T createProvider(Class cls) { NullProvider provider = new NullProvider(cls); - try { - provider.init(); - } catch (Exception e) { - // Probably a permission problem (can't get declared members) - Logger.getAnonymousLogger().warning( - "Could not initialize tracing provider: " + e.getMessage()); - } + provider.init(); return provider.newProxyInstance(); } } diff --git a/src/share/classes/sun/tracing/PrintStreamProviderFactory.java b/src/share/classes/sun/tracing/PrintStreamProviderFactory.java index 530130416..cca6bb58d 100644 --- a/src/share/classes/sun/tracing/PrintStreamProviderFactory.java +++ b/src/share/classes/sun/tracing/PrintStreamProviderFactory.java @@ -28,7 +28,6 @@ package sun.tracing; import java.lang.reflect.Method; import java.io.PrintStream; import java.util.HashMap; -import java.util.logging.Logger; import com.sun.tracing.ProviderFactory; import com.sun.tracing.Provider; @@ -54,13 +53,7 @@ public class PrintStreamProviderFactory extends ProviderFactory { public T createProvider(Class cls) { PrintStreamProvider provider = new PrintStreamProvider(cls, stream); - try { - provider.init(); - } catch (Exception e) { - // Probably a permission problem (can't get declared members) - Logger.getAnonymousLogger().warning( - "Could not initialize tracing provider: " + e.getMessage()); - } + provider.init(); return provider.newProxyInstance(); } } diff --git a/src/share/classes/sun/tracing/ProviderSkeleton.java b/src/share/classes/sun/tracing/ProviderSkeleton.java index 0178fe46e..dd7cbb197 100644 --- a/src/share/classes/sun/tracing/ProviderSkeleton.java +++ b/src/share/classes/sun/tracing/ProviderSkeleton.java @@ -32,6 +32,8 @@ import java.lang.reflect.InvocationTargetException; import java.lang.reflect.AnnotatedElement; import java.lang.annotation.Annotation; import java.util.HashMap; +import java.security.AccessController; +import java.security.PrivilegedAction; import com.sun.tracing.Provider; import com.sun.tracing.Probe; @@ -99,7 +101,13 @@ public abstract class ProviderSkeleton implements InvocationHandler, Provider { * It is up to the factory implementations to call this after construction. */ public void init() { - for (Method m : providerType.getDeclaredMethods()) { + Method[] methods = AccessController.doPrivileged(new PrivilegedAction() { + public Method[] run() { + return providerType.getDeclaredMethods(); + } + }); + + for (Method m : methods) { if ( m.getReturnType() != Void.TYPE ) { throw new IllegalArgumentException( "Return value of method is not void"); diff --git a/src/share/classes/sun/tracing/dtrace/DTraceProviderFactory.java b/src/share/classes/sun/tracing/dtrace/DTraceProviderFactory.java index 3148f6f6d..ce921d88e 100644 --- a/src/share/classes/sun/tracing/dtrace/DTraceProviderFactory.java +++ b/src/share/classes/sun/tracing/dtrace/DTraceProviderFactory.java @@ -29,7 +29,6 @@ import java.util.Map; import java.util.Set; import java.util.HashMap; import java.util.HashSet; -import java.util.logging.Logger; import java.security.Permission; import com.sun.tracing.ProviderFactory; @@ -80,15 +79,8 @@ public final class DTraceProviderFactory extends ProviderFactory { DTraceProvider jsdt = new DTraceProvider(cls); T proxy = jsdt.newProxyInstance(); jsdt.setProxy(proxy); - try { - jsdt.init(); - new Activation(jsdt.getModuleName(), new DTraceProvider[] { jsdt }); - } catch (Exception e) { - // Probably a permission problem (can't get declared members) - Logger.getAnonymousLogger().warning( - "Could not initialize tracing provider: " + e.getMessage()); - jsdt.dispose(); - } + jsdt.init(); + new Activation(jsdt.getModuleName(), new DTraceProvider[] { jsdt }); return proxy; } diff --git a/test/com/sun/tracing/BasicWithSecurityMgr.java b/test/com/sun/tracing/BasicWithSecurityMgr.java new file mode 100644 index 000000000..547eb0944 --- /dev/null +++ b/test/com/sun/tracing/BasicWithSecurityMgr.java @@ -0,0 +1,149 @@ +/* + * 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 6899605 + * @summary Basic unit test for tracing framework with security manager + * enabled + */ + +import com.sun.tracing.*; +import java.lang.reflect.Method; + +@ProviderName("NamedProvider") +interface BasicProvider extends Provider { + void plainProbe(); + void probeWithArgs(int a, float f, String s, Long l); + @ProbeName("namedProbe") void probeWithName(); + void overloadedProbe(); + void overloadedProbe(int i); +} + +interface InvalidProvider extends Provider { + int nonVoidProbe(); +} + +public class BasicWithSecurityMgr { + + public static ProviderFactory factory; + public static BasicProvider bp; + + public static void main(String[] args) throws Exception { + // enable security manager + System.setSecurityManager(new SecurityManager()); + + factory = ProviderFactory.getDefaultFactory(); + if (factory != null) { + bp = factory.createProvider(BasicProvider.class); + } + + testProviderFactory(); + testProbe(); + testProvider(); + } + + static void fail(String s) throws Exception { + throw new Exception(s); + } + + static void testProviderFactory() throws Exception { + if (factory == null) { + fail("ProviderFactory.getDefaultFactory: Did not create factory"); + } + if (bp == null) { + fail("ProviderFactory.createProvider: Did not create provider"); + } + try { + factory.createProvider(null); + fail("ProviderFactory.createProvider: Did not throw NPE for null"); + } catch (NullPointerException e) {} + + try { + factory.createProvider(InvalidProvider.class); + fail("Factory.createProvider: Should error with non-void probes"); + } catch (IllegalArgumentException e) {} + } + + public static void testProvider() throws Exception { + + // These just shouldn't throw any exeptions: + bp.plainProbe(); + bp.probeWithArgs(42, (float)3.14, "spam", new Long(2L)); + bp.probeWithArgs(42, (float)3.14, null, null); + bp.probeWithName(); + bp.overloadedProbe(); + bp.overloadedProbe(42); + + Method m = BasicProvider.class.getMethod("plainProbe"); + Probe p = bp.getProbe(m); + if (p == null) { + fail("Provider.getProbe: Did not return probe"); + } + + Method m2 = BasicWithSecurityMgr.class.getMethod("testProvider"); + p = bp.getProbe(m2); + if (p != null) { + fail("Provider.getProbe: Got probe with invalid spec"); + } + + bp.dispose(); + // These just shouldn't throw any exeptions: + bp.plainProbe(); + bp.probeWithArgs(42, (float)3.14, "spam", new Long(2L)); + bp.probeWithArgs(42, (float)3.14, null, null); + bp.probeWithName(); + bp.overloadedProbe(); + bp.overloadedProbe(42); + + if (bp.getProbe(m) != null) { + fail("Provider.getProbe: Should return null after dispose()"); + } + + bp.dispose(); // just to make sure nothing bad happens + } + + static void testProbe() throws Exception { + Method m = BasicProvider.class.getMethod("plainProbe"); + Probe p = bp.getProbe(m); + p.isEnabled(); // just make sure it doesn't do anything bad + p.trigger(); + + try { + p.trigger(0); + fail("Probe.trigger: too many arguments not caught"); + } catch (IllegalArgumentException e) {} + + p = bp.getProbe(BasicProvider.class.getMethod( + "probeWithArgs", int.class, float.class, String.class, Long.class)); + try { + p.trigger(); + fail("Probe.trigger: too few arguments not caught"); + } catch (IllegalArgumentException e) {} + + try { + p.trigger((float)3.14, (float)3.14, "", new Long(0L)); + fail("Probe.trigger: wrong type primitive arguments not caught"); + } catch (IllegalArgumentException e) {} + } +} -- GitLab