From 5aa9b1ba786f2acb630f7f3b1157641cc5162acc Mon Sep 17 00:00:00 2001 From: James Braver <218421390+JamesBraver@users.noreply.github.com> Date: Thu, 14 May 2026 10:51:37 +0800 Subject: [PATCH] Support preserving original package when remapping illegal names - Add `mapClass(info, mapPackage)` to `NameGenerator` to control package remapping. - Implement package-aware naming in `AlphabetNameGenerator` and `IncrementingNameGenerator`. - Introduce `shouldMapPackage` in `NameGeneratorFilter` and all filter implementations to decide whether a class's package should be remapped. - Update `MappingGenerator` and `IllegalNameMappingTransformer` to use the new package mapping logic. - Fix argument order in `getMappedFieldName`/`getMappedMethodName` calls within `IllegalNameMappingTransformer`. - Add tests verifying that illegal class names are remapped while keeping legal packages intact. --- .../IllegalNameMappingTransformer.java | 6 +-- .../mapping/gen/MappingGenerator.java | 2 +- .../gen/filter/IncludeClassesFilter.java | 5 +++ .../gen/filter/IncludeKeywordNameFilter.java | 8 ++++ .../gen/filter/IncludeLongNameFilter.java | 8 ++++ .../filter/IncludeModifiersNameFilter.java | 5 +++ .../mapping/gen/filter/IncludeNameFilter.java | 5 +++ .../gen/filter/IncludeNonAsciiNameFilter.java | 8 ++++ .../IncludeNonJavaIdentifierNameFilter.java | 8 ++++ .../filter/IncludeWhitespaceNameFilter.java | 8 ++++ .../gen/filter/NameGeneratorFilter.java | 11 ++++++ .../gen/naming/AlphabetNameGenerator.java | 26 +++++++++++-- .../gen/naming/IncrementingNameGenerator.java | 18 +++++++-- .../mapping/gen/naming/NameGenerator.java | 21 ++++++++++ .../mapping/gen/MappingGeneratorTest.java | 39 +++++++++++++++++++ 15 files changed, 166 insertions(+), 12 deletions(-) diff --git a/recaf-core/src/main/java/software/coley/recaf/services/deobfuscation/transform/generic/IllegalNameMappingTransformer.java b/recaf-core/src/main/java/software/coley/recaf/services/deobfuscation/transform/generic/IllegalNameMappingTransformer.java index f22fa4634..324e1bd5b 100644 --- a/recaf-core/src/main/java/software/coley/recaf/services/deobfuscation/transform/generic/IllegalNameMappingTransformer.java +++ b/recaf-core/src/main/java/software/coley/recaf/services/deobfuscation/transform/generic/IllegalNameMappingTransformer.java @@ -40,17 +40,17 @@ public void transform(@Nonnull JvmTransformerContext context, @Nonnull Workspace IntermediateMappings mappings = context.getMappings(); String ownerName = initialClassState.getName(); if (ILLEGAL_NAME_FILTER.shouldMapClass(initialClassState) && mappings.getMappedClassName(ownerName) == null) - mappings.addClass(ownerName, NAME_GENERATOR.mapClass(initialClassState)); + mappings.addClass(ownerName, NAME_GENERATOR.mapClass(initialClassState, ILLEGAL_NAME_FILTER.shouldMapPackage(initialClassState))); for (FieldMember field : initialClassState.getFields()) { String fieldDesc = field.getDescriptor(); String fieldName = field.getName(); - if (ILLEGAL_NAME_FILTER.shouldMapField(initialClassState, field) && mappings.getMappedFieldName(ownerName, fieldDesc, fieldName) == null) + if (ILLEGAL_NAME_FILTER.shouldMapField(initialClassState, field) && mappings.getMappedFieldName(ownerName, fieldName, fieldDesc) == null) mappings.addField(ownerName, fieldDesc, field.getName(), NAME_GENERATOR.mapField(initialClassState, field)); } for (MethodMember method : initialClassState.getMethods()) { String methodDesc = method.getDescriptor(); String methodName = method.getName(); - if (ILLEGAL_NAME_FILTER.shouldMapMethod(initialClassState, method) && mappings.getMappedMethodName(ownerName, methodDesc, methodName) == null) + if (ILLEGAL_NAME_FILTER.shouldMapMethod(initialClassState, method) && mappings.getMappedMethodName(ownerName, methodName, methodDesc) == null) mappings.addMethod(ownerName, methodDesc, method.getName(), NAME_GENERATOR.mapMethod(initialClassState, method)); } } diff --git a/recaf-core/src/main/java/software/coley/recaf/services/mapping/gen/MappingGenerator.java b/recaf-core/src/main/java/software/coley/recaf/services/mapping/gen/MappingGenerator.java index 917456366..4cc142fe8 100644 --- a/recaf-core/src/main/java/software/coley/recaf/services/mapping/gen/MappingGenerator.java +++ b/recaf-core/src/main/java/software/coley/recaf/services/mapping/gen/MappingGenerator.java @@ -260,7 +260,7 @@ private void generateFamilyMappings(@Nonnull MappingsAdapter mappings, @Nonnull // Add mapping. String name = vertex.getName(); - String mapped = generator.mapClass(classInfo); + String mapped = generator.mapClass(classInfo, filter.shouldMapPackage(classInfo)); mappings.addClass(name, mapped); }); } diff --git a/recaf-core/src/main/java/software/coley/recaf/services/mapping/gen/filter/IncludeClassesFilter.java b/recaf-core/src/main/java/software/coley/recaf/services/mapping/gen/filter/IncludeClassesFilter.java index b44b640f1..6d8fdfcc8 100644 --- a/recaf-core/src/main/java/software/coley/recaf/services/mapping/gen/filter/IncludeClassesFilter.java +++ b/recaf-core/src/main/java/software/coley/recaf/services/mapping/gen/filter/IncludeClassesFilter.java @@ -34,6 +34,11 @@ public boolean shouldMapClass(@Nonnull ClassInfo info) { (namePredicate.match(info.getName())); } + @Override + public boolean shouldMapPackage(@Nonnull ClassInfo info) { + return shouldMapClass(info); + } + @Override public boolean shouldMapField(@Nonnull ClassInfo owner, @Nonnull FieldMember field) { // Consider owner type, we do not want to map fields if they are outside the inclusion filter diff --git a/recaf-core/src/main/java/software/coley/recaf/services/mapping/gen/filter/IncludeKeywordNameFilter.java b/recaf-core/src/main/java/software/coley/recaf/services/mapping/gen/filter/IncludeKeywordNameFilter.java index 4ec9335db..77f8aaec8 100644 --- a/recaf-core/src/main/java/software/coley/recaf/services/mapping/gen/filter/IncludeKeywordNameFilter.java +++ b/recaf-core/src/main/java/software/coley/recaf/services/mapping/gen/filter/IncludeKeywordNameFilter.java @@ -37,6 +37,14 @@ public boolean shouldMapClass(@Nonnull ClassInfo info) { return super.shouldMapClass(info); } + @Override + public boolean shouldMapPackage(@Nonnull ClassInfo info) { + String packageName = info.getPackageName(); + if (packageName != null && containsKeyword(packageName)) + return true; + return super.shouldMapPackage(info); + } + @Override public boolean shouldMapField(@Nonnull ClassInfo owner, @Nonnull FieldMember info) { if (containsKeyword(info.getName())) diff --git a/recaf-core/src/main/java/software/coley/recaf/services/mapping/gen/filter/IncludeLongNameFilter.java b/recaf-core/src/main/java/software/coley/recaf/services/mapping/gen/filter/IncludeLongNameFilter.java index d5381df57..df0c9d115 100644 --- a/recaf-core/src/main/java/software/coley/recaf/services/mapping/gen/filter/IncludeLongNameFilter.java +++ b/recaf-core/src/main/java/software/coley/recaf/services/mapping/gen/filter/IncludeLongNameFilter.java @@ -51,6 +51,14 @@ public boolean shouldMapClass(@Nonnull ClassInfo info) { return super.shouldMapClass(info); } + @Override + public boolean shouldMapPackage(@Nonnull ClassInfo info) { + String packageName = info.getPackageName(); + if (targetClasses && packageName != null && shouldMap(packageName)) + return true; + return super.shouldMapPackage(info); + } + @Override public boolean shouldMapField(@Nonnull ClassInfo owner, @Nonnull FieldMember field) { if (targetFields && shouldMap(field)) diff --git a/recaf-core/src/main/java/software/coley/recaf/services/mapping/gen/filter/IncludeModifiersNameFilter.java b/recaf-core/src/main/java/software/coley/recaf/services/mapping/gen/filter/IncludeModifiersNameFilter.java index cca8b6ef3..ab998da64 100644 --- a/recaf-core/src/main/java/software/coley/recaf/services/mapping/gen/filter/IncludeModifiersNameFilter.java +++ b/recaf-core/src/main/java/software/coley/recaf/services/mapping/gen/filter/IncludeModifiersNameFilter.java @@ -49,6 +49,11 @@ public boolean shouldMapClass(@Nonnull ClassInfo info) { return super.shouldMapClass(info); } + @Override + public boolean shouldMapPackage(@Nonnull ClassInfo info) { + return shouldMapClass(info); + } + @Override public boolean shouldMapField(@Nonnull ClassInfo owner, @Nonnull FieldMember field) { if (targetFields && field.hasAnyModifiers(flags)) diff --git a/recaf-core/src/main/java/software/coley/recaf/services/mapping/gen/filter/IncludeNameFilter.java b/recaf-core/src/main/java/software/coley/recaf/services/mapping/gen/filter/IncludeNameFilter.java index d22d6a14e..6e375b205 100644 --- a/recaf-core/src/main/java/software/coley/recaf/services/mapping/gen/filter/IncludeNameFilter.java +++ b/recaf-core/src/main/java/software/coley/recaf/services/mapping/gen/filter/IncludeNameFilter.java @@ -53,6 +53,11 @@ public boolean shouldMapClass(@Nonnull ClassInfo info) { return super.shouldMapClass(info); } + @Override + public boolean shouldMapPackage(@Nonnull ClassInfo info) { + return shouldMapClass(info); + } + @Override public boolean shouldMapField(@Nonnull ClassInfo owner, @Nonnull FieldMember field) { if (fieldPredicate != null && fieldPredicate.match(field.getName())) diff --git a/recaf-core/src/main/java/software/coley/recaf/services/mapping/gen/filter/IncludeNonAsciiNameFilter.java b/recaf-core/src/main/java/software/coley/recaf/services/mapping/gen/filter/IncludeNonAsciiNameFilter.java index ef6f364c5..0f05a3fa4 100644 --- a/recaf-core/src/main/java/software/coley/recaf/services/mapping/gen/filter/IncludeNonAsciiNameFilter.java +++ b/recaf-core/src/main/java/software/coley/recaf/services/mapping/gen/filter/IncludeNonAsciiNameFilter.java @@ -29,6 +29,14 @@ public boolean shouldMapClass(@Nonnull ClassInfo info) { return super.shouldMapClass(info); } + @Override + public boolean shouldMapPackage(@Nonnull ClassInfo info) { + String packageName = info.getPackageName(); + if (packageName != null && shouldMap(packageName)) + return true; + return super.shouldMapPackage(info); + } + @Override public boolean shouldMapField(@Nonnull ClassInfo owner, @Nonnull FieldMember field) { if (shouldMap(field)) diff --git a/recaf-core/src/main/java/software/coley/recaf/services/mapping/gen/filter/IncludeNonJavaIdentifierNameFilter.java b/recaf-core/src/main/java/software/coley/recaf/services/mapping/gen/filter/IncludeNonJavaIdentifierNameFilter.java index 39731ea4f..6925d15fd 100644 --- a/recaf-core/src/main/java/software/coley/recaf/services/mapping/gen/filter/IncludeNonJavaIdentifierNameFilter.java +++ b/recaf-core/src/main/java/software/coley/recaf/services/mapping/gen/filter/IncludeNonJavaIdentifierNameFilter.java @@ -42,6 +42,14 @@ else if (name.endsWith("module-info")) return super.shouldMapClass(info); } + @Override + public boolean shouldMapPackage(@Nonnull ClassInfo info) { + String packageName = info.getPackageName(); + if (packageName != null && isInvalidName(packageName)) + return true; + return super.shouldMapPackage(info); + } + @Override public boolean shouldMapField(@Nonnull ClassInfo owner, @Nonnull FieldMember info) { if (isInvalidName(info.getName())) diff --git a/recaf-core/src/main/java/software/coley/recaf/services/mapping/gen/filter/IncludeWhitespaceNameFilter.java b/recaf-core/src/main/java/software/coley/recaf/services/mapping/gen/filter/IncludeWhitespaceNameFilter.java index c51c39e99..95776cbd3 100644 --- a/recaf-core/src/main/java/software/coley/recaf/services/mapping/gen/filter/IncludeWhitespaceNameFilter.java +++ b/recaf-core/src/main/java/software/coley/recaf/services/mapping/gen/filter/IncludeWhitespaceNameFilter.java @@ -30,6 +30,14 @@ public boolean shouldMapClass(@Nonnull ClassInfo info) { return super.shouldMapClass(info); } + @Override + public boolean shouldMapPackage(@Nonnull ClassInfo info) { + String packageName = info.getPackageName(); + if (packageName != null && shouldMap(packageName)) + return true; + return super.shouldMapPackage(info); + } + @Override public boolean shouldMapField(@Nonnull ClassInfo owner, @Nonnull FieldMember field) { if (shouldMap(field)) diff --git a/recaf-core/src/main/java/software/coley/recaf/services/mapping/gen/filter/NameGeneratorFilter.java b/recaf-core/src/main/java/software/coley/recaf/services/mapping/gen/filter/NameGeneratorFilter.java index 51acb03e4..639ec987c 100644 --- a/recaf-core/src/main/java/software/coley/recaf/services/mapping/gen/filter/NameGeneratorFilter.java +++ b/recaf-core/src/main/java/software/coley/recaf/services/mapping/gen/filter/NameGeneratorFilter.java @@ -49,6 +49,17 @@ public boolean shouldMapClass(@Nonnull ClassInfo info) { return next != null && next.shouldMapClass(info); } + /** + * @param info Class whose package should be checked. + * @return {@code true} when a remapped class should also be moved to a different package. + */ + public boolean shouldMapPackage(@Nonnull ClassInfo info) { + if (defaultMap) + return next == null || next.shouldMapPackage(info); + else + return next != null && next.shouldMapPackage(info); + } + /** * @param owner * Class the field is defined in. diff --git a/recaf-core/src/main/java/software/coley/recaf/services/mapping/gen/naming/AlphabetNameGenerator.java b/recaf-core/src/main/java/software/coley/recaf/services/mapping/gen/naming/AlphabetNameGenerator.java index 0be7c3bf3..ca94c173c 100644 --- a/recaf-core/src/main/java/software/coley/recaf/services/mapping/gen/naming/AlphabetNameGenerator.java +++ b/recaf-core/src/main/java/software/coley/recaf/services/mapping/gen/naming/AlphabetNameGenerator.java @@ -33,11 +33,19 @@ public AlphabetNameGenerator(@Nonnull String alphabet, int length) { @Nonnull private String name(@Nullable String original) { + return name(original, null); + } + + @Nonnull + private String name(@Nullable String original, @Nullable String packageName) { int seed = original == null ? alphabet.hashCode() : original.hashCode(); - String name = StringUtil.generateName(alphabet, length, seed); + String simpleName = StringUtil.generateName(alphabet, length, seed); + String name = packageName == null ? simpleName : packageName + "/" + simpleName; if (workspace != null) { while (workspace.findClass(name) != null) - name = StringUtil.generateName(alphabet, length, seed++); + name = packageName == null ? + StringUtil.generateName(alphabet, length, ++seed) : + packageName + "/" + StringUtil.generateName(alphabet, length, ++seed); } return name; } @@ -50,11 +58,21 @@ public void setWorkspace(@Nullable Workspace workspace) { @Nonnull @Override public String mapClass(@Nonnull ClassInfo info) { + return mapClass(info, true); + } + + @Nonnull + @Override + public String mapClass(@Nonnull ClassInfo info, boolean mapPackage) { if (info.isInDefaultPackage()) return name(info.getName()); - // Ensure classes in the same package are kept together - return name(info.getPackageName()) + "/" + name(info.getName()); + if (!mapPackage) + return name(info.getName(), info.getPackageName()); + + // Ensure classes in the same package are kept together. + String mappedPackage = name(info.getPackageName()); + return name(info.getName(), mappedPackage); } @Nonnull diff --git a/recaf-core/src/main/java/software/coley/recaf/services/mapping/gen/naming/IncrementingNameGenerator.java b/recaf-core/src/main/java/software/coley/recaf/services/mapping/gen/naming/IncrementingNameGenerator.java index 37e753631..6c0b0bd86 100644 --- a/recaf-core/src/main/java/software/coley/recaf/services/mapping/gen/naming/IncrementingNameGenerator.java +++ b/recaf-core/src/main/java/software/coley/recaf/services/mapping/gen/naming/IncrementingNameGenerator.java @@ -22,8 +22,11 @@ public class IncrementingNameGenerator implements DeconflictingNameGenerator { private long varIndex = 1; @Nonnull - private String nextClassName() { - return "mapped/Class" + classIndex++; + private String nextClassName(@Nullable String packageName) { + String simpleName = "Class" + classIndex++; + if (packageName == null || packageName.isEmpty()) + return simpleName; + return packageName + "/" + simpleName; } @Nonnull @@ -49,10 +52,17 @@ public void setWorkspace(@Nullable Workspace workspace) { @Nonnull @Override public String mapClass(@Nonnull ClassInfo info) { - String name = nextClassName(); + return mapClass(info, true); + } + + @Nonnull + @Override + public String mapClass(@Nonnull ClassInfo info, boolean mapPackage) { + String packageName = mapPackage ? "mapped" : info.getPackageName(); + String name = nextClassName(packageName); if (workspace != null) { while (workspace.findClass(name) != null) - name = nextClassName(); + name = nextClassName(packageName); } return name; } diff --git a/recaf-core/src/main/java/software/coley/recaf/services/mapping/gen/naming/NameGenerator.java b/recaf-core/src/main/java/software/coley/recaf/services/mapping/gen/naming/NameGenerator.java index 58c88e00a..9b5637043 100644 --- a/recaf-core/src/main/java/software/coley/recaf/services/mapping/gen/naming/NameGenerator.java +++ b/recaf-core/src/main/java/software/coley/recaf/services/mapping/gen/naming/NameGenerator.java @@ -21,6 +21,27 @@ public interface NameGenerator { @Nonnull String mapClass(@Nonnull ClassInfo info); + /** + * @param info + * Class to rename. + * @param mapPackage + * {@code true} to allow remapping the package path. + * {@code false} to keep the class in its current package. + * + * @return New class name. + */ + @Nonnull + default String mapClass(@Nonnull ClassInfo info, boolean mapPackage) { + String mappedName = mapClass(info); + if (mapPackage || info.isInDefaultPackage()) + return mappedName; + + String packageName = info.getPackageName(); + int separatorIndex = mappedName.lastIndexOf('/'); + String simpleName = separatorIndex >= 0 ? mappedName.substring(separatorIndex + 1) : mappedName; + return packageName + "/" + simpleName; + } + /** * @param owner * Class the field is defined in. diff --git a/recaf-core/src/test/java/software/coley/recaf/services/mapping/gen/MappingGeneratorTest.java b/recaf-core/src/test/java/software/coley/recaf/services/mapping/gen/MappingGeneratorTest.java index 742db462a..1986b7fb5 100644 --- a/recaf-core/src/test/java/software/coley/recaf/services/mapping/gen/MappingGeneratorTest.java +++ b/recaf-core/src/test/java/software/coley/recaf/services/mapping/gen/MappingGeneratorTest.java @@ -32,6 +32,8 @@ import software.coley.recaf.services.mapping.gen.filter.IncludeNonJavaIdentifierNameFilter; import software.coley.recaf.services.mapping.gen.filter.IncludeWhitespaceNameFilter; import software.coley.recaf.services.mapping.gen.filter.NameGeneratorFilter; +import software.coley.recaf.services.mapping.gen.naming.AlphabetNameGenerator; +import software.coley.recaf.services.mapping.gen.naming.IncrementingNameGenerator; import software.coley.recaf.services.mapping.gen.naming.NameGenerator; import software.coley.recaf.services.search.match.StringPredicate; import software.coley.recaf.services.search.match.StringPredicateProvider; @@ -444,6 +446,10 @@ void testIncludeKeywordNameFilter() { // Edge case: 'package-info' and 'module-info' are not keywords, but they contain the keyword 'package' and 'module' respectively. assertFalse(filter.shouldMapClass(new StubClassInfo("package-info"))); assertFalse(filter.shouldMapClass(new StubClassInfo("module-info"))); + + // Only remap the package when the package path itself is the problem. + assertFalse(filter.shouldMapPackage(new StubClassInfo("com/example/class"))); + assertTrue(filter.shouldMapPackage(new StubClassInfo("com/class/Example"))); } @Test @@ -458,6 +464,8 @@ void testIncludeWhitespaceNameFilter() { assertTrue(filter.shouldMapClass(new StubClassInfo("com/_/Example".replace("_", space)))); assertTrue(filter.shouldMapClass(new StubClassInfo("_/example/Example".replace("_", space)))); assertTrue(filter.shouldMapClass(new StubClassInfo("_".replace("_", space)))); + assertFalse(filter.shouldMapPackage(new StubClassInfo("com/example/Bad_".replace("_", space)))); + assertTrue(filter.shouldMapPackage(new StubClassInfo("com/_/Example".replace("_", space)))); } } @@ -477,6 +485,10 @@ void testIncludeNonAsciiNameFilter() { // Technically in the ASCII set, but still non-ASCII in the sense of Java identifiers, should be mapped. assertTrue(filter.shouldMapClass(new StubClassInfo("\0"))); + + // Only remap the package when the package path itself is the problem. + assertFalse(filter.shouldMapPackage(new StubClassInfo("com/example/Exämple"))); + assertTrue(filter.shouldMapPackage(new StubClassInfo("cöm/example/Example"))); } @Test @@ -497,6 +509,8 @@ void testIncludeNonJavaIdentifierNameFilter() { // A package part that starts with a number is not valid, should be mapped. assertTrue(filter.shouldMapClass(new StubClassInfo("com/1A/Example"))); assertFalse(filter.shouldMapClass(new StubClassInfo("com/A1/Example"))); + assertFalse(filter.shouldMapPackage(new StubClassInfo("com/example/1Example"))); + assertTrue(filter.shouldMapPackage(new StubClassInfo("com/1A/Example"))); // Edge case: 'package-info' and 'module-info' are not valid Java identifiers, but they are exempt // from this filter since they have special meaning in Java. @@ -504,6 +518,31 @@ void testIncludeNonJavaIdentifierNameFilter() { assertFalse(filter.shouldMapClass(new StubClassInfo("module-info"))); } + @Test + void testIncrementingGeneratorPreservesLegalPackageForIllegalClassName() { + IncludeNonJavaIdentifierNameFilter filter = new IncludeNonJavaIdentifierNameFilter(null); + ClassInfo info = new StubClassInfo("pack/pack/1"); + IncrementingNameGenerator generator = new IncrementingNameGenerator(); + + assertTrue(filter.shouldMapClass(info)); + assertFalse(filter.shouldMapPackage(info)); + assertEquals("pack/pack/Class1", generator.mapClass(info, filter.shouldMapPackage(info))); + } + + @Test + void testAlphabetGeneratorPreservesLegalPackageForIllegalClassName() { + IncludeNonJavaIdentifierNameFilter filter = new IncludeNonJavaIdentifierNameFilter(null); + ClassInfo info = new StubClassInfo("pack/pack/1"); + AlphabetNameGenerator generator = new AlphabetNameGenerator("abcdefghijklmnopqrstuvwxyz", 3); + + assertTrue(filter.shouldMapClass(info)); + assertFalse(filter.shouldMapPackage(info)); + + String mapped = generator.mapClass(info, filter.shouldMapPackage(info)); + assertTrue(mapped.startsWith("pack/pack/")); + assertNotEquals(info.getName(), mapped); + } + @Test void testIncludeLongNameFilter() { // Anything > 100 characters