feat(res): improve resource names (PR #2316)

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,

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:
+	 * <ul>
+	 * <li>It can be used by aapt2 as a resource entry name.
+	 * <li>It can be converted to a valid R class field name.
+	 * </ul>
+	 * <p>
+	 * 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).
+	 * <p>
+	 * Source: <a href=
+	 * "https://cs.android.com/android/platform/superproject/+/android15-release:frameworks/base/tools/aapt2/text/Unicode.cpp;l=112">aapt2/text/Unicode.cpp#L112</a>
+	 */
+	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).
+	 * <p>
+	 * Source: <a href=
+	 * "https://cs.android.com/android/platform/superproject/+/android15-release:frameworks/base/tools/aapt2/text/Unicode.cpp;l=118">aapt2/text/Unicode.cpp#L118</a>
+	 */
+	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));
+	}
+}

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<String> 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());

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<Arguments> 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<Arguments> 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_/"));
+	}
+}