From e52391816db6f8ed4c48ec415d4ae929b7974052 Mon Sep 17 00:00:00 2001 From: Dustin Byrne Date: Thu, 18 Mar 2021 01:14:00 -0400 Subject: [PATCH] fix: prevent potential null access when retrieving source paths --- .../appland/appmap/output/v1/CodeObject.java | 50 ++++++++++---- .../appmap/output/v1/CodeObjectTest.java | 68 +++++++++++++++++++ .../appmap/test/util/ClassBuilderTest.java | 12 +++- 3 files changed, 116 insertions(+), 14 deletions(-) create mode 100644 src/test/java/com/appland/appmap/output/v1/CodeObjectTest.java diff --git a/src/main/java/com/appland/appmap/output/v1/CodeObject.java b/src/main/java/com/appland/appmap/output/v1/CodeObject.java index 6b0e1369..226826e1 100644 --- a/src/main/java/com/appland/appmap/output/v1/CodeObject.java +++ b/src/main/java/com/appland/appmap/output/v1/CodeObject.java @@ -6,6 +6,7 @@ import javassist.CtBehavior; import javassist.CtClass; +import javassist.bytecode.ClassFile; import java.lang.reflect.Modifier; import java.util.ArrayDeque; @@ -194,11 +195,24 @@ public CodeObject(CodeObject src) { * @return An estimated source file path */ public static String getSourceFilePath(CtClass classType) { - String[] parts = { - "src", "main", "java", - classType.getPackageName().replace('.', '/'), - classType.getClassFile().getSourceFile() - }; + final List parts = new ArrayList(Arrays.asList("src", "main", "java")); + final String packageName = classType.getPackageName(); + + if (packageName != null) { + parts.add(packageName.replace('.', '/')); + } + + final ClassFile classFile = classType.getClassFile(); + if (classFile != null) { + String sourceFile = classFile.getSourceFile(); + if (sourceFile != null) { + parts.add(sourceFile); + } else { + final String className = classType.getName().replaceAll("\\$.*", ""); + parts.add(className + ".java"); + } + } + return String.join("/", parts); } @@ -209,7 +223,13 @@ public static String getSourceFilePath(CtClass classType) { * @return The root CodeObject */ public static CodeObject createTree(String packageName) { - String[] packageTokens = packageName.split("\\."); + final List packageTokens = new ArrayList(); + if (packageName != null) { + for (String token : packageName.split("\\.")) { + packageTokens.add(token); + } + } + CodeObject rootObject = null; CodeObject previousObject = null; @@ -241,15 +261,19 @@ public static CodeObject createTree(String packageName) { */ public static CodeObject createTree(CtClass classType) { String packageName = classType.getPackageName(); + CodeObject classObj = new CodeObject(classType); CodeObject rootObject = CodeObject.createTree(packageName); - CodeObject pkgLeafObject = rootObject.get(packageName); - if (pkgLeafObject == null) { - Logger.println("failed to get leaf pkg object for package " + packageName); - return null; - } + if (rootObject == null) { + rootObject = classObj; + } else { + CodeObject pkgLeafObject = rootObject.get(packageName); + if (pkgLeafObject == null) { + Logger.println("failed to get leaf pkg object for package " + packageName); + return null; + } - CodeObject classObj = new CodeObject(classType); - pkgLeafObject.addChild(classObj); + pkgLeafObject.addChild(classObj); + } return rootObject; } diff --git a/src/test/java/com/appland/appmap/output/v1/CodeObjectTest.java b/src/test/java/com/appland/appmap/output/v1/CodeObjectTest.java new file mode 100644 index 00000000..6b3190ee --- /dev/null +++ b/src/test/java/com/appland/appmap/output/v1/CodeObjectTest.java @@ -0,0 +1,68 @@ +package src.test.java.com.appland.appmap.output.v1; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +import com.appland.appmap.output.v1.CodeObject; +import com.appland.appmap.output.v1.Event; +import com.appland.appmap.test.util.ClassBuilder; + +import javassist.CtClass; + +import org.apache.commons.lang3.StringUtils; +import org.junit.Before; +import org.junit.Test; + +import java.util.List; + +public class CodeObjectTest { + + /** + * Validate the objects in a CodeObject tree match the given names and types + * @param obj The root object + * @param names The names of the objects, beginning with the root + * @param types The types of the objects, beginning with the root + */ + private void validateCodeObjectTree(CodeObject obj, String[] names, String[] types) { + CodeObject currentObject = obj; + assertTrue(currentObject != null); + + for (int i = 0; currentObject != null; ++i) { + assertEquals(currentObject.name, names[i]); + assertEquals(currentObject.type, types[i]); + + List children = currentObject.getChildren(); + if (children != null && children.size() > 0) { + currentObject = children.get(0); + } else { + currentObject = null; + } + } + } + + @Test + public void testGetSourceFilePath() { + CtClass testClass = new ClassBuilder("testGetSourceFilePath1").ctClass(); + assertEquals(CodeObject.getSourceFilePath(testClass), "src/main/java/testGetSourceFilePath1.java"); + + testClass = new ClassBuilder("com.myorg.testGetSourceFilePath2").ctClass(); + assertEquals(CodeObject.getSourceFilePath(testClass), "src/main/java/com/myorg/testGetSourceFilePath2.java"); + } + + @Test + public void testCreateTree() { + CtClass testClass = new ClassBuilder("testCreateTree").ctClass(); + validateCodeObjectTree( + CodeObject.createTree(testClass), + new String[] { "testCreateTree" }, + new String[] { "class" } + ); + + testClass = new ClassBuilder("com.myorg.testCreateTree").ctClass(); + validateCodeObjectTree( + CodeObject.createTree(testClass), + new String[] { "com", "myorg", "testCreateTree" }, + new String[] { "package", "package", "class" } + ); + } +} diff --git a/src/test/java/com/appland/appmap/test/util/ClassBuilderTest.java b/src/test/java/com/appland/appmap/test/util/ClassBuilderTest.java index 1167daab..fc4e0dad 100644 --- a/src/test/java/com/appland/appmap/test/util/ClassBuilderTest.java +++ b/src/test/java/com/appland/appmap/test/util/ClassBuilderTest.java @@ -4,7 +4,10 @@ import org.junit.Test; import org.junit.contrib.java.lang.system.SystemOutRule; +import javassist.CtClass; + import static org.junit.Assert.assertTrue; +import static org.junit.Assert.assertEquals; public class ClassBuilderTest { @Rule @@ -17,4 +20,11 @@ public void testBuild() throws Exception { assertTrue(testClass.isInstance(obj)); } -} \ No newline at end of file + + @Test + public void testAttributes() { + CtClass testClass = new ClassBuilder("com.example.TestAttributes").ctClass(); + assertEquals(testClass.getPackageName(), "com.example"); + assertEquals(testClass.getClassFile().getSourceFile(), "TestAttributes.java"); + } +}