From afe703cd033ef168ee606458e0edd3c1c4a84518 Mon Sep 17 00:00:00 2001 From: 2xsaiko Date: Fri, 2 Oct 2020 22:07:23 +0200 Subject: Revert "Merge pull request #299 from thiakil/validation-changes" This reverts commit 112a49dccb1fe7792366112bc829352462cd298c, reversing changes made to b8f06abafc47065f980a94c4ddf7be70cee83411. --- enigma/build.gradle | 2 +- .../translation/mapping/MappingValidator.java | 116 +++++---------------- .../translation/representation/AccessFlags.java | 21 ---- .../java/cuchaz/enigma/TestAllowableClashes.java | 51 --------- .../cuchaz/enigma/ValidationContextMatcher.java | 35 ------- .../cuchaz/enigma/inputs/visibility/ClassA.java | 28 ----- .../cuchaz/enigma/inputs/visibility/ClassB.java | 31 ------ 7 files changed, 27 insertions(+), 257 deletions(-) delete mode 100644 enigma/src/test/java/cuchaz/enigma/TestAllowableClashes.java delete mode 100644 enigma/src/test/java/cuchaz/enigma/ValidationContextMatcher.java delete mode 100644 enigma/src/test/java/cuchaz/enigma/inputs/visibility/ClassA.java delete mode 100644 enigma/src/test/java/cuchaz/enigma/inputs/visibility/ClassB.java diff --git a/enigma/build.gradle b/enigma/build.gradle index 2c5d329..7cebcd1 100644 --- a/enigma/build.gradle +++ b/enigma/build.gradle @@ -53,7 +53,7 @@ file('src/test/java/cuchaz/enigma/inputs').listFiles().each { theFile -> args '@src/test/resources/proguard-test.conf', '-injars', file('build/test-inputs/' + "${theFile.name}.jar"), '-libraryjars', libraryJarsArg, - '-outjars', file('build/test-obf/' + "${theFile.name}.jar"), '-printmapping', file("build/${theFile.name}-mapping.txt") + '-outjars', file('build/test-obf/' + "${theFile.name}.jar") } test.dependsOn "${theFile.name}TestObf" diff --git a/enigma/src/main/java/cuchaz/enigma/translation/mapping/MappingValidator.java b/enigma/src/main/java/cuchaz/enigma/translation/mapping/MappingValidator.java index a84b0fb..f9f3b88 100644 --- a/enigma/src/main/java/cuchaz/enigma/translation/mapping/MappingValidator.java +++ b/enigma/src/main/java/cuchaz/enigma/translation/mapping/MappingValidator.java @@ -1,19 +1,15 @@ package cuchaz.enigma.translation.mapping; import java.util.Collection; -import java.util.Objects; -import java.util.Set; +import java.util.HashSet; import java.util.stream.Collectors; import cuchaz.enigma.analysis.index.InheritanceIndex; import cuchaz.enigma.analysis.index.JarIndex; import cuchaz.enigma.translation.Translator; import cuchaz.enigma.translation.mapping.tree.EntryTree; -import cuchaz.enigma.translation.representation.AccessFlags; import cuchaz.enigma.translation.representation.entry.ClassEntry; -import cuchaz.enigma.translation.representation.entry.DefEntry; import cuchaz.enigma.translation.representation.entry.Entry; -import cuchaz.enigma.translation.representation.entry.FieldEntry; import cuchaz.enigma.utils.validation.Message; import cuchaz.enigma.utils.validation.ValidationContext; @@ -39,16 +35,18 @@ public class MappingValidator { private void validateUnique(ValidationContext vc, Entry entry, String name) { ClassEntry containingClass = entry.getContainingClass(); - InheritanceIndex inheritanceIndex = index.getInheritanceIndex(); + Collection relatedClasses = getRelatedClasses(containingClass); + + for (ClassEntry relatedClass : relatedClasses) { + Entry relatedEntry = entry.replaceAncestor(containingClass, relatedClass); + Entry translatedEntry = deobfuscator.translate(relatedEntry); + + Collection> translatedSiblings = obfToDeobf.getSiblings(relatedEntry).stream() + .map(deobfuscator::translate) + .collect(Collectors.toList()); - //Step 1, check it's unique within its own siblings - Collection> directTranslatedSiblings = obfToDeobf.getSiblings(entry).stream() - .map(deobfuscator::translate) - .collect(Collectors.toList()); - for (Entry sibling : directTranslatedSiblings) { - if (entry.canConflictWith(sibling) && sibling.getName().equals(name) && !isSynthetic(entry) && !isSynthetic(sibling)) { - // allow clash if one is synthetic and the other is not - Entry parent = entry.getParent(); + if (!isUnique(translatedEntry, translatedSiblings, name)) { + Entry parent = translatedEntry.getParent(); if (parent != null) { vc.raise(Message.NONUNIQUE_NAME_CLASS, name, parent); } else { @@ -56,88 +54,26 @@ public class MappingValidator { } } } - - //Step 2, check ancestors, ignoring members invisible to children - Set ancestors = inheritanceIndex.getAncestors(containingClass); - for (ClassEntry ancestor : ancestors) { - Entry reparentedEntry = entry.replaceAncestor(containingClass, ancestor); - Entry translatedEntry = Objects.requireNonNull(deobfuscator.translate(reparentedEntry), "Translation failed"); - Collection> translatedSiblings = obfToDeobf.getSiblings(reparentedEntry).stream() - .filter(it->!entry.equals(it))//e.g. for root classes, ensure we dont match the name against itself - .filter(this::isVisibleToChildren) - .collect(Collectors.toList()); - for (Entry parentSibling : translatedSiblings) { - Entry parentSiblingTranslated = Objects.requireNonNull(deobfuscator.translate(parentSibling), "Translation failed"); - if (translatedEntry.canConflictWith(parentSiblingTranslated) && parentSiblingTranslated.getName().equals(name) && !isAcceptableOverride(parentSibling, entry)) { - Entry parent = translatedEntry.getParent(); - if (parent != null) { - vc.raise(Message.NONUNIQUE_NAME_CLASS, name, parent); - } else { - vc.raise(Message.NONUNIQUE_NAME, name); - } - } - } - } - - //Step 3, if this entry is visible to children, see if it clashes with any of their names - if (isVisibleToChildren(entry)) { - Collection children = inheritanceIndex.getDescendants(containingClass); - for (ClassEntry child : children) { - Entry reparentedEntry = entry.replaceAncestor(containingClass, child); - Entry translatedEntry = Objects.requireNonNull(deobfuscator.translate(reparentedEntry), "Translation failed"); - Collection> siblings = obfToDeobf.getSiblings(reparentedEntry).stream() - .filter(it->!entry.equals(it))//e.g. for root classes, ensure we dont match the name against itself - .collect(Collectors.toList()); - for (Entry childSibling : siblings) { - Entry childSiblingTranslated = Objects.requireNonNull(deobfuscator.translate(childSibling), "Translation failed"); - if (translatedEntry.canConflictWith(childSiblingTranslated) && childSiblingTranslated.getName().equals(name) && !isAcceptableOverride(entry, childSibling)) { - Entry parent = translatedEntry.getParent(); - if (parent != null) { - vc.raise(Message.NONUNIQUE_NAME_CLASS, name, parent); - } else { - vc.raise(Message.NONUNIQUE_NAME, name); - } - } - } - } - } } - private boolean isVisibleToChildren(Entry entry) { - if (entry instanceof DefEntry) { - return !((DefEntry) entry).getAccess().isPrivate(); - } - AccessFlags accessFlags = index.getEntryIndex().getEntryAccess(entry); - if (accessFlags != null) { - return !accessFlags.isPrivate(); - } - return true;//unknown, assume yes - } - - private boolean isAcceptableOverride(Entry ancestor, Entry descendent) { - if (ancestor instanceof FieldEntry && descendent instanceof FieldEntry){ - return true;//fields don't apply here - } - - AccessFlags ancestorFlags = findAccessFlags(ancestor); - AccessFlags descendentFlags = findAccessFlags(descendent); - - if (ancestorFlags == null || descendentFlags == null) { - return false;//we can't make any assumptions - } + private Collection getRelatedClasses(ClassEntry classEntry) { + InheritanceIndex inheritanceIndex = index.getInheritanceIndex(); - //bad == accessLevel < superAccessLevel - return !(descendentFlags.getAccessLevel() < ancestorFlags.getAccessLevel()); - } + Collection relatedClasses = new HashSet<>(); + relatedClasses.add(classEntry); + relatedClasses.addAll(inheritanceIndex.getChildren(classEntry)); + relatedClasses.addAll(inheritanceIndex.getAncestors(classEntry)); - private boolean isSynthetic(Entry entry) { - AccessFlags accessFlags = findAccessFlags(entry); - return accessFlags != null && accessFlags.isSynthetic(); + return relatedClasses; } - private AccessFlags findAccessFlags(Entry entry) { - return (entry instanceof DefEntry) ? ((DefEntry) entry).getAccess() : index.getEntryIndex() - .getEntryAccess(entry); + private boolean isUnique(Entry entry, Collection> siblings, String name) { + for (Entry sibling : siblings) { + if (entry.canConflictWith(sibling) && sibling.getName().equals(name)) { + return false; + } + } + return true; } } diff --git a/enigma/src/main/java/cuchaz/enigma/translation/representation/AccessFlags.java b/enigma/src/main/java/cuchaz/enigma/translation/representation/AccessFlags.java index aa48a5b..b280eef 100644 --- a/enigma/src/main/java/cuchaz/enigma/translation/representation/AccessFlags.java +++ b/enigma/src/main/java/cuchaz/enigma/translation/representation/AccessFlags.java @@ -8,10 +8,6 @@ import java.lang.reflect.Modifier; public class AccessFlags { public static final AccessFlags PRIVATE = new AccessFlags(Opcodes.ACC_PRIVATE); public static final AccessFlags PUBLIC = new AccessFlags(Opcodes.ACC_PUBLIC); - public static final int ACCESS_LEVEL_PUBLIC = 4; - public static final int ACCESS_LEVEL_PROTECTED = 3; - public static final int ACCESS_LEVEL_PACKAGE_LOCAL = 2; - public static final int ACCESS_LEVEL_PRIVATE = 1; private int flags; @@ -93,23 +89,6 @@ public class AccessFlags { return this.flags; } - /** - * Adapted from https://github.com/JetBrains/intellij-community/blob/6472c347db91d11bbf02895a767198f9d884b119/java/java-psi-api/src/com/intellij/psi/util/PsiUtil.java#L389 - * @return visibility access level on a 'weakness scale' - */ - public int getAccessLevel() { - if (isPrivate()) { - return ACCESS_LEVEL_PRIVATE; - } - if (isProtected()) { - return ACCESS_LEVEL_PROTECTED; - } - if (isPublic()) { - return ACCESS_LEVEL_PUBLIC; - } - return ACCESS_LEVEL_PACKAGE_LOCAL; - } - @Override public boolean equals(Object obj) { return obj instanceof AccessFlags && ((AccessFlags) obj).flags == flags; diff --git a/enigma/src/test/java/cuchaz/enigma/TestAllowableClashes.java b/enigma/src/test/java/cuchaz/enigma/TestAllowableClashes.java deleted file mode 100644 index ce82e38..0000000 --- a/enigma/src/test/java/cuchaz/enigma/TestAllowableClashes.java +++ /dev/null @@ -1,51 +0,0 @@ -package cuchaz.enigma; - -import java.io.IOException; -import java.nio.file.Paths; - -import org.hamcrest.MatcherAssert; -import org.junit.Assert; -import org.junit.Test; - -import cuchaz.enigma.classprovider.ClasspathClassProvider; -import cuchaz.enigma.translation.mapping.EntryMapping; -import cuchaz.enigma.translation.mapping.EntryRemapper; -import cuchaz.enigma.translation.mapping.serde.MappingFormat; -import cuchaz.enigma.translation.mapping.serde.MappingParseException; -import cuchaz.enigma.translation.mapping.tree.EntryTree; -import cuchaz.enigma.translation.mapping.tree.EntryTreeNode; -import cuchaz.enigma.translation.representation.entry.MethodEntry; -import cuchaz.enigma.utils.validation.ValidationContext; - -/** - * Test that we can accept some name clashes that are allowed by javac - */ -public class TestAllowableClashes { - - @Test - public void test() throws IOException, MappingParseException { - //Load produced mappings - Enigma enigma = Enigma.create(); - EnigmaProject project = enigma.openJar(Paths.get("build/test-obf/visibility.jar"), new ClasspathClassProvider(), ProgressListener.none()); - EntryTree obfToDeobf = MappingFormat.PROGUARD.read(Paths.get("build/visibility-mapping.txt"), ProgressListener.none(), null); - - //Load them into enigma, none should conflict - EntryRemapper mapper = project.getMapper(); - for (int round=0; round<2; round++) { - for (EntryTreeNode node : obfToDeobf) { - Assert.assertNotEquals(null, node.getValue()); - if (node.getEntry() instanceof MethodEntry && (node.getEntry() - .getName() - .equals("") || node.getEntry().getName().equals(""))) { - //skip proguard's constructor entries - continue; - } - System.out.println(node.getEntry().toString() + " -> " + node.getValue().getTargetName()); - ValidationContext vc = new ValidationContext(); - mapper.mapFromObf(vc, node.getEntry(), node.getValue()); - MatcherAssert.assertThat(vc, ValidationContextMatcher.INSTANCE); - } - } - } - -} diff --git a/enigma/src/test/java/cuchaz/enigma/ValidationContextMatcher.java b/enigma/src/test/java/cuchaz/enigma/ValidationContextMatcher.java deleted file mode 100644 index 5404077..0000000 --- a/enigma/src/test/java/cuchaz/enigma/ValidationContextMatcher.java +++ /dev/null @@ -1,35 +0,0 @@ -package cuchaz.enigma; - -import cuchaz.enigma.utils.validation.ParameterizedMessage; -import cuchaz.enigma.utils.validation.ValidationContext; -import org.hamcrest.CustomMatcher; -import org.hamcrest.Description; - -class ValidationContextMatcher extends CustomMatcher { - public static final ValidationContextMatcher INSTANCE = new ValidationContextMatcher(); - - private ValidationContextMatcher() { - super("ValidationContext can proceed"); - } - - @Override - public boolean matches(Object item) { - return item instanceof ValidationContext && ((ValidationContext) item).canProceed(); - } - - @Override - public void describeMismatch(Object item, Description description) { - if (!(item instanceof ValidationContext)) { - description.appendText("expected ValidationContext, was").appendValue(item); - return; - } - ValidationContext vc = (ValidationContext) item; - for (ParameterizedMessage message : vc.getMessages()) { - description.appendText(message.getText()); - String longMessage = message.getLongText(); - if (longMessage != null && !longMessage.trim().isEmpty()){ - description.appendText(longMessage); - } - } - } -} diff --git a/enigma/src/test/java/cuchaz/enigma/inputs/visibility/ClassA.java b/enigma/src/test/java/cuchaz/enigma/inputs/visibility/ClassA.java deleted file mode 100644 index 458f7c0..0000000 --- a/enigma/src/test/java/cuchaz/enigma/inputs/visibility/ClassA.java +++ /dev/null @@ -1,28 +0,0 @@ -package cuchaz.enigma.inputs.visibility; - -class ClassA { - - protected Object protectedParentPrivateChild; - public Object publicParentPrivateChild; - - public static Object LOGGER = null; - - protected static Object LOGGER2 = null; - - public Object publicPublic; - - public static void equalAccessStatic() {} - - protected static void protectedPublicStatic(){} - - private static void privateStaticParentPublicStaticChild(){} - - private void privateParentPublicStaticChild() {} - - static void packagePrivateParentProtectedChild(){} - - private static void packagePrivateChild(){} - - public ClassA returningSubclass(){return null;} - -} \ No newline at end of file diff --git a/enigma/src/test/java/cuchaz/enigma/inputs/visibility/ClassB.java b/enigma/src/test/java/cuchaz/enigma/inputs/visibility/ClassB.java deleted file mode 100644 index 2a1b68a..0000000 --- a/enigma/src/test/java/cuchaz/enigma/inputs/visibility/ClassB.java +++ /dev/null @@ -1,31 +0,0 @@ -package cuchaz.enigma.inputs.visibility; - -public class ClassB extends ClassA { - private Object protectedParentPrivateChild; - - private Object publicParentPrivateChild; - - public Object publicPublic; - - public static Object LOGGER; - - public static Object LOGGER2 = null; - - public static void equalAccessStatic() { - } - - public static void protectedPublicStatic() { - } - - public static void privateStaticParentPublicStaticChild() { - } - - public static void privateParentPublicStaticChild() { - } - - protected static void packagePrivateParentProtectedChild(){} - - static void packagePrivateChild(){} - - public ClassB returningSubclass(){return null;} -} -- cgit v1.2.3