diff options
3 files changed, 31 insertions, 30 deletions
diff --git a/tzdata/update/src/main/libcore/tzdata/update/ConfigBundle.java b/tzdata/update/src/main/libcore/tzdata/update/ConfigBundle.java index 6e2ff9d..b497c85 100644 --- a/tzdata/update/src/main/libcore/tzdata/update/ConfigBundle.java +++ b/tzdata/update/src/main/libcore/tzdata/update/ConfigBundle.java @@ -72,6 +72,8 @@ public final class ConfigBundle { // Validate the entry name: make sure the unpacked file will exist beneath the // targetDir. String name = entry.getName(); + // Note, we assume that nothing will quickly insert a symlink after createSubFile() + // that might invalidate the guarantees about name existing beneath targetDir. File entryFile = FileUtils.createSubFile(targetDir, name); if (entry.isDirectory()) { diff --git a/tzdata/update/src/main/libcore/tzdata/update/FileUtils.java b/tzdata/update/src/main/libcore/tzdata/update/FileUtils.java index 8b7da78..652b786 100644 --- a/tzdata/update/src/main/libcore/tzdata/update/FileUtils.java +++ b/tzdata/update/src/main/libcore/tzdata/update/FileUtils.java @@ -37,19 +37,18 @@ public final class FileUtils { /** * Creates a new {@link java.io.File} from the {@code parentDir} and {@code name}, but only if - * the - * resulting file would exist beneath {@code parentDir}. Useful if {@code name} could contain - * "/../" or symlinks. The returned object has an absolute path. + * the resulting file would exist beneath {@code parentDir}. Useful if {@code name} could + * contain "/../" or symlinks. The returned object has a canonicalized path. * - * @throws java.io.IOException - * if the file would not exist beneath {@code parentDir} + * @throws java.io.IOException if the file would not exist beneath {@code parentDir} */ public static File createSubFile(File parentDir, String name) throws IOException { // The subFile must exist beneath parentDir. If name contains "/../" this may not be the // case so we check. - File subFile = canonicalizeDirPath(new File(parentDir, name)); + File subFile = new File(parentDir, name).getCanonicalFile(); if (!subFile.getPath().startsWith(parentDir.getCanonicalPath())) { - throw new IOException(name + " must exist beneath " + parentDir); + throw new IOException(name + " must exist beneath " + parentDir + + ". Canonicalized subpath: " + subFile); } return subFile; } @@ -61,8 +60,8 @@ public final class FileUtils { * directories explicitly created will have their permissions set; existing directories are * untouched. * - * @throws IOException - * if the directory or one of its parents did not already exist and could not be created + * @throws IOException if the directory or one of its parents did not already exist and could + * not be created */ public static void ensureDirectoriesExist(File dir, boolean makeWorldReadable) throws IOException { @@ -87,14 +86,6 @@ public final class FileUtils { } } - /** - * Returns a file with all symlinks and relative paths such as "/../" resolved <em>except</em> - * for the base name (the last element of the path). Useful for detecting symlinks. - */ - public static File canonicalizeDirPath(File file) throws IOException { - return new File(file.getParentFile().getCanonicalFile(), file.getName()); - } - public static void makeDirectoryWorldAccessible(File directory) throws IOException { if (!directory.isDirectory()) { throw new IOException(directory + " must be a directory"); @@ -150,7 +141,10 @@ public final class FileUtils { } public static boolean isSymlink(File file) throws IOException { - return !file.getCanonicalPath().equals(canonicalizeDirPath(file).getPath()); + String baseName = file.getName(); + String canonicalPathExceptBaseName = + new File(file.getParentFile().getCanonicalFile(), baseName).getPath(); + return !file.getCanonicalPath().equals(canonicalPathExceptBaseName); } public static void deleteRecursive(File toDelete) throws IOException { diff --git a/tzdata/update/src/test/libcore/tzdata/update/FileUtilsTest.java b/tzdata/update/src/test/libcore/tzdata/update/FileUtilsTest.java index ce02bfe..d002820 100644 --- a/tzdata/update/src/test/libcore/tzdata/update/FileUtilsTest.java +++ b/tzdata/update/src/test/libcore/tzdata/update/FileUtilsTest.java @@ -113,21 +113,30 @@ public class FileUtilsTest extends TestCase { } public void testCreateSubFile() throws Exception { - File dir1 = createTempDir(); - File subFile = FileUtils.createSubFile(dir1, "file"); - assertFileCanonicalEquals(new File(dir1, "file"), subFile); + File dir1 = createTempDir().getCanonicalFile(); + + File actualSubFile = FileUtils.createSubFile(dir1, "file"); + assertEquals(new File(dir1, "file"), actualSubFile); + + File existingSubFile = createRegularFile(dir1, "file"); + actualSubFile = FileUtils.createSubFile(dir1, "file"); + assertEquals(existingSubFile, actualSubFile); + + File existingSubDir = createDir(dir1, "subdir"); + actualSubFile = FileUtils.createSubFile(dir1, "subdir"); + assertEquals(existingSubDir, actualSubFile); assertCreateSubFileThrows(dir1, "../file"); assertCreateSubFileThrows(dir1, "../../file"); assertCreateSubFileThrows(dir1, "../otherdir/file"); - File dir2 = createTempDir(); - File dir2Subdir = createDir(dir2, "dir2Subdir"); - File expectedSymlinkToDir2 = createSymlink(dir2Subdir, dir1, "symlinkToDir2"); + File dir2 = createTempDir().getCanonicalFile(); + createSymlink(dir2, dir1, "symlinkToDir2"); + assertCreateSubFileThrows(dir1, "symlinkToDir2"); - File actualSymlinkToDir2 = FileUtils.createSubFile(dir1, "symlinkToDir2"); - assertEquals(expectedSymlinkToDir2, actualSymlinkToDir2); + assertCreateSubFileThrows(dir1, "symlinkToDir2/fileInSymlinkedDir"); + createRegularFile(dir1, "symlinkToDir2/fileInSymlinkedDir"); assertCreateSubFileThrows(dir1, "symlinkToDir2/fileInSymlinkedDir"); } @@ -295,10 +304,6 @@ public class FileUtilsTest extends TestCase { (sb.st_mode & mask) == mask); } - private static void assertFileCanonicalEquals(File expected, File actual) throws IOException { - assertEquals(expected.getCanonicalFile(), actual.getCanonicalFile()); - } - private File createTempDir() { final String tempPrefix = getClass().getSimpleName(); File tempDir = IoUtils.createTemporaryDirectory(tempPrefix); |