Fix FELIX-2716 [iPOJO] Failure when creating proxies for classes in java.* packages
Change the package name of smart proxies of java.* interfaces
Add a second check to be sure to never create proxy of non-interface type
Improve the logger messages (cosmetic fixes)
Avoid an NPE in the InstanceManager.
git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@1042125 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/ipojo/core/src/main/java/org/apache/felix/ipojo/InstanceManager.java b/ipojo/core/src/main/java/org/apache/felix/ipojo/InstanceManager.java
index 709d70c..4910516 100644
--- a/ipojo/core/src/main/java/org/apache/felix/ipojo/InstanceManager.java
+++ b/ipojo/core/src/main/java/org/apache/felix/ipojo/InstanceManager.java
@@ -1068,7 +1068,7 @@
private Method getMethodById(String methodId) {
// Not necessary synchronized as recomputing the methodID will give the same Method twice.
Method method = (Method) m_methods.get(methodId);
- if (method == null) {
+ if (method == null && m_clazz != null) {
Method[] mets = m_clazz.getDeclaredMethods();
for (int i = 0; i < mets.length; i++) {
// Check if the method was not already computed. If not, compute the Id and check.
diff --git a/ipojo/core/src/main/java/org/apache/felix/ipojo/handlers/dependency/Dependency.java b/ipojo/core/src/main/java/org/apache/felix/ipojo/handlers/dependency/Dependency.java
index c41c6f8..3ffe905 100644
--- a/ipojo/core/src/main/java/org/apache/felix/ipojo/handlers/dependency/Dependency.java
+++ b/ipojo/core/src/main/java/org/apache/felix/ipojo/handlers/dependency/Dependency.java
@@ -397,14 +397,20 @@
if (isAggregate()) {
m_proxyObject = new ServiceCollection(this);
} else {
- String type = getHandler().getInstanceManager().getContext().getProperty(DependencyHandler.PROXY_TYPE_PROPERTY);
- if (type == null || type.equals(DependencyHandler.SMART_PROXY)) {
- SmartProxyFactory proxyFactory = new SmartProxyFactory(this.getClass().getClassLoader());
- m_proxyObject = proxyFactory.getProxy(getSpecification(), this);
- } else {
- DynamicProxyFactory proxyFactory = new DynamicProxyFactory();
- m_proxyObject = proxyFactory.getProxy(getSpecification());
- }
+ // Can we really proxy ? We can proxy only interfaces.
+ if (getSpecification().isInterface()) {
+ String type = getHandler().getInstanceManager().getContext().getProperty(DependencyHandler.PROXY_TYPE_PROPERTY);
+ if (type == null || type.equals(DependencyHandler.SMART_PROXY)) {
+ SmartProxyFactory proxyFactory = new SmartProxyFactory(this.getClass().getClassLoader());
+ m_proxyObject = proxyFactory.getProxy(getSpecification(), this);
+ } else {
+ DynamicProxyFactory proxyFactory = new DynamicProxyFactory();
+ m_proxyObject = proxyFactory.getProxy(getSpecification());
+ }
+ } else {
+ m_handler.warn("Cannot create a proxy for a service dependency which is not an interface " +
+ "- disabling proxy for " + getId());
+ }
}
}
@@ -868,7 +874,12 @@
*/
protected Class getProxyClass(Class clazz) {
byte[] clz = ProxyGenerator.dumpProxy(clazz); // Generate the proxy.
- return defineClass(clazz.getName() + "$$Proxy", clz, 0, clz.length);
+ // Turn around the VM changes (FELIX-2716) about java.* classes.
+ String cn = clazz.getName();
+ if (cn.startsWith("java.")) {
+ cn = "$" + cn;
+ }
+ return defineClass(cn + "$$Proxy", clz, 0, clz.length);
}
/**
diff --git a/ipojo/core/src/main/java/org/apache/felix/ipojo/handlers/dependency/ProxyGenerator.java b/ipojo/core/src/main/java/org/apache/felix/ipojo/handlers/dependency/ProxyGenerator.java
index a7d965d..1dd2cf3 100644
--- a/ipojo/core/src/main/java/org/apache/felix/ipojo/handlers/dependency/ProxyGenerator.java
+++ b/ipojo/core/src/main/java/org/apache/felix/ipojo/handlers/dependency/ProxyGenerator.java
@@ -1,4 +1,4 @@
-/*
+/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
@@ -34,26 +34,26 @@
* @author <a href="mailto:dev@felix.apache.org">Felix Project Team</a>
*/
public class ProxyGenerator implements Opcodes {
-
+
/**
- * The dependency name.
+ * The dependency name.
*/
private static final String DEPENDENCY = "m_dependency";
-
+
/**
* The dependency descriptor.
*/
private static final String DEPENDENCY_DESC = Type.getDescriptor(Dependency.class);
-
+
/**
- * Dependency internal class name.
+ * Dependency internal class name.
*/
private static final String DEPENDENCY_INTERNAL_NAME = "org/apache/felix/ipojo/handlers/dependency/Dependency";
-
+
/**
- * Gets the internal names of the given class objects.
+ * Gets the internal names of the given class objects.
* @param classes the classes
- * @return the array containing internal names of the given class array.
+ * @return the array containing internal names of the given class array.
*/
private static String[] getInternalClassNames(Class[] classes) {
final String[] names = new String[classes.length];
@@ -62,7 +62,7 @@
}
return names;
}
-
+
/**
* Generates a proxy class.
* @param spec the proxied service specification
@@ -71,25 +71,39 @@
public static byte[] dumpProxy(Class spec) {
ClassWriter cw = new ClassWriter(ClassWriter.COMPUTE_MAXS);
String internalClassName = Type.getInternalName(spec); // Specification class internal name.
- String[] itfs = new String[] {internalClassName}; // Implemented interface.
+ String[] itfs = new String[0];
+ String parent = "java/lang/Object";
+ if (spec.isInterface()) {
+ itfs = new String[] {internalClassName}; // Implemented interface.
+ } else {
+ parent = internalClassName;
+ }
String className = internalClassName + "$$Proxy"; // Unique name.
+
+ // Turn around the VM changes (FELIX-2716) about java.* classes.
+ if (className.startsWith("java/")) {
+ className = "$" + className;
+ }
+
Method[] methods = spec.getMethods(); // Method to delegate
-
- cw.visit(Opcodes.V1_3, Opcodes.ACC_PUBLIC | Opcodes.ACC_FINAL, className, null, "java/lang/Object", itfs);
+
+ cw.visit(Opcodes.V1_3, Opcodes.ACC_PUBLIC | Opcodes.ACC_FINAL, className, null, parent, itfs);
addDependencyField(cw);
- generateConstructor(cw, className);
-
+
+ // We try to call super() on the parent, however this should not be used as proxing does work only for interface.
+ generateConstructor(cw, className, parent);
+
// For each method, create the delegator code.
for (int i = 0; i < methods.length; i++) {
if ((methods[i].getModifiers() & (Modifier.STATIC | Modifier.FINAL)) == 0) {
generateDelegator(cw, methods[i], className, internalClassName);
}
- }
-
+ }
+
cw.visitEnd();
-
+
return cw.toByteArray();
-
+
}
/**
@@ -121,10 +135,10 @@
mv.visitFieldInsn(GETFIELD, className, DEPENDENCY, DEPENDENCY_DESC); // The dependency is on the stack.
mv.visitMethodInsn(INVOKEVIRTUAL, DEPENDENCY_INTERNAL_NAME, "getService", // Call getService
"()Ljava/lang/Object;"); // The service object is on the stack.
- int varSvc = freeRoom;
+ int varSvc = freeRoom;
freeRoom = freeRoom + 1; // Object Reference.
mv.visitVarInsn(ASTORE, varSvc); // Store the service object.
-
+
Label notNull = new Label();
Label isNull = new Label();
mv.visitVarInsn(ALOAD, varSvc); // Load the service
@@ -137,16 +151,16 @@
mv.visitMethodInsn(INVOKESPECIAL, "java/lang/RuntimeException", "<init>", "(Ljava/lang/String;)V");
mv.visitInsn(ATHROW);
// End of the null branch
-
+
// Not null, go one the execution
mv.visitLabel(notNull);
-
+
// Invoke the method on the service object.
mv.visitVarInsn(ALOAD, varSvc);
// Push argument on the stack.
int i = 1; // Arguments. (non static method)
for (int t = 0; t < types.length; t++) {
- mv.visitVarInsn(types[t].getOpcode(ILOAD), i);
+ mv.visitVarInsn(types[t].getOpcode(ILOAD), i);
i = i + types[t].getSize();
}
// Invocation
@@ -171,13 +185,13 @@
* @param cw the class writer
* @param className the generated class name.
*/
- private static void generateConstructor(ClassWriter cw, String className) {
+ private static void generateConstructor(ClassWriter cw, String className, String parent) {
MethodVisitor mv = cw.visitMethod(Opcodes.ACC_PUBLIC, "<init>", '(' + DEPENDENCY_DESC + ")V", null, null);
mv.visitCode();
mv.visitVarInsn(ALOAD, 0); // Load this
mv.visitInsn(DUP); // Dup
- mv.visitMethodInsn(INVOKESPECIAL, "java/lang/Object", "<init>", "()V"); // Call super
+ mv.visitMethodInsn(INVOKESPECIAL, parent, "<init>", "()V"); // Call super
mv.visitVarInsn(ALOAD, 1); // Load the argument
mv.visitFieldInsn(PUTFIELD, className, DEPENDENCY, DEPENDENCY_DESC); // Assign the dependency field
mv.visitInsn(RETURN); // Return void
diff --git a/ipojo/core/src/main/java/org/apache/felix/ipojo/util/Logger.java b/ipojo/core/src/main/java/org/apache/felix/ipojo/util/Logger.java
index 5382603..387ff75 100644
--- a/ipojo/core/src/main/java/org/apache/felix/ipojo/util/Logger.java
+++ b/ipojo/core/src/main/java/org/apache/felix/ipojo/util/Logger.java
@@ -182,9 +182,14 @@
}
String message = null;
+ String name = m_name;
+ if (name == null) {
+ name = "";
+ }
+
switch (level) {
case DEBUG:
- message = "[DEBUG] " + m_name + " : " + msg;
+ message = "[DEBUG] " + name + " : " + msg;
if (log != null) {
log.log(LogService.LOG_DEBUG, message);
} else {
@@ -192,7 +197,7 @@
}
break;
case ERROR:
- message = "[ERROR] " + m_name + " : " + msg;
+ message = "[ERROR] " + name + " : " + msg;
if (log != null) {
log.log(LogService.LOG_ERROR, message);
} else {
@@ -200,7 +205,7 @@
}
break;
case INFO:
- message = "[INFO] " + m_name + " : " + msg;
+ message = "[INFO] " + name + " : " + msg;
if (log != null) {
log.log(LogService.LOG_INFO, message);
} else {
@@ -208,7 +213,7 @@
}
break;
case WARNING:
- message = "[WARNING] " + m_name + " : " + msg;
+ message = "[WARNING] " + name + " : " + msg;
if (log != null) {
log.log(LogService.LOG_WARNING, message);
} else {
@@ -216,7 +221,7 @@
}
break;
default:
- message = "[UNKNOWN] " + m_name + " : " + msg;
+ message = "[UNKNOWN] " + name + " : " + msg;
System.err.println(message);
break;
}
@@ -251,9 +256,14 @@
}
String message = null;
+ String name = m_name;
+ if (name == null) {
+ name = "";
+ }
+
switch (level) {
case DEBUG:
- message = "[DEBUG] " + m_name + " : " + msg;
+ message = "[DEBUG] " + name + " : " + msg;
if (log != null) {
log.log(LogService.LOG_DEBUG, message, exception);
} else {
@@ -262,7 +272,7 @@
}
break;
case ERROR:
- message = "[ERROR] " + m_name + " : " + msg;
+ message = "[ERROR] " + name + " : " + msg;
if (log != null) {
log.log(LogService.LOG_ERROR, message, exception);
} else {
@@ -271,7 +281,7 @@
}
break;
case INFO:
- message = "[INFO] " + m_name + " : " + msg;
+ message = "[INFO] " + name + " : " + msg;
if (log != null) {
log.log(LogService.LOG_INFO, message, exception);
} else {
@@ -280,7 +290,7 @@
}
break;
case WARNING:
- message = "[WARNING] " + m_name + " : " + msg;
+ message = "[WARNING] " + name + " : " + msg;
if (log != null) {
log.log(LogService.LOG_WARNING, message, exception);
} else {
@@ -289,7 +299,7 @@
}
break;
default:
- message = "[UNKNOWN] " + m_name + " : " + msg;
+ message = "[UNKNOWN] " + name + " : " + msg;
System.err.println(message);
exception.printStackTrace();
break;
diff --git a/ipojo/core/src/test/java/org/apache/felix/ipojo/handlers/dependency/SmartProxyTest.java b/ipojo/core/src/test/java/org/apache/felix/ipojo/handlers/dependency/SmartProxyTest.java
new file mode 100644
index 0000000..baf9725
--- /dev/null
+++ b/ipojo/core/src/test/java/org/apache/felix/ipojo/handlers/dependency/SmartProxyTest.java
@@ -0,0 +1,168 @@
+package org.apache.felix.ipojo.handlers.dependency;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.URL;
+import java.util.Dictionary;
+import java.util.Enumeration;
+import java.util.List;
+
+import javax.swing.Action;
+
+import junit.framework.Assert;
+import junit.framework.TestCase;
+
+import org.apache.felix.ipojo.ComponentFactory;
+import org.apache.felix.ipojo.InstanceManager;
+import org.apache.felix.ipojo.util.Logger;
+import org.mockito.Mockito;
+import org.osgi.framework.Bundle;
+import org.osgi.framework.BundleContext;
+import org.osgi.framework.BundleException;
+import org.osgi.framework.ServiceReference;
+
+public class SmartProxyTest extends TestCase {
+
+ private Dependency dependency;
+
+
+ public void setUp() {
+ }
+
+
+ /**
+ * Tests if we can proxies classes from java.* package.
+ * Indeed, a recent JVM bug fix introduces a bug:
+ * <code>
+ * [ERROR] test : Cannot create the proxy object
+ * java.lang.SecurityException: Prohibited package name: java.awt
+ * </code>
+ */
+ public void testProxiesOfJavaClasses() {
+ Bundle bundle = new Bundle() {
+
+ public int getState() {
+ return 0;
+ }
+
+ public void start() throws BundleException {
+ }
+
+ public void stop() throws BundleException {
+ }
+
+ public void update() throws BundleException {
+ }
+
+ public void update(InputStream in) throws BundleException {
+ }
+
+ public void uninstall() throws BundleException {
+ }
+
+ public Dictionary getHeaders() {
+ return null;
+ }
+
+ public long getBundleId() {
+ return 0;
+ }
+
+ public String getLocation() {
+ return null;
+ }
+
+ public ServiceReference[] getRegisteredServices() {
+ return null;
+ }
+
+ public ServiceReference[] getServicesInUse() {
+ return null;
+ }
+
+ public boolean hasPermission(Object permission) {
+ return false;
+ }
+
+ public URL getResource(String name) {
+ return null;
+ }
+
+ public Dictionary getHeaders(String locale) {
+ return null;
+ }
+
+ public String getSymbolicName() {
+ return null;
+ }
+
+ public Class loadClass(String name) throws ClassNotFoundException {
+ return Dependency.class.getClassLoader().loadClass(name);
+ }
+
+ public Enumeration getResources(String name) throws IOException {
+ return null;
+ }
+
+ public Enumeration getEntryPaths(String path) {
+ return null;
+ }
+
+ public URL getEntry(String name) {
+ return null;
+ }
+
+ public long getLastModified() {
+ return 0;
+ }
+
+ public Enumeration findEntries(String path, String filePattern,
+ boolean recurse) {
+ return null;
+ }
+
+ };
+
+
+ BundleContext context = (BundleContext) Mockito.mock(BundleContext.class);
+ Mockito.when(context.getProperty(DependencyHandler.PROXY_TYPE_PROPERTY)).thenReturn(null);
+ Mockito.when(context.getProperty(Logger.IPOJO_LOG_LEVEL_PROP)).thenReturn(null);
+ Mockito.when(context.getBundle()).thenReturn(bundle);
+
+ //getBundle().loadClass(name);
+
+ ComponentFactory factory = (ComponentFactory) Mockito.mock(ComponentFactory.class);
+ Mockito.when(factory.getBundleClassLoader()).thenReturn(Dependency.class.getClassLoader());
+
+ InstanceManager im = (InstanceManager) Mockito.mock(InstanceManager.class);
+ Mockito.when(im.getContext()).thenReturn(context);
+ Mockito.when(im.getFactory()).thenReturn(factory);
+
+ DependencyHandler handler = (DependencyHandler) Mockito.mock(DependencyHandler.class);
+ Mockito.when(handler.getInstanceManager()).thenReturn(im);
+ Logger logger = new Logger(context, "test", Logger.INFO);
+
+
+ Mockito.when(handler.getLogger()).thenReturn(logger);
+
+ // Try with java.List
+ dependency = new Dependency(handler, "a_field", List.class, null, false, false, false,
+ true, "dep", context, Dependency.DYNAMIC_BINDING_POLICY, null, null);
+ dependency.start();
+
+ // OK
+ Assert.assertNotNull(dependency.onGet(new Object(), "a_field", null));
+ Assert.assertTrue(dependency.onGet(new Object(), "a_field", null) instanceof List);
+
+ dependency.stop();
+
+ // Try with javax.swing.Action
+ dependency = new Dependency(handler, "a_field", Action.class, null, false, false, false,
+ true, "dep", context, Dependency.DYNAMIC_BINDING_POLICY, null, null);
+ dependency.start();
+ // OK
+ Assert.assertNotNull(dependency.onGet(new Object(), "a_field", null));
+ Assert.assertTrue(dependency.onGet(new Object(), "a_field", null) instanceof Action);
+ }
+
+}