Fix FELIX-4256 Avoid manipulation native and static methods from inner classes
Static inner classes and native method from inner classes are no more manipulated.
git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@1527604 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/ipojo/manipulator/manipulator/pom.xml b/ipojo/manipulator/manipulator/pom.xml
index 0d796dd..8dd60c5 100644
--- a/ipojo/manipulator/manipulator/pom.xml
+++ b/ipojo/manipulator/manipulator/pom.xml
@@ -71,6 +71,12 @@
<version>2.4</version>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.easytesting</groupId>
+ <artifactId>fest-assert</artifactId>
+ <version>1.4</version>
+ <scope>test</scope>
+ </dependency>
</dependencies>
<build>
<resources>
diff --git a/ipojo/manipulator/manipulator/src/main/java/org/apache/felix/ipojo/manipulation/ClassChecker.java b/ipojo/manipulator/manipulator/src/main/java/org/apache/felix/ipojo/manipulation/ClassChecker.java
index cced500..3dbebb7 100644
--- a/ipojo/manipulator/manipulator/src/main/java/org/apache/felix/ipojo/manipulation/ClassChecker.java
+++ b/ipojo/manipulator/manipulator/src/main/java/org/apache/felix/ipojo/manipulation/ClassChecker.java
@@ -148,7 +148,10 @@
*/
public void visitInnerClass(String name, String outerName, String innerName, int access) {
if (m_className.equals(outerName) || outerName == null) { // Anonymous classes does not have an outer class.
- m_inners.add(name);
+ // Do not include inner static class
+ if (! ((access & ACC_STATIC) == ACC_STATIC)) {
+ m_inners.add(name);
+ }
}
}
diff --git a/ipojo/manipulator/manipulator/src/main/java/org/apache/felix/ipojo/manipulation/InnerClassAdapter.java b/ipojo/manipulator/manipulator/src/main/java/org/apache/felix/ipojo/manipulation/InnerClassAdapter.java
index 033d687..facb172 100644
--- a/ipojo/manipulator/manipulator/src/main/java/org/apache/felix/ipojo/manipulation/InnerClassAdapter.java
+++ b/ipojo/manipulator/manipulator/src/main/java/org/apache/felix/ipojo/manipulation/InnerClassAdapter.java
@@ -19,16 +19,16 @@
package org.apache.felix.ipojo.manipulation;
-import java.util.Set;
+import org.objectweb.asm.*;
-import org.objectweb.asm.ClassAdapter;
-import org.objectweb.asm.ClassVisitor;
-import org.objectweb.asm.MethodVisitor;
-import org.objectweb.asm.Opcodes;
+import java.util.Set;
/**
* Adapts a inner class in order to allow accessing outer class fields.
* A manipulated inner class has access to the managed field of the outer class.
+ *
+ * Only non-static inner classes are manipulated, others are not.
+ *
* @author <a href="mailto:dev@felix.apache.org">Felix Project Team</a>
*/
public class InnerClassAdapter extends ClassAdapter implements Opcodes {
@@ -37,7 +37,6 @@
* Implementation class name.
*/
private String m_outer;
-
/**
* List of fields of the implementation class.
*/
@@ -45,9 +44,10 @@
/**
* Creates the inner class adapter.
- * @param arg0 parent class visitor
+ *
+ * @param arg0 parent class visitor
* @param outerClass outer class (implementation class)
- * @param fields fields of the implementation class
+ * @param fields fields of the implementation class
*/
public InnerClassAdapter(ClassVisitor arg0, String outerClass, Set<String> fields) {
super(arg0);
@@ -58,15 +58,29 @@
/**
* Visits a method.
* This methods create a code visitor manipulating outer class field accesses.
- * @param access method visibility
- * @param name method name
- * @param desc method descriptor
- * @param signature method signature
+ *
+ * @param access method visibility
+ * @param name method name
+ * @param desc method descriptor
+ * @param signature method signature
* @param exceptions list of exceptions thrown by the method
* @return a code adapter manipulating field accesses
* @see org.objectweb.asm.ClassAdapter#visitMethod(int, java.lang.String, java.lang.String, java.lang.String, java.lang.String[])
*/
public MethodVisitor visitMethod(int access, String name, String desc, String signature, String[] exceptions) {
+ // For non static and non constructor method, generate the wrapping method and move the method content within
+ // the manipulated method.
+
+ // Do nothing on static methods, should not happen in non-static inner classes.
+ if ((access & ACC_STATIC) == ACC_STATIC) {
+ return super.visitMethod(access, name, desc, signature, exceptions);
+ }
+
+ // Do nothing on native methods
+ if ((access & ACC_NATIVE) == ACC_NATIVE) {
+ return super.visitMethod(access, name, desc, signature, exceptions);
+ }
+
MethodVisitor mv = super.visitMethod(access, name, desc, signature, exceptions);
return new MethodCodeAdapter(mv, m_outer, access, name, desc, m_fields);
}
diff --git a/ipojo/manipulator/manipulator/src/test/java/org/apache/felix/ipojo/manipulation/InnerClassAdapterTest.java b/ipojo/manipulator/manipulator/src/test/java/org/apache/felix/ipojo/manipulation/InnerClassAdapterTest.java
new file mode 100644
index 0000000..2e1bd61
--- /dev/null
+++ b/ipojo/manipulator/manipulator/src/test/java/org/apache/felix/ipojo/manipulation/InnerClassAdapterTest.java
@@ -0,0 +1,149 @@
+/*
+ * 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
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.felix.ipojo.manipulation;
+
+import junit.framework.Assert;
+import org.apache.felix.ipojo.InstanceManager;
+import org.apache.felix.ipojo.Pojo;
+import org.junit.Ignore;
+import org.junit.Test;
+import org.mockito.Mockito;
+
+import java.io.File;
+import java.io.IOException;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.util.Arrays;
+
+import static junit.framework.Assert.assertEquals;
+import static org.fest.assertions.Assertions.assertThat;
+
+/**
+ * Checks the inner class manipulation
+ */
+public class InnerClassAdapterTest {
+
+ public static String baseClassDirectory = "target/test-classes/";
+
+ private static ManipulatedClassLoader manipulate(String className, Manipulator manipulator) throws IOException {
+
+ byte[] clazz = manipulator.manipulate(ManipulatorTest.getBytesFromFile(new File
+ (baseClassDirectory + className.replace(".", "/") + ".class")));
+ ManipulatedClassLoader classloader = new ManipulatedClassLoader(className, clazz);
+
+ // Manipulate all inner classes
+ for (String s : manipulator.getInnerClasses()) {
+ String outerClassInternalName = className.replace(".", "/");
+ byte[] innerClassBytecode = ManipulatorTest.getBytesFromFile(new File(baseClassDirectory + s + "" +
+ ".class"));
+ String innerClassName = s.replace("/", ".");
+ InnerClassManipulator innerManipulator = new InnerClassManipulator(outerClassInternalName, manipulator.getFields().keySet());
+ byte[] manipulated = innerManipulator.manipulate(innerClassBytecode, manipulator.getClassVersion());
+ classloader.addInnerClass(innerClassName, manipulated);
+ }
+
+ return classloader;
+ }
+
+ @Test
+ public void testManipulatingTheInner() throws Exception {
+ Manipulator manipulator = new Manipulator();
+ String className = "test.PojoWithInner";
+ byte[] origin = ManipulatorTest.getBytesFromFile(new File(baseClassDirectory + className.replace(".",
+ "/") + ".class"));
+
+ ManipulatedClassLoader classloader = manipulate(className, manipulator);
+
+ Class cl = classloader.findClass(className);
+ Assert.assertNotNull(cl);
+ Assert.assertNotNull(manipulator.getManipulationMetadata());
+ Assert.assertFalse(manipulator.getInnerClasses().isEmpty());
+
+ System.out.println(manipulator.getManipulationMetadata());
+
+ // The manipulation add stuff to the class.
+ Assert.assertTrue(classloader.get(className).length > origin.length);
+
+
+ boolean found = false;
+ Constructor cst = null;
+ Constructor[] csts = cl.getDeclaredConstructors();
+ for (int i = 0; i < csts.length; i++) {
+ System.out.println(Arrays.asList(csts[i].getParameterTypes()));
+ if (csts[i].getParameterTypes().length == 1 &&
+ csts[i].getParameterTypes()[0].equals(InstanceManager.class)) {
+ found = true;
+ cst = csts[i];
+ }
+ }
+ Assert.assertTrue(found);
+
+ // We still have the empty constructor
+ found = false;
+ csts = cl.getDeclaredConstructors();
+ for (Constructor cst1 : csts) {
+ System.out.println(Arrays.asList(cst1.getParameterTypes()));
+ if (cst1.getParameterTypes().length == 0) {
+ found = true;
+ }
+ }
+ Assert.assertTrue(found);
+
+ // Check the POJO interface
+ Assert.assertTrue(Arrays.asList(cl.getInterfaces()).contains(Pojo.class));
+
+ InstanceManager im = Mockito.mock(InstanceManager.class);
+ cst.setAccessible(true);
+ Object pojo = cst.newInstance(new Object[] {im});
+ Assert.assertNotNull(pojo);
+ Assert.assertTrue(pojo instanceof Pojo);
+ Method method = cl.getMethod("doSomething", new Class[0]);
+ Assert.assertTrue(((Boolean) method.invoke(pojo, new Object[0])).booleanValue());
+
+ }
+
+
+ @Test
+ public void testInnerClasses() throws IOException, ClassNotFoundException, NoSuchMethodException, IllegalAccessException, InvocationTargetException, InstantiationException {
+ Manipulator manipulator = new Manipulator();
+ String className = "test.inner.ComponentWithInnerClasses";
+ ManipulatedClassLoader classloader = manipulate(className, manipulator);
+
+ Class clazz = classloader.findClass(className);
+ Assert.assertNotNull(clazz);
+ Assert.assertNotNull(manipulator.getManipulationMetadata());
+ Assert.assertFalse(manipulator.getInnerClasses().isEmpty());
+ // We should have found only 2 inner classes.
+ assertThat(manipulator.getInnerClasses().size()).isEqualTo(3);
+
+ // Check that all inner classes are manipulated.
+ InstanceManager im = Mockito.mock(InstanceManager.class);
+ Constructor constructor = clazz.getDeclaredConstructor(InstanceManager.class);
+ constructor.setAccessible(true);
+ Object pojo = constructor.newInstance(new Object[] {im});
+ Assert.assertNotNull(pojo);
+ Assert.assertTrue(pojo instanceof Pojo);
+ Method method = clazz.getMethod("doSomething", new Class[0]);
+ String result = (String) method.invoke(pojo);
+ assertEquals(result, "foofoofoofoo");
+ }
+
+}
diff --git a/ipojo/manipulator/manipulator/src/test/java/org/apache/felix/ipojo/manipulation/ManipulatedClassLoader.java b/ipojo/manipulator/manipulator/src/test/java/org/apache/felix/ipojo/manipulation/ManipulatedClassLoader.java
index b511c75..eae2eb7 100644
--- a/ipojo/manipulator/manipulator/src/test/java/org/apache/felix/ipojo/manipulation/ManipulatedClassLoader.java
+++ b/ipojo/manipulator/manipulator/src/test/java/org/apache/felix/ipojo/manipulation/ManipulatedClassLoader.java
@@ -19,6 +19,9 @@
package org.apache.felix.ipojo.manipulation;
+import java.util.LinkedHashMap;
+import java.util.Map;
+
/**
* A classloader used to load manipulated classes.
*/
@@ -27,19 +30,43 @@
private String name;
private byte[] clazz;
+ private Map<String, byte[]> inner = new LinkedHashMap<String, byte[]>();
+
public ManipulatedClassLoader(String name, byte[] clazz) {
this.name = name;
this.clazz = clazz;
}
+ public byte[] get(String name) {
+ if (name.equals(this.name)) {
+ return clazz;
+ }
+ if (inner.containsKey(name)) {
+ return inner.get(name);
+ }
+ return null;
+ }
+
+ public void addInnerClass(String name, byte[] clazz) {
+ inner.put(name, clazz);
+ }
+
public Class findClass(String name) throws ClassNotFoundException {
if (name.equals(this.name)) {
return defineClass(name, clazz, 0, clazz.length);
}
+
+ if (inner.containsKey(name)) {
+ return defineClass(name, inner.get(name), 0, inner.get(name).length);
+ }
+
return super.findClass(name);
}
- public Class loadClass(String arg0) throws ClassNotFoundException {
- return super.loadClass(arg0);
+ public Class loadClass(String classname) throws ClassNotFoundException {
+ if (inner.containsKey(classname)) {
+ return findClass(classname);
+ }
+ return super.loadClass(classname);
}
}
diff --git a/ipojo/manipulator/manipulator/src/test/java/org/apache/felix/ipojo/manipulation/ManipulatorTest.java b/ipojo/manipulator/manipulator/src/test/java/org/apache/felix/ipojo/manipulation/ManipulatorTest.java
index d98497b..ff0f772 100644
--- a/ipojo/manipulator/manipulator/src/test/java/org/apache/felix/ipojo/manipulation/ManipulatorTest.java
+++ b/ipojo/manipulator/manipulator/src/test/java/org/apache/felix/ipojo/manipulation/ManipulatorTest.java
@@ -197,61 +197,6 @@
}
- @Ignore("This test requires a classloader storing the inner class")
- public void _testManipulatingTheInner() throws Exception {
- Manipulator manipulator = new Manipulator();
- byte[] clazz = manipulator.manipulate(getBytesFromFile(new File("target/test-classes/test/PojoWithInner.class")));
- ManipulatedClassLoader classloader = new ManipulatedClassLoader("test.PojoWithInner", clazz);
- Class cl = classloader.findClass("test.PojoWithInner");
- Assert.assertNotNull(cl);
- Assert.assertNotNull(manipulator.getManipulationMetadata());
- Assert.assertFalse(manipulator.getInnerClasses().isEmpty());
-
-
- System.out.println(manipulator.getManipulationMetadata());
-
- // The manipulation add stuff to the class.
- Assert.assertTrue(clazz.length > getBytesFromFile(new File("target/test-classes/test/PojoWithInner.class")).length);
-
-
- boolean found = false;
- Constructor cst = null;
- Constructor[] csts = cl.getDeclaredConstructors();
- for (int i = 0; i < csts.length; i++) {
- System.out.println(Arrays.asList(csts[i].getParameterTypes()));
- if (csts[i].getParameterTypes().length == 1 &&
- csts[i].getParameterTypes()[0].equals(InstanceManager.class)) {
- found = true;
- cst = csts[i];
- }
- }
- Assert.assertTrue(found);
-
- // We still have the empty constructor
- found = false;
- csts = cl.getDeclaredConstructors();
- for (int i = 0; i < csts.length; i++) {
- System.out.println(Arrays.asList(csts[i].getParameterTypes()));
- if (csts[i].getParameterTypes().length == 0) {
- found = true;
- }
- }
- Assert.assertTrue(found);
-
- // Check the POJO interface
- Assert.assertTrue(Arrays.asList(cl.getInterfaces()).contains(Pojo.class));
-
- InstanceManager im = (InstanceManager) Mockito.mock(InstanceManager.class);
- cst.setAccessible(true);
- Object pojo = cst.newInstance(new Object[] {im});
- Assert.assertNotNull(pojo);
- Assert.assertTrue(pojo instanceof Pojo);
-
- Method method = cl.getMethod("doSomething", new Class[0]);
- Assert.assertTrue(((Boolean) method.invoke(pojo, new Object[0])).booleanValue());
-
- }
-
public void testManipulatingWithConstructorModification() throws Exception {
Manipulator manipulator = new Manipulator();
byte[] clazz = manipulator.manipulate(getBytesFromFile(new File("target/test-classes/test/Child.class")));
diff --git a/ipojo/manipulator/manipulator/src/test/java/test/inner/ComponentWithInnerClasses.java b/ipojo/manipulator/manipulator/src/test/java/test/inner/ComponentWithInnerClasses.java
new file mode 100644
index 0000000..f568c7e
--- /dev/null
+++ b/ipojo/manipulator/manipulator/src/test/java/test/inner/ComponentWithInnerClasses.java
@@ -0,0 +1,72 @@
+/*
+ * 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
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package test.inner;
+
+import org.apache.felix.ipojo.annotations.Component;
+
+/**
+ * A component containing inner classes.
+ */
+@Component
+public class ComponentWithInnerClasses{
+
+ public String doSomething() {
+ MyInnerWithANativeMethod nat = new MyInnerWithANativeMethod();
+ MyInnerClass inn = new MyInnerClass();
+ Computation compute = new Computation() {
+
+ public String compute() {
+ return "foo";
+ }
+ };
+ return nat.foo() + MyStaticInnerClass.foo() + inn.foo() + compute.compute();
+ }
+
+ private class MyInnerWithANativeMethod {
+
+ public String foo() {
+ return "foo";
+ }
+
+ public native void baz();
+
+ }
+
+ public static class MyStaticInnerClass {
+
+ public static String foo() {
+ return "foo";
+ }
+
+ public String bar() {
+ return "bar";
+ }
+
+ public native void baz();
+ }
+
+ private class MyInnerClass {
+ public String foo() {
+ return "foo";
+ }
+ }
+
+
+}
diff --git a/ipojo/manipulator/manipulator/src/test/java/test/inner/Computation.java b/ipojo/manipulator/manipulator/src/test/java/test/inner/Computation.java
new file mode 100644
index 0000000..d9252b9
--- /dev/null
+++ b/ipojo/manipulator/manipulator/src/test/java/test/inner/Computation.java
@@ -0,0 +1,28 @@
+/*
+ * 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
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package test.inner;
+
+/**
+ * A simple interface intended to be implemented by an anonymous class.
+ */
+public interface Computation {
+
+ public String compute();
+}