From 5d064d3e507081330f96de5935bcef8366cc704e Mon Sep 17 00:00:00 2001 From: pubiqq <82187521+pubiqq@users.noreply.github.com> Date: Wed, 6 Nov 2024 18:08:35 +0300 Subject: [PATCH] feat(res): improve resource names (PR #2316) --- .../utils/android/AndroidResourcesUtils.java | 2 +- .../java/jadx/core/xmlgen/ResNameUtils.java | 121 ++++++++++++++++++ .../core/xmlgen/ResTableBinaryParser.java | 95 ++++---------- .../jadx/core/xmlgen/ResNameUtilsTest.java | 89 +++++++++++++ 4 files changed, 234 insertions(+), 73 deletions(-) create mode 100644 jadx-core/src/main/java/jadx/core/xmlgen/ResNameUtils.java create mode 100644 jadx-core/src/test/java/jadx/core/xmlgen/ResNameUtilsTest.java diff --git a/jadx-core/src/main/java/jadx/core/utils/android/AndroidResourcesUtils.java b/jadx-core/src/main/java/jadx/core/utils/android/AndroidResourcesUtils.java index 2e737f23b..a42698b06 100644 --- a/jadx-core/src/main/java/jadx/core/utils/android/AndroidResourcesUtils.java +++ b/jadx-core/src/main/java/jadx/core/utils/android/AndroidResourcesUtils.java @@ -114,7 +114,7 @@ public class AndroidResourcesUtils { } for (ResourceEntry resource : resStorage.getResources()) { String resTypeName = resource.getTypeName(); - String resName = resTypeName.equals("style") ? resource.getKeyName().replace('.', '_') : resource.getKeyName(); + String resName = resource.getKeyName().replace('.', '_'); ResClsInfo typeClsInfo = innerClsMap.computeIfAbsent( resTypeName, diff --git a/jadx-core/src/main/java/jadx/core/xmlgen/ResNameUtils.java b/jadx-core/src/main/java/jadx/core/xmlgen/ResNameUtils.java new file mode 100644 index 000000000..e0f4f7646 --- /dev/null +++ b/jadx-core/src/main/java/jadx/core/xmlgen/ResNameUtils.java @@ -0,0 +1,121 @@ +package jadx.core.xmlgen; + +import jadx.core.deobf.NameMapper; + +import static jadx.core.deobf.NameMapper.*; + +class ResNameUtils { + + private ResNameUtils() { + } + + /** + * Sanitizes the name so that it can be used as a resource name. + * By resource name is meant that: + * + *

+ * If the {@code name} is already a valid resource name, the method returns it unchanged. + * If not, the method creates a valid resource name based on {@code name}, appends the + * {@code postfix}, and returns the result. + */ + static String sanitizeAsResourceName(String name, String postfix, boolean allowNonPrintable) { + if (name.isEmpty()) { + return postfix; + } + + final StringBuilder sb = new StringBuilder(name.length() + 1); + boolean nameChanged = false; + + int cp = name.codePointAt(0); + if (isValidResourceNameStart(cp, allowNonPrintable)) { + sb.appendCodePoint(cp); + } else { + sb.append('_'); + nameChanged = true; + + if (isValidResourceNamePart(cp, allowNonPrintable)) { + sb.appendCodePoint(cp); + } + } + + for (int i = Character.charCount(cp); i < name.length(); i += Character.charCount(cp)) { + cp = name.codePointAt(i); + if (isValidResourceNamePart(cp, allowNonPrintable)) { + sb.appendCodePoint(cp); + } else { + sb.append('_'); + nameChanged = true; + } + } + + final String sanitizedName = sb.toString(); + if (NameMapper.isReserved(sanitizedName)) { + nameChanged = true; + } + + return nameChanged + ? sanitizedName + postfix + : sanitizedName; + } + + /** + * Converts the resource name to a field name of the R class. + */ + static String convertToRFieldName(String resourceName) { + return resourceName.replace('.', '_'); + } + + /** + * Determines whether the code point may be part of a resource name as the first character (aapt2 + + * R class gen). + */ + private static boolean isValidResourceNameStart(int codePoint, boolean allowNonPrintable) { + return (allowNonPrintable || isPrintableAsciiCodePoint(codePoint)) + && (isValidAapt2ResourceNameStart(codePoint) && isValidIdentifierStart(codePoint)); + } + + /** + * Determines whether the code point may be part of a resource name as other than the first + * character + * (aapt2 + R class gen). + */ + private static boolean isValidResourceNamePart(int codePoint, boolean allowNonPrintable) { + return (allowNonPrintable || isPrintableAsciiCodePoint(codePoint)) + && ((isValidAapt2ResourceNamePart(codePoint) && isValidIdentifierPart(codePoint)) || codePoint == '.'); + } + + /** + * Determines whether the code point may be part of a resource name as the first character (aapt2). + *

+ * Source: aapt2/text/Unicode.cpp#L112 + */ + private static boolean isValidAapt2ResourceNameStart(int codePoint) { + return isXidStart(codePoint) || codePoint == '_'; + } + + /** + * Determines whether the code point may be part of a resource name as other than the first + * character (aapt2). + *

+ * Source: aapt2/text/Unicode.cpp#L118 + */ + private static boolean isValidAapt2ResourceNamePart(int codePoint) { + return isXidContinue(codePoint) || codePoint == '.' || codePoint == '-'; + } + + private static boolean isXidStart(int codePoint) { + // TODO: Need to implement a full check if the code point is XID_Start. + return codePoint < 0x0370 && Character.isUnicodeIdentifierStart(codePoint); + } + + private static boolean isXidContinue(int codePoint) { + // TODO: Need to implement a full check if the code point is XID_Continue. + return codePoint < 0x0370 + && (Character.isUnicodeIdentifierPart(codePoint) && !Character.isIdentifierIgnorable(codePoint)); + } +} diff --git a/jadx-core/src/main/java/jadx/core/xmlgen/ResTableBinaryParser.java b/jadx-core/src/main/java/jadx/core/xmlgen/ResTableBinaryParser.java index e854fbb9e..6f9b32570 100644 --- a/jadx-core/src/main/java/jadx/core/xmlgen/ResTableBinaryParser.java +++ b/jadx-core/src/main/java/jadx/core/xmlgen/ResTableBinaryParser.java @@ -3,12 +3,8 @@ package jadx.core.xmlgen; import java.io.IOException; import java.io.InputStream; import java.util.ArrayList; -import java.util.HashSet; import java.util.List; import java.util.Objects; -import java.util.Set; -import java.util.regex.Matcher; -import java.util.regex.Pattern; import org.jetbrains.annotations.Nullable; import org.slf4j.Logger; @@ -17,7 +13,6 @@ import org.slf4j.LoggerFactory; import jadx.api.ICodeInfo; import jadx.api.args.ResourceNameSource; import jadx.api.plugins.utils.ZipSecurity; -import jadx.core.deobf.NameMapper; import jadx.core.dex.attributes.AFlag; import jadx.core.dex.nodes.FieldNode; import jadx.core.dex.nodes.IFieldInfoRef; @@ -33,8 +28,6 @@ import jadx.core.xmlgen.entry.ValuesParser; public class ResTableBinaryParser extends CommonBinaryParser implements IResTableParser { private static final Logger LOG = LoggerFactory.getLogger(ResTableBinaryParser.class); - private static final Pattern VALID_RES_KEY_PATTERN = Pattern.compile("[\\w\\d_]+"); - private static final class PackageChunk { private final int id; private final String name; @@ -154,7 +147,6 @@ public class ResTableBinaryParser extends CommonBinaryParser implements IResTabl if (keyStringsOffset != 0) { is.skipToPos(keyStringsOffset, "Expected keyStrings string pool"); keyStrings = parseStringPool(); - deobfKeyStrings(keyStrings); } PackageChunk pkg = new PackageChunk(id, name, typeStrings, keyStrings); @@ -193,32 +185,6 @@ public class ResTableBinaryParser extends CommonBinaryParser implements IResTabl return pkg; } - private void deobfKeyStrings(BinaryXMLStrings keyStrings) { - int keysCount = keyStrings.size(); - if (root.getArgs().isRenamePrintable()) { - for (int i = 0; i < keysCount; i++) { - String keyString = keyStrings.get(i); - if (!NameMapper.isAllCharsPrintable(keyString)) { - keyStrings.put(i, makeNewKeyName(i)); - } - } - } - if (root.getArgs().isRenameValid()) { - Set keySet = new HashSet<>(keysCount); - for (int i = 0; i < keysCount; i++) { - String keyString = keyStrings.get(i); - boolean isNew = keySet.add(keyString); - if (!isNew) { - keyStrings.put(i, makeNewKeyName(i)); - } - } - } - } - - private String makeNewKeyName(int idx) { - return String.format("jadx_deobf_0x%08x", idx); - } - @SuppressWarnings("unused") private void parseTypeSpecChunk(long chunkStart) throws IOException { is.checkInt16(0x0010, "Unexpected type spec header size"); @@ -429,7 +395,7 @@ public class ResTableBinaryParser extends CommonBinaryParser implements IResTabl if (useRawResName) { newResEntry = new ResourceEntry(resRef, pkg.getName(), typeName, origKeyName, config); } else { - String resName = getResName(typeName, resRef, origKeyName); + String resName = getResName(resRef, origKeyName); newResEntry = new ResourceEntry(resRef, pkg.getName(), typeName, resName, config); ResourceEntry prevResEntry = resStorage.searchEntryWithSameName(newResEntry); if (prevResEntry != null) { @@ -449,7 +415,7 @@ public class ResTableBinaryParser extends CommonBinaryParser implements IResTabl return newResEntry; } - private String getResName(String typeName, int resRef, String origKeyName) { + private String getResName(int resRef, String origKeyName) { if (this.useRawResName) { return origKeyName; } @@ -457,40 +423,38 @@ public class ResTableBinaryParser extends CommonBinaryParser implements IResTabl if (renamedKey != null) { return renamedKey; } - // styles might contain dots in name, search for alias only for resources names - if (typeName.equals("style")) { - return origKeyName; - } + IFieldInfoRef fldRef = root.getConstValues().getGlobalConstFields().get(resRef); FieldNode constField = fldRef instanceof FieldNode ? (FieldNode) fldRef : null; - String resAlias = getResAlias(resRef, origKeyName, constField); - resStorage.addRename(resRef, resAlias); + + String newResName = getNewResName(resRef, origKeyName, constField); + if (!origKeyName.equals(newResName)) { + resStorage.addRename(resRef, newResName); + } + if (constField != null) { - constField.rename(resAlias); + final String newFieldName = ResNameUtils.convertToRFieldName(newResName); + constField.rename(newFieldName); constField.add(AFlag.DONT_RENAME); } - return resAlias; + + return newResName; } - private String getResAlias(int resRef, String origKeyName, @Nullable FieldNode constField) { - String name; + private String getNewResName(int resRef, String origKeyName, @Nullable FieldNode constField) { + String newResName; if (constField == null || constField.getTopParentClass().isSynthetic()) { - name = origKeyName; + newResName = origKeyName; } else { - name = getBetterName(root.getArgs().getResourceNameSource(), origKeyName, constField.getName()); + newResName = getBetterName(root.getArgs().getResourceNameSource(), origKeyName, constField.getName()); } - Matcher matcher = VALID_RES_KEY_PATTERN.matcher(name); - if (matcher.matches()) { - return name; + + if (root.getArgs().isRenameValid()) { + final boolean allowNonPrintable = !root.getArgs().isRenamePrintable(); + newResName = ResNameUtils.sanitizeAsResourceName(newResName, String.format("_res_0x%08x", resRef), allowNonPrintable); } - // Making sure origKeyName compliant with resource file name rules - String cleanedResName = cleanName(matcher); - String newResName = String.format("res_0x%08x", resRef); - if (cleanedResName.isEmpty()) { - return newResName; - } - // autogenerate key name, appended with cleaned origKeyName to be human-friendly - return newResName + "_" + cleanedResName.toLowerCase(); + + return newResName; } public static String getBetterName(ResourceNameSource nameSource, String resName, String codeName) { @@ -507,19 +471,6 @@ public class ResTableBinaryParser extends CommonBinaryParser implements IResTabl } } - private String cleanName(Matcher matcher) { - StringBuilder sb = new StringBuilder(); - boolean first = true; - while (matcher.find()) { - if (!first) { - sb.append("_"); - } - sb.append(matcher.group()); - first = false; - } - return sb.toString(); - } - private RawNamedValue parseValueMap() throws IOException { int nameRef = is.readInt32(); return new RawNamedValue(nameRef, parseValue()); diff --git a/jadx-core/src/test/java/jadx/core/xmlgen/ResNameUtilsTest.java b/jadx-core/src/test/java/jadx/core/xmlgen/ResNameUtilsTest.java new file mode 100644 index 000000000..06802f078 --- /dev/null +++ b/jadx-core/src/test/java/jadx/core/xmlgen/ResNameUtilsTest.java @@ -0,0 +1,89 @@ +package jadx.core.xmlgen; + +import java.util.stream.Stream; + +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +import static org.assertj.core.api.Assertions.assertThat; + +class ResNameUtilsTest { + + @DisplayName("Check sanitizeAsResourceName(name, postfix, allowNonPrintable)") + @ParameterizedTest(name = "({0}, {1}, {2}) -> {3}") + @MethodSource("provideArgsForSanitizeAsResourceNameTest") + void testSanitizeAsResourceName(String name, String postfix, boolean allowNonPrintable, String expectedResult) { + assertThat(ResNameUtils.sanitizeAsResourceName(name, postfix, allowNonPrintable)).isEqualTo(expectedResult); + } + + @DisplayName("Check convertToRFieldName(resourceName)") + @ParameterizedTest(name = "{0} -> {1}") + @MethodSource("provideArgsForConvertToRFieldNameTest") + void testConvertToRFieldName(String resourceName, String expectedResult) { + assertThat(ResNameUtils.convertToRFieldName(resourceName)).isEqualTo(expectedResult); + } + + private static Stream provideArgsForSanitizeAsResourceNameTest() { + return Stream.of( + Arguments.of("name", "_postfix", false, "name"), + + Arguments.of("/name", "_postfix", true, "_name_postfix"), + Arguments.of("na/me", "_postfix", true, "na_me_postfix"), + Arguments.of("name/", "_postfix", true, "name__postfix"), + + Arguments.of("$name", "_postfix", true, "_name_postfix"), + Arguments.of("na$me", "_postfix", true, "na_me_postfix"), + Arguments.of("name$", "_postfix", true, "name__postfix"), + + Arguments.of(".name", "_postfix", true, "_.name_postfix"), + Arguments.of("na.me", "_postfix", true, "na.me"), + Arguments.of("name.", "_postfix", true, "name."), + + Arguments.of("0name", "_postfix", true, "_0name_postfix"), + Arguments.of("na0me", "_postfix", true, "na0me"), + Arguments.of("name0", "_postfix", true, "name0"), + + Arguments.of("-name", "_postfix", true, "_name_postfix"), + Arguments.of("na-me", "_postfix", true, "na_me_postfix"), + Arguments.of("name-", "_postfix", true, "name__postfix"), + + Arguments.of("Ĉname", "_postfix", false, "_name_postfix"), + Arguments.of("naĈme", "_postfix", false, "na_me_postfix"), + Arguments.of("nameĈ", "_postfix", false, "name__postfix"), + + Arguments.of("Ĉname", "_postfix", true, "Ĉname"), + Arguments.of("naĈme", "_postfix", true, "naĈme"), + Arguments.of("nameĈ", "_postfix", true, "nameĈ"), + + // Uncomment this when XID_Start and XID_Continue characters are correctly determined. + // Arguments.of("Жname", "_postfix", true, "Жname"), + // Arguments.of("naЖme", "_postfix", true, "naЖme"), + // Arguments.of("nameЖ", "_postfix", true, "nameЖ"), + // + // Arguments.of("€name", "_postfix", true, "_name_postfix"), + // Arguments.of("na€me", "_postfix", true, "na_me_postfix"), + // Arguments.of("name€", "_postfix", true, "name__postfix"), + + Arguments.of("", "_postfix", true, "_postfix"), + + Arguments.of("if", "_postfix", true, "if_postfix"), + Arguments.of("default", "_postfix", true, "default_postfix"), + Arguments.of("true", "_postfix", true, "true_postfix"), + Arguments.of("_", "_postfix", true, "__postfix")); + } + + private static Stream provideArgsForConvertToRFieldNameTest() { + return Stream.of( + Arguments.of("ThemeDesign", "ThemeDesign"), + Arguments.of("Theme.Design", "Theme_Design"), + + Arguments.of("Ĉ_ThemeDesign_Ĉ", "Ĉ_ThemeDesign_Ĉ"), + Arguments.of("Ĉ_Theme.Design_Ĉ", "Ĉ_Theme_Design_Ĉ"), + + // The function must return a plausible result even though the resource name is invalid. + Arguments.of("/_ThemeDesign_/", "/_ThemeDesign_/"), + Arguments.of("/_Theme.Design_/", "/_Theme_Design_/")); + } +}