From c93e9eea1483b28d0cc078158c6b1f2e3b6b7bd7 Mon Sep 17 00:00:00 2001 From: Skylot Date: Sun, 13 Mar 2022 12:08:03 +0000 Subject: [PATCH] fix: improve class names collision detection (#1406) --- .../main/java/jadx/core/codegen/ClassGen.java | 9 ++- .../java/jadx/tests/api/IntegrationTest.java | 76 ++++++++++++++----- .../names/TestClassNamesCollision.java | 32 ++++++++ .../names/TestClassNamesCollision2.java | 37 +++++++++ .../jadx/tests/integration/names/pkg/a.java | 8 ++ .../jadx/tests/integration/names/pkg/b.java | 9 +++ 6 files changed, 149 insertions(+), 22 deletions(-) create mode 100644 jadx-core/src/test/java/jadx/tests/integration/names/TestClassNamesCollision.java create mode 100644 jadx-core/src/test/java/jadx/tests/integration/names/TestClassNamesCollision2.java create mode 100644 jadx-core/src/test/java/jadx/tests/integration/names/pkg/a.java create mode 100644 jadx-core/src/test/java/jadx/tests/integration/names/pkg/b.java diff --git a/jadx-core/src/main/java/jadx/core/codegen/ClassGen.java b/jadx-core/src/main/java/jadx/core/codegen/ClassGen.java index 5e6f344bc..7b33af0dd 100644 --- a/jadx-core/src/main/java/jadx/core/codegen/ClassGen.java +++ b/jadx-core/src/main/java/jadx/core/codegen/ClassGen.java @@ -611,6 +611,9 @@ public class ClassGen { return fullName; } String shortName = extClsInfo.getAliasShortName(); + if (useCls.equals(extClsInfo)) { + return shortName; + } if (extClsInfo.getPackage().equals("java.lang") && extClsInfo.getParentClass() == null) { return shortName; } @@ -620,6 +623,9 @@ public class ClassGen { if (extClsInfo.isInner()) { return expandInnerClassName(useCls, extClsInfo); } + if (searchCollision(cls.root(), useCls, extClsInfo)) { + return fullName; + } if (isBothClassesInOneTopClass(useCls, extClsInfo)) { return shortName; } @@ -627,9 +633,6 @@ public class ClassGen { if (extClsInfo.getPackage().equals(useCls.getPackage()) && !extClsInfo.isInner()) { return shortName; } - if (searchCollision(cls.root(), useCls, extClsInfo)) { - return fullName; - } // ignore classes from default package if (extClsInfo.isDefaultPackage()) { return shortName; diff --git a/jadx-core/src/test/java/jadx/tests/api/IntegrationTest.java b/jadx-core/src/test/java/jadx/tests/api/IntegrationTest.java index 348ecc073..995a2d2d5 100644 --- a/jadx-core/src/test/java/jadx/tests/api/IntegrationTest.java +++ b/jadx-core/src/test/java/jadx/tests/api/IntegrationTest.java @@ -8,6 +8,7 @@ import java.lang.reflect.Method; import java.lang.reflect.Modifier; import java.nio.file.Files; import java.nio.file.Path; +import java.util.Collection; import java.util.Collections; import java.util.List; import java.util.Map; @@ -20,6 +21,8 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; import java.util.jar.JarEntry; import java.util.jar.JarOutputStream; +import java.util.stream.Collectors; +import java.util.stream.Stream; import org.jetbrains.annotations.Nullable; import org.junit.jupiter.api.AfterEach; @@ -35,6 +38,7 @@ import jadx.api.ICodeWriter; import jadx.api.JadxArgs; import jadx.api.JadxDecompiler; import jadx.api.JadxInternalAccess; +import jadx.api.JavaClass; import jadx.api.args.DeobfuscationMapFileMode; import jadx.api.data.annotations.InsnCodeOffset; import jadx.core.dex.attributes.AFlag; @@ -60,6 +64,7 @@ import static org.apache.commons.lang3.StringUtils.rightPad; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.empty; +import static org.hamcrest.Matchers.emptyArray; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.notNullValue; @@ -166,8 +171,22 @@ public abstract class IntegrationTest extends TestUtils { } catch (Exception e) { LOG.error("Failed to get class node", e); fail(e.getMessage()); + return null; + } + } + + public List getClassNodes(Class... classes) { + try { + assertThat("Class list is empty", classes, not(emptyArray())); + List srcFiles = Stream.of(classes).map(this::getSourceFileForClass).collect(Collectors.toList()); + List clsFiles = compileSourceFiles(srcFiles); + assertThat("Class files list is empty", clsFiles, not(empty())); + return decompileFiles(clsFiles); + } catch (Exception e) { + LOG.error("Failed to get class node", e); + fail(e.getMessage()); + return null; } - return null; } public ClassNode getClassNodeFromFiles(List files, String clsName) { @@ -188,6 +207,18 @@ public abstract class IntegrationTest extends TestUtils { return cls; } + public List decompileFiles(List files) { + jadxDecompiler = loadFiles(files); + List sortedClsNodes = jadxDecompiler.getDecompileScheduler() + .buildBatches(jadxDecompiler.getClasses()) + .stream() + .flatMap(Collection::stream) + .map(JavaClass::getClassNode) + .collect(Collectors.toList()); + decompileAndCheck(sortedClsNodes); + return sortedClsNodes; + } + @Nullable public ClassNode searchCls(List list, String clsName) { for (ClassNode cls : list) { @@ -256,7 +287,7 @@ public abstract class IntegrationTest extends TestUtils { protected void runChecks(List clsList) { clsList.forEach(this::checkCode); - compile(clsList); + compileClassNode(clsList); clsList.forEach(this::runAutoCheck); } @@ -439,7 +470,7 @@ public abstract class IntegrationTest extends TestUtils { return null; } - void compile(List clsList) { + void compileClassNode(List clsList) { if (!compile) { return; } @@ -460,33 +491,40 @@ public abstract class IntegrationTest extends TestUtils { } private List compileClass(Class cls) throws IOException { - String clsFullName = cls.getName(); - String rootClsName; - int end = clsFullName.indexOf('$'); - if (end != -1) { - rootClsName = clsFullName.substring(0, end); - } else { - rootClsName = clsFullName; + File sourceFile = getSourceFileForClass(cls); + List clsFiles = compileSourceFiles(Collections.singletonList(sourceFile)); + if (removeParentClassOnInput) { + // remove classes which are parents for test class + String clsFullName = cls.getName(); + String clsName = clsFullName.substring(clsFullName.lastIndexOf('.') + 1); + clsFiles.removeIf(next -> !next.getName().contains(clsName)); } + return clsFiles; + } + + private File getSourceFileForClass(Class cls) { + String clsFullName = cls.getName(); + int innerEnd = clsFullName.indexOf('$'); + String rootClsName = innerEnd == -1 ? clsFullName : clsFullName.substring(0, innerEnd); String javaFileName = rootClsName.replace('.', '/') + ".java"; File file = new File(TEST_DIRECTORY, javaFileName); - if (!file.exists()) { - file = new File(TEST_DIRECTORY2, javaFileName); + if (file.exists()) { + return file; } - assertThat("Test source file not found: " + javaFileName, file.exists(), is(true)); - List compileFileList = Collections.singletonList(file); + File file2 = new File(TEST_DIRECTORY2, javaFileName); + if (file2.exists()) { + return file2; + } + throw new JadxRuntimeException("Test source not found for class: " + clsFullName); + } + private List compileSourceFiles(List compileFileList) throws IOException { Path outTmp = FileUtils.createTempDir("jadx-tmp-classes"); sourceCompiler = new TestCompiler(compilerOptions); List files = sourceCompiler.compileFiles(compileFileList, outTmp); if (saveTestJar) { saveToJar(files, outTmp); } - if (removeParentClassOnInput) { - // remove classes which are parents for test class - String clsName = clsFullName.substring(clsFullName.lastIndexOf('.') + 1); - files.removeIf(next -> !next.getName().contains(clsName)); - } return files; } diff --git a/jadx-core/src/test/java/jadx/tests/integration/names/TestClassNamesCollision.java b/jadx-core/src/test/java/jadx/tests/integration/names/TestClassNamesCollision.java new file mode 100644 index 000000000..28865a5be --- /dev/null +++ b/jadx-core/src/test/java/jadx/tests/integration/names/TestClassNamesCollision.java @@ -0,0 +1,32 @@ +package jadx.tests.integration.names; + +import java.util.List; + +import org.junit.jupiter.api.Test; + +import jadx.api.CommentsLevel; +import jadx.core.dex.nodes.ClassNode; +import jadx.tests.api.IntegrationTest; +import jadx.tests.integration.names.pkg.a; +import jadx.tests.integration.names.pkg.b; + +import static jadx.tests.api.utils.assertj.JadxAssertions.assertThat; + +public class TestClassNamesCollision extends IntegrationTest { + + @Test + public void test() { + getArgs().setCommentsLevel(CommentsLevel.WARN); + List classNodes = getClassNodes(a.class, b.class); + + assertThat(searchCls(classNodes, "a")) + .code() + .containsOne("public class a {") + .containsOne("public static a a() {"); + + assertThat(searchCls(classNodes, "b")) + .code() + .containsOne("class a {") + .containsOne("jadx.tests.integration.names.pkg.a a = jadx.tests.integration.names.pkg.a.a();"); + } +} diff --git a/jadx-core/src/test/java/jadx/tests/integration/names/TestClassNamesCollision2.java b/jadx-core/src/test/java/jadx/tests/integration/names/TestClassNamesCollision2.java new file mode 100644 index 000000000..e92c8e3bc --- /dev/null +++ b/jadx-core/src/test/java/jadx/tests/integration/names/TestClassNamesCollision2.java @@ -0,0 +1,37 @@ +package jadx.tests.integration.names; + +import java.util.List; + +import org.junit.jupiter.api.Test; + +import jadx.api.CommentsLevel; +import jadx.tests.api.IntegrationTest; + +import static jadx.tests.api.utils.assertj.JadxAssertions.assertThat; + +public class TestClassNamesCollision2 extends IntegrationTest { + + @SuppressWarnings("rawtypes") + public static class TestCls { + static class List { + public static List getList() { + return null; + } + } + + protected List list = List.getList(); + + protected void clearList(java.util.List l) { + l.clear(); + } + } + + @Test + public void test() { + getArgs().setCommentsLevel(CommentsLevel.WARN); + assertThat(getClassNode(TestCls.class)) + .code() + .containsOne("static class List {") + .containsOne("protected void clearList(java.util.List l) {"); + } +} diff --git a/jadx-core/src/test/java/jadx/tests/integration/names/pkg/a.java b/jadx-core/src/test/java/jadx/tests/integration/names/pkg/a.java new file mode 100644 index 000000000..d2b08e470 --- /dev/null +++ b/jadx-core/src/test/java/jadx/tests/integration/names/pkg/a.java @@ -0,0 +1,8 @@ +package jadx.tests.integration.names.pkg; + +@SuppressWarnings({ "TypeName", "MethodName" }) +public class a { + public static a a() { + return null; + } +} diff --git a/jadx-core/src/test/java/jadx/tests/integration/names/pkg/b.java b/jadx-core/src/test/java/jadx/tests/integration/names/pkg/b.java new file mode 100644 index 000000000..4cf7a342b --- /dev/null +++ b/jadx-core/src/test/java/jadx/tests/integration/names/pkg/b.java @@ -0,0 +1,9 @@ +package jadx.tests.integration.names.pkg; + +@SuppressWarnings("TypeName") +public class b { + class a { + } + + private jadx.tests.integration.names.pkg.a a = jadx.tests.integration.names.pkg.a.a(); +}