Merge #548 by oscarthecat: Prevent DexFileSplitter from dropping colliding class names ## Problem `DexFileSplitter` stores input entries in a `TreeMap` using `ZipEntryComparator::compareClassNames`, then inserts them with `putIfAbsent`. The comparator replaces `$` with `0` before comparing names. When generated nested class names include entries such as `Foo$2$1$1.class.dex` and `Foo$2$101.class.dex`, both normalize to the same value. The comparator therefore returns zero, the `TreeMap` treats the distinct classes as the same key, and `putIfAbsent` silently drops one. The resulting APK can fail at runtime with `NoClassDefFoundError` for the missing class. ## Change Keep the existing dx-style normalized ordering. When two normalized names compare equal, fall back to comparing the original names so distinct entries remain distinct. No ordering changes for names whose normalized forms differ. Add regression coverage that inserts the colliding names into the same `TreeMap` shape used by `DexFileSplitter` and verifies that both remain present. ## Testing - Passed a self-contained `javac` regression check against the modified `ZipEntryComparator`. - Added the collision case to `DexFileSplitterTest`. - Attempted `bazel test //src/tools/javatests/com/google/devtools/build/android/dexer:AllTests --test_output=errors`; repository analysis could not complete locally because connections to `maven.google.com` timed out before compilation or test execution. Closes #548 COPYBARA_INTEGRATE_REVIEW=https://github.com/bazelbuild/rules_android/pull/548 from oscarthecat:fix/dex-splitter-class-name-collision 140e6c0c407250c09d6fc931c70a0360c1b1c3e5 PiperOrigin-RevId: 971560232 Change-Id: Iba7facbdb7e7c4d784560bb0b359132d5c7c552f
diff --git a/src/tools/java/com/google/devtools/build/android/dexer/ZipEntryComparator.java b/src/tools/java/com/google/devtools/build/android/dexer/ZipEntryComparator.java index e17650d..2736f86 100644 --- a/src/tools/java/com/google/devtools/build/android/dexer/ZipEntryComparator.java +++ b/src/tools/java/com/google/devtools/build/android/dexer/ZipEntryComparator.java
@@ -46,6 +46,9 @@ // https://cs.android.com/android/platform/superproject/+/android-latest-release:dalvik/dx/src/com/android/dx/cf/direct/ClassPathOpener.java;l=187-200;drc=9dbd802c8c96c3a66873bc600bc7d1374a1d08e5 @VisibleForTesting static int compareClassNames(String a, String b) { + String originalA = a; + String originalB = b; + // Ensure inner classes sort second // The strings being sorted are filenames (e.g. MyThing$1.class, MyThing.class), so this // normalization is necessary. @@ -59,6 +62,13 @@ a = a.replace("package-info", ""); b = b.replace("package-info", ""); - return a.compareTo(b); + int normalizedResult = a.compareTo(b); + if (normalizedResult != 0) { + return normalizedResult; + } + + // Normalization is lossy. Keep distinct raw names distinct when this comparator is used as a + // TreeMap key comparator, otherwise entries such as Foo$2$1$1 and Foo$2$101 collapse. + return originalA.compareTo(originalB); } }
diff --git a/src/tools/javatests/com/google/devtools/build/android/dexer/DexFileSplitterTest.java b/src/tools/javatests/com/google/devtools/build/android/dexer/DexFileSplitterTest.java index 1551df2..5784e87 100644 --- a/src/tools/javatests/com/google/devtools/build/android/dexer/DexFileSplitterTest.java +++ b/src/tools/javatests/com/google/devtools/build/android/dexer/DexFileSplitterTest.java
@@ -29,6 +29,7 @@ import java.nio.file.Path; import java.util.HashSet; import java.util.Set; +import java.util.TreeMap; import java.util.concurrent.ExecutionException; import java.util.zip.CRC32; import java.util.zip.ZipEntry; @@ -124,6 +125,19 @@ } @Test + public void testClassNameComparatorPreservesNormalizationCollisions() { + TreeMap<String, Boolean> classNames = + new TreeMap<>(ZipEntryComparator::compareClassNames); + + classNames.put("Foo$2$1$1.class.dex", true); + classNames.put("Foo$2$101.class.dex", true); + + assertThat(classNames.keySet()) + .containsExactly("Foo$2$1$1.class.dex", "Foo$2$101.class.dex") + .inOrder(); + } + + @Test public void testSingleInputMultidexOutput() throws Exception { ImmutableList<Path> outputArchives = runDexSplitter(SMALL_IDX_PER_DEX, "multidex_from_single", multidexArchive);