From 1e655c2e547dbc3ea948acbbde6acb3547764ab5 Mon Sep 17 00:00:00 2001 From: Ben Gruver Date: Tue, 16 Feb 2021 16:14:57 -0800 Subject: [PATCH] Handle unicode filename collisions on mac Some unicode characters are considered equivalent for filename purposes on mac. This adds logic to treat these collisions the same way as case sensitivy collisions are currently handled -- by adding a numeric suffix to the file/directory anytime there is a collision. See pull request #789 for more information. --- .../main/java/org/jf/baksmali/Baksmali.java | 9 +- .../org/jf/util/ClassFileNameHandler.java | 145 +++++++++--------- util/src/main/java/org/jf/util/PathUtil.java | 58 ++++++- .../org/jf/util/ClassFileNameHandlerTest.java | 72 +++++++-- 4 files changed, 197 insertions(+), 87 deletions(-) diff --git a/baksmali/src/main/java/org/jf/baksmali/Baksmali.java b/baksmali/src/main/java/org/jf/baksmali/Baksmali.java index 1c0231b5..4b017bd7 100644 --- a/baksmali/src/main/java/org/jf/baksmali/Baksmali.java +++ b/baksmali/src/main/java/org/jf/baksmali/Baksmali.java @@ -118,7 +118,14 @@ public class Baksmali { return false; } - File smaliFile = fileNameHandler.getUniqueFilenameForClass(classDescriptor); + File smaliFile = null; + try { + smaliFile = fileNameHandler.getUniqueFilenameForClass(classDescriptor); + } catch (IOException ex) { + System.err.println("\n\nError occurred while creating file for class " + classDescriptor); + ex.printStackTrace(); + return false; + } //create and initialize the top level string template ClassDefinition classDefinition = new ClassDefinition(options, classDef); diff --git a/util/src/main/java/org/jf/util/ClassFileNameHandler.java b/util/src/main/java/org/jf/util/ClassFileNameHandler.java index a223d30e..700134d4 100644 --- a/util/src/main/java/org/jf/util/ClassFileNameHandler.java +++ b/util/src/main/java/org/jf/util/ClassFileNameHandler.java @@ -33,13 +33,18 @@ import com.google.common.collect.Multimap; import javax.annotation.Nonnull; import javax.annotation.Nullable; -import java.io.*; +import java.io.File; +import java.io.IOException; +import java.io.UnsupportedEncodingException; import java.nio.ByteBuffer; -import java.nio.CharBuffer; import java.nio.IntBuffer; import java.util.Collection; +import java.util.HashMap; +import java.util.Map; import java.util.regex.Pattern; +import static org.jf.util.PathUtil.testCaseSensitivity; + /** * This class handles the complexities of translating a class name into a file name. i.e. dealing with case insensitive * file systems, windows reserved filenames, class names with extremely long package/class elements, etc. @@ -84,7 +89,7 @@ public class ClassFileNameHandler { return MAX_FILENAME_LENGTH - NUMERIC_SUFFIX_RESERVE; } - public File getUniqueFilenameForClass(String className) { + public File getUniqueFilenameForClass(String className) throws IOException { //class names should be passed in the normal dalvik style, with a leading L, a trailing ;, and using //'/' as a separator. if (className.charAt(0) != 'L' || className.charAt(className.length()-1) != ';') { @@ -129,7 +134,7 @@ public class ClassFileNameHandler { @Nonnull private File addUniqueChild(@Nonnull DirectoryEntry parent, @Nonnull String[] packageElements, - int packageElementIndex) { + int packageElementIndex) throws IOException { if (packageElementIndex == packageElements.length - 1) { FileEntry fileEntry = new FileEntry(parent, packageElements[packageElementIndex] + fileExtension); parent.addChild(fileEntry); @@ -271,7 +276,7 @@ public class ClassFileNameHandler { return physicalName; } - public void setSuffix(int suffix) { + public void setSuffix(int suffix) throws IOException { if (suffix < 0 || suffix > 99999) { throw new IllegalArgumentException("suffix must be in [0, 100000)"); } @@ -279,10 +284,18 @@ public class ClassFileNameHandler { if (this.physicalName != null) { throw new IllegalStateException("The suffix can only be set once"); } - this.physicalName = makePhysicalName(suffix); + String physicalName = getPhysicalNameWithSuffix(suffix); + File file = new File(parent.file, physicalName).getCanonicalFile(); + this.physicalName = file.getName(); + createIfNeeded(); } - protected abstract String makePhysicalName(int suffix); + /** + * Actually create the (empty) file or directory, if it doesn't exist. + */ + protected abstract void createIfNeeded() throws IOException; + + public abstract String getPhysicalNameWithSuffix(int suffix); } private class DirectoryEntry extends FileSystemEntry { @@ -290,9 +303,11 @@ public class ClassFileNameHandler { private int caseSensitivity = forcedCaseSensitivity; // maps a normalized (but not suffixed) entry name to 1 or more FileSystemEntries. - // Each FileSystemEntry asociated with a normalized entry name must have a distinct + // Each FileSystemEntry associated with a normalized entry name must have a distinct // physical name private final Multimap children = ArrayListMultimap.create(); + private final Map physicalToEntry = new HashMap<>(); + private final Map lastSuffixMap = new HashMap<>(); public DirectoryEntry(@Nonnull File path) { super(null, path.getName()); @@ -304,7 +319,7 @@ public class ClassFileNameHandler { super(parent, logicalName); } - public synchronized FileSystemEntry addChild(FileSystemEntry entry) { + public synchronized FileSystemEntry addChild(FileSystemEntry entry) throws IOException { String normalizedChildName = entry.getNormalizedName(false); Collection entries = children.get(normalizedChildName); if (entry instanceof DirectoryEntry) { @@ -314,25 +329,52 @@ public class ClassFileNameHandler { } } } - entry.setSuffix(entries.size()); + + Integer lastSuffix = lastSuffixMap.get(normalizedChildName); + if (lastSuffix == null) { + lastSuffix = -1; + } + + int suffix = lastSuffix; + while (true) { + suffix++; + + String entryPhysicalName = entry.getPhysicalNameWithSuffix(suffix); + File entryFile = new File(this.file, entryPhysicalName); + entryPhysicalName = entryFile.getCanonicalFile().getName(); + + if (!this.physicalToEntry.containsKey(entryPhysicalName)) { + entry.setSuffix(suffix); + lastSuffixMap.put(normalizedChildName, suffix); + physicalToEntry.put(entry.getPhysicalName(), entry); + break; + } + } entries.add(entry); return entry; } @Override - protected String makePhysicalName(int suffix) { + public String getPhysicalNameWithSuffix(int suffix) { if (suffix > 0) { - return getNormalizedName(true) + "." + Integer.toString(suffix); + return getNormalizedName(true) + "." + suffix; } return getNormalizedName(true); } - @Override - public void setSuffix(int suffix) { - super.setSuffix(suffix); + @Override protected void createIfNeeded() throws IOException { String physicalName = getPhysicalName(); if (parent != null && physicalName != null) { - file = new File(parent.file, physicalName); + file = new File(parent.file, physicalName).getCanonicalFile(); + + // If there are 2 non-existent files with different names that collide after filesystem + // canonicalization, getCanonicalPath() for each will return different values. But once one of the 2 + // files gets created, the other will return the same name as the one that was created. + // + // In order to detect these collisions, we need to ensure that the same value would be returned for any + // future potential filename that would end up colliding. So we have to actually create the file here, + // to force the Schrodinger filename to collapse to this particular version. + file.mkdirs(); } } @@ -366,59 +408,6 @@ public class ClassFileNameHandler { } } - private boolean testCaseSensitivity(File path) throws IOException { - int num = 1; - File f, f2; - do { - f = new File(path, "test." + num); - f2 = new File(path, "TEST." + num++); - } while(f.exists() || f2.exists()); - - try { - try { - FileWriter writer = new FileWriter(f); - writer.write("test"); - writer.flush(); - writer.close(); - } catch (IOException ex) { - try {f.delete();} catch (Exception ex2) {} - throw ex; - } - - if (f2.exists()) { - return false; - } - - if (f2.createNewFile()) { - return true; - } - - //the above 2 tests should catch almost all cases. But maybe there was a failure while creating f2 - //that isn't related to case sensitivity. Let's see if we can open the file we just created using - //f2 - try { - CharBuffer buf = CharBuffer.allocate(32); - FileReader reader = new FileReader(f2); - - while (reader.read(buf) != -1 && buf.length() < 4); - if (buf.length() == 4 && buf.toString().equals("test")) { - return false; - } else { - //we probably shouldn't get here. If the filesystem was case-sensetive, creating a new - //FileReader should have thrown a FileNotFoundException. Otherwise, we should have opened - //the file and read in the string "test". It's remotely possible that someone else modified - //the file after we created it. Let's be safe and return false here as well - assert(false); - return false; - } - } catch (FileNotFoundException ex) { - return true; - } - } finally { - try { f.delete(); } catch (Exception ex) {} - try { f2.delete(); } catch (Exception ex) {} - } - } } private class FileEntry extends FileSystemEntry { @@ -427,12 +416,28 @@ public class ClassFileNameHandler { } @Override - protected String makePhysicalName(int suffix) { + public String getPhysicalNameWithSuffix(int suffix) { if (suffix > 0) { return addSuffixBeforeExtension(getNormalizedName(true), '.' + Integer.toString(suffix)); } return getNormalizedName(true); } + + @Override protected void createIfNeeded() throws IOException { + String physicalName = getPhysicalName(); + if (parent != null && physicalName != null) { + File file = new File(parent.file, physicalName).getCanonicalFile(); + + // If there are 2 non-existent files with different names that collide after filesystem + // canonicalization, getCanonicalPath() for each will return different values. But once one of the 2 + // files gets created, the other will return the same name as the one that was created. + // + // In order to detect these collisions, we need to ensure that the same value would be returned for any + // future potential filename that would end up colliding. So we have to actually create the file here, + // to force the Schrodinger filename to collapse to this particular version. + file.createNewFile(); + } + } } private static String addSuffixBeforeExtension(String pathElement, String suffix) { diff --git a/util/src/main/java/org/jf/util/PathUtil.java b/util/src/main/java/org/jf/util/PathUtil.java index 9ba9f301..5c0bab72 100644 --- a/util/src/main/java/org/jf/util/PathUtil.java +++ b/util/src/main/java/org/jf/util/PathUtil.java @@ -30,8 +30,8 @@ package org.jf.util; import com.google.common.collect.Lists; -import java.io.File; -import java.io.IOException; +import java.io.*; +import java.nio.CharBuffer; import java.util.ArrayList; import java.util.List; @@ -115,4 +115,58 @@ public class PathUtil { return Lists.reverse(path); } + + public static boolean testCaseSensitivity(File path) throws IOException { + int num = 1; + File f, f2; + do { + f = new File(path, "test." + num); + f2 = new File(path, "TEST." + num++); + } while(f.exists() || f2.exists()); + + try { + try { + FileWriter writer = new FileWriter(f); + writer.write("test"); + writer.flush(); + writer.close(); + } catch (IOException ex) { + try {f.delete();} catch (Exception ex2) {} + throw ex; + } + + if (f2.exists()) { + return false; + } + + if (f2.createNewFile()) { + return true; + } + + //the above 2 tests should catch almost all cases. But maybe there was a failure while creating f2 + //that isn't related to case sensitivity. Let's see if we can open the file we just created using + //f2 + try { + CharBuffer buf = CharBuffer.allocate(32); + FileReader reader = new FileReader(f2); + + while (reader.read(buf) != -1 && buf.length() < 4); + if (buf.length() == 4 && buf.toString().equals("test")) { + return false; + } else { + //we probably shouldn't get here. If the filesystem was case-sensetive, creating a new + //FileReader should have thrown a FileNotFoundException. Otherwise, we should have opened + //the file and read in the string "test". It's remotely possible that someone else modified + //the file after we created it. Let's be safe and return false here as well + assert(false); + return false; + } + } catch (FileNotFoundException ex) { + return true; + } + } finally { + try { f.delete(); } catch (Exception ex) {} + try { f2.delete(); } catch (Exception ex) {} + } + } } diff --git a/util/src/test/java/org/jf/util/ClassFileNameHandlerTest.java b/util/src/test/java/org/jf/util/ClassFileNameHandlerTest.java index 125fbd2f..5232e2a4 100644 --- a/util/src/test/java/org/jf/util/ClassFileNameHandlerTest.java +++ b/util/src/test/java/org/jf/util/ClassFileNameHandlerTest.java @@ -37,6 +37,7 @@ import junit.framework.Assert; import org.junit.Test; import java.io.File; +import java.io.IOException; import java.nio.charset.Charset; public class ClassFileNameHandlerTest { @@ -115,10 +116,10 @@ public class ClassFileNameHandlerTest { } @Test - public void testMultipleLongNames() { + public void testMultipleLongNames() throws IOException { String filenameFragment = Strings.repeat("a", 512); - File tempDir = Files.createTempDir(); + File tempDir = Files.createTempDir().getCanonicalFile(); ClassFileNameHandler handler = new ClassFileNameHandler(tempDir, ".smali"); // put the differentiating character in the middle, where it will get stripped out by the filename shortening @@ -133,8 +134,8 @@ public class ClassFileNameHandlerTest { } @Test - public void testBasicFunctionality() { - File tempDir = Files.createTempDir(); + public void testBasicFunctionality() throws IOException { + File tempDir = Files.createTempDir().getCanonicalFile(); ClassFileNameHandler handler = new ClassFileNameHandler(tempDir, ".smali"); File file = handler.getUniqueFilenameForClass("La/b/c/d;"); @@ -154,8 +155,8 @@ public class ClassFileNameHandlerTest { } @Test - public void testCaseInsensitiveFilesystem() { - File tempDir = Files.createTempDir(); + public void testCaseInsensitiveFilesystem() throws IOException { + File tempDir = Files.createTempDir().getCanonicalFile(); ClassFileNameHandler handler = new ClassFileNameHandler(tempDir, ".smali", false, false); File file = handler.getUniqueFilenameForClass("La/b/c;"); @@ -169,8 +170,14 @@ public class ClassFileNameHandlerTest { } @Test - public void testCaseSensitiveFilesystem() { - File tempDir = Files.createTempDir(); + public void testCaseSensitiveFilesystem() throws IOException { + + File tempDir = Files.createTempDir().getCanonicalFile(); + if (!PathUtil.testCaseSensitivity(tempDir)) { + // Test can only be performed on case sensitive systems + return; + } + ClassFileNameHandler handler = new ClassFileNameHandler(tempDir, ".smali", true, false); File file = handler.getUniqueFilenameForClass("La/b/c;"); @@ -184,8 +191,8 @@ public class ClassFileNameHandlerTest { } @Test - public void testWindowsReservedFilenames() { - File tempDir = Files.createTempDir(); + public void testWindowsReservedFilenames() throws IOException { + File tempDir = Files.createTempDir().getCanonicalFile(); ClassFileNameHandler handler = new ClassFileNameHandler(tempDir, ".smali", false, true); File file = handler.getUniqueFilenameForClass("La/con/c;"); @@ -210,21 +217,29 @@ public class ClassFileNameHandlerTest { } @Test - public void testIgnoringWindowsReservedFilenames() { - File tempDir = Files.createTempDir(); + public void testIgnoringWindowsReservedFilenames() throws IOException { + File tempDir = Files.createTempDir().getCanonicalFile(); ClassFileNameHandler handler = new ClassFileNameHandler(tempDir, ".smali", true, false); File file = handler.getUniqueFilenameForClass("La/con/c;"); checkFilename(tempDir, file, "a", "con", "c.smali"); file = handler.getUniqueFilenameForClass("La/Con/c;"); - checkFilename(tempDir, file, "a", "Con", "c.smali"); + if (PathUtil.testCaseSensitivity(tempDir)) { + checkFilename(tempDir, file, "a", "Con", "c.smali"); + } else { + checkFilename(tempDir, file, "a", "Con.1", "c.smali"); + } file = handler.getUniqueFilenameForClass("La/b/PRN;"); checkFilename(tempDir, file, "a", "b", "PRN.smali"); file = handler.getUniqueFilenameForClass("La/b/prN;"); - checkFilename(tempDir, file, "a", "b", "prN.smali"); + if (PathUtil.testCaseSensitivity(tempDir)) { + checkFilename(tempDir, file, "a", "b", "prN.smali"); + } else { + checkFilename(tempDir, file, "a", "b", "prN.1.smali"); + } file = handler.getUniqueFilenameForClass("La/b/com0;"); checkFilename(tempDir, file, "a", "b", "com0.smali"); @@ -235,6 +250,35 @@ public class ClassFileNameHandlerTest { } } + @Test + public void testUnicodeCollisionOnMac() throws IOException { + if (!System.getProperty("os.name").toLowerCase().contains("mac")) { + // The test is only applicable when run on a mac system + return; + } + + File tempDir = Files.createTempDir().getCanonicalFile(); + ClassFileNameHandler handler = new ClassFileNameHandler(tempDir, ".smali", true, false); + + File file = handler.getUniqueFilenameForClass("Lε;"); + checkFilename(tempDir, file, "ε.smali"); + + file = handler.getUniqueFilenameForClass("Lϵ;"); + checkFilename(tempDir, file, "ϵ.1.smali"); + + file = handler.getUniqueFilenameForClass("Lε/ε;"); + checkFilename(tempDir, file, "ε", "ε.smali"); + + file = handler.getUniqueFilenameForClass("Lε/ϵ;"); + checkFilename(tempDir, file, "ε", "ϵ.1.smali"); + + file = handler.getUniqueFilenameForClass("Lϵ/ϵ;"); + checkFilename(tempDir, file, "ϵ.1", "ϵ.smali"); + + file = handler.getUniqueFilenameForClass("Lϵ/ε;"); + checkFilename(tempDir, file, "ϵ.1", "ε.1.smali"); + } + private void checkFilename(File base, File file, String... elements) { for (int i=elements.length-1; i>=0; i--) { Assert.assertEquals(elements[i], file.getName());