diff --git a/F-Droid/src/org/fdroid/fdroid/Utils.java b/F-Droid/src/org/fdroid/fdroid/Utils.java index 7358104bd..01430b396 100644 --- a/F-Droid/src/org/fdroid/fdroid/Utils.java +++ b/F-Droid/src/org/fdroid/fdroid/Utils.java @@ -29,7 +29,10 @@ import android.text.TextUtils; import android.util.DisplayMetrics; import android.util.Log; import com.nostra13.universalimageloader.utils.StorageUtils; +import org.fdroid.fdroid.compat.FileCompat; +import org.fdroid.fdroid.data.Apk; import org.fdroid.fdroid.data.Repo; +import org.fdroid.fdroid.data.SanitizedFile; import org.xml.sax.XMLReader; import org.xmlpull.v1.XmlPullParser; import org.xmlpull.v1.XmlPullParserException; @@ -115,42 +118,25 @@ public final class Utils { } /** - * use symlinks if they are available, otherwise fall back to copying + * Attempt to symlink, but if that fails, it will make a copy of the file. */ - public static boolean symlinkOrCopyFile(File inFile, File outFile) { - if (new File("/system/bin/ln").exists()) { - return symlink(inFile, outFile); - } else { - return copy(inFile, outFile); - } + public static boolean symlinkOrCopyFile(SanitizedFile inFile, SanitizedFile outFile) { + return FileCompat.symlink(inFile, outFile) || copy(inFile, outFile); } - public static boolean symlink(File inFile, File outFile) { - int exitCode = -1; + /** + * Read the input stream until it reaches the end, ignoring any exceptions. + */ + public static void consumeStream(InputStream stream) { + final byte buffer[] = new byte[256]; try { - Process sh = Runtime.getRuntime().exec("sh"); - OutputStream out = sh.getOutputStream(); - String command = "/system/bin/ln -s " + inFile + " " + outFile - + "\nexit\n"; - out.write(command.getBytes("ASCII")); - - final char buf[] = new char[40]; - InputStreamReader reader = new InputStreamReader(sh.getInputStream()); - while (reader.read(buf) != -1) - throw new IOException("stdout: " + new String(buf)); - reader = new InputStreamReader(sh.getErrorStream()); - while (reader.read(buf) != -1) - throw new IOException("stderr: " + new String(buf)); - - exitCode = sh.waitFor(); + int read; + do { + read = stream.read(buffer); + } while (read != -1); } catch (IOException e) { - e.printStackTrace(); - return false; - } catch (InterruptedException e) { - e.printStackTrace(); - return false; + // Ignore... } - return exitCode == 0; } public static boolean copy(File inFile, File outFile) { @@ -311,12 +297,21 @@ public final class Utils { return b.build(); } + /** + * The directory where .apk files are downloaded (and stored - if the relevant property is enabled). + * This must be on internal storage, to prevent other apps with "write external storage" from being + * able to change the .apk file between F-Droid requesting the Package Manger to install, and the + * Package Manager receiving that request. + */ public static File getApkCacheDir(Context context) { - File apkCacheDir = new File( - StorageUtils.getCacheDirectory(context, true), "apks"); + SanitizedFile apkCacheDir = new SanitizedFile(StorageUtils.getCacheDirectory(context, false), "apks"); if (!apkCacheDir.exists()) { apkCacheDir.mkdir(); } + + // All parent directories of the .apk file need to be executable for the package installer + // to be able to have permission to read our world-readable .apk files. + FileCompat.setExecutable(apkCacheDir, true, false); return apkCacheDir; } diff --git a/F-Droid/src/org/fdroid/fdroid/compat/FileCompat.java b/F-Droid/src/org/fdroid/fdroid/compat/FileCompat.java new file mode 100644 index 000000000..7cc6c411e --- /dev/null +++ b/F-Droid/src/org/fdroid/fdroid/compat/FileCompat.java @@ -0,0 +1,138 @@ +package org.fdroid.fdroid.compat; + +import android.annotation.TargetApi; +import android.os.Build; +import android.system.ErrnoException; +import android.util.Log; +import org.fdroid.fdroid.Utils; +import org.fdroid.fdroid.data.SanitizedFile; + +import java.io.IOException; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; + +public class FileCompat { + + private static final String TAG = "org.fdroid.fdroid.compat.FileCompat"; + + public static boolean symlink(SanitizedFile source, SanitizedFile dest) { + + if (Compatibility.hasApi(21)) { + symlinkOs(source, dest); + } else if (Compatibility.hasApi(15)) { + symlinkLibcore(source, dest); + } else { + symlinkRuntime(source, dest); + } + + return dest.exists(); + } + + /** + * Moved into a separate class rather than just a method, so that phones without API 21 will + * not attempt to load this class at runtime. Otherwise, using the Os.symlink method will cause + * a VerifyError to be thrown at runtime when the FileCompat class is first used. + */ + private static class Symlink21 { + + @TargetApi(Build.VERSION_CODES.LOLLIPOP) + public void symlink(SanitizedFile source, SanitizedFile dest) { + try { + android.system.Os.symlink(source.getAbsolutePath(), dest.getAbsolutePath()); + } catch (ErrnoException e) { + // Do nothing... + } + } + + } + + @TargetApi(21) + protected static void symlinkOs(SanitizedFile source, SanitizedFile dest) { + new Symlink21().symlink(source, dest); + } + + protected static void symlinkRuntime(SanitizedFile source, SanitizedFile dest) { + String commands[] = { + "/system/bin/ln", + source.getAbsolutePath(), + dest.getAbsolutePath() + }; + try { + Log.d(TAG, "Executing command: " + commands[0] + " " + commands[1] + " " + commands[2]); + Process proc = Runtime.getRuntime().exec(commands); + Utils.consumeStream(proc.getInputStream()); + Utils.consumeStream(proc.getErrorStream()); + } catch (IOException e) { + // Do nothing + } + } + + protected static void symlinkLibcore(SanitizedFile source, SanitizedFile dest) { + try { + Object os = Class.forName("libcore.io.Libcore").getField("os").get(null); + Method symlink = os.getClass().getMethod("symlink", String.class, String.class); + symlink.invoke(os, source.getAbsolutePath(), dest.getAbsolutePath()); + } catch (Exception e) { + // Should catch more specific exceptions than just "Exception" here, but there are + // some which come from libcore.io.Libcore, which we don't have access to at compile time. + Log.e(TAG, "Could not symlink " + source.getAbsolutePath() + " to " + dest.getAbsolutePath() + ": " + e.getMessage()); + } + } + + @TargetApi(9) + public static boolean setReadable(SanitizedFile file, boolean readable, boolean ownerOnly) { + + if (Compatibility.hasApi(9)) { + return file.setReadable(readable, ownerOnly); + } else { + String mode; + if (readable) { + mode = ownerOnly ? "0600" : "0644"; + } else { + mode = "0000"; + } + return setMode(file, mode); + } + + } + + private static boolean setMode(SanitizedFile file, String mode) { + + // The "file" must be a sanitized file, and hence only contain A-Za-z0-9.-_ already, + // but it makes no assurances about the parent directory. + String[] args = { + "/system/bin/chmod", + mode, + file.getAbsolutePath() + }; + + try { + Log.d(TAG, "Executing following command: " + args[0] + " " + args[1] + " " + args[2]); + Process proc = Runtime.getRuntime().exec(args); + Utils.consumeStream(proc.getInputStream()); + Utils.consumeStream(proc.getErrorStream()); + return true; + } catch (IOException e) { + return false; + } + + } + + @TargetApi(9) + public static boolean setExecutable(SanitizedFile file, boolean readable, boolean ownerOnly) { + + if (Compatibility.hasApi(9)) { + return file.setExecutable(readable, ownerOnly); + } else { + String mode; + if ( readable ) { + mode = ownerOnly ? "0700" : "0711"; + } else { + mode = ownerOnly ? "0600" : "0600"; + } + return setMode( file, mode ); + } + + } + +} diff --git a/F-Droid/src/org/fdroid/fdroid/data/Apk.java b/F-Droid/src/org/fdroid/fdroid/data/Apk.java index 6177f6e9a..f70166a60 100644 --- a/F-Droid/src/org/fdroid/fdroid/data/Apk.java +++ b/F-Droid/src/org/fdroid/fdroid/data/Apk.java @@ -4,7 +4,6 @@ import android.content.ContentValues; import android.database.Cursor; import org.fdroid.fdroid.Utils; -import java.io.File; import java.util.Date; public class Apk extends ValueObject implements Comparable { @@ -33,7 +32,7 @@ public class Apk extends ValueObject implements Comparable { public boolean compatible; public String apkName; // F-Droid style APK name - public File installedFile; // the .apk file on this device's filesystem + public SanitizedFile installedFile; // the .apk file on this device's filesystem // If not null, this is the name of the source tarball for the // application. Null indicates that it's a developer's binary diff --git a/F-Droid/src/org/fdroid/fdroid/data/App.java b/F-Droid/src/org/fdroid/fdroid/data/App.java index 2c8ed0fb9..61b364f5f 100644 --- a/F-Droid/src/org/fdroid/fdroid/data/App.java +++ b/F-Droid/src/org/fdroid/fdroid/data/App.java @@ -224,7 +224,7 @@ public class App extends ValueObject implements Comparable { this.name = (String) appInfo.loadLabel(pm); - File apkFile = new File(appInfo.publicSourceDir); + SanitizedFile apkFile = SanitizedFile.knownSanitized(appInfo.publicSourceDir); Apk apk = new Apk(); apk.version = packageInfo.versionName; apk.vercode = packageInfo.versionCode; diff --git a/F-Droid/src/org/fdroid/fdroid/data/SanitizedFile.java b/F-Droid/src/org/fdroid/fdroid/data/SanitizedFile.java new file mode 100644 index 000000000..cdf4e8981 --- /dev/null +++ b/F-Droid/src/org/fdroid/fdroid/data/SanitizedFile.java @@ -0,0 +1,63 @@ +package org.fdroid.fdroid.data; + +import java.io.File; + +/** + * File guaranteed to have a santitized name (though not a sanitized path to the parent dir). + * Useful so that we can use Java's type system to enforce that the file we are accessing + * doesn't contain illegal characters. + * Sanitized names are those which only have the following characters: [A-Za-z0-9.-_] + */ +public class SanitizedFile extends File { + + /** + * The "name" argument is assumed to be a file name, _not including any path separators_. + * If it is a relative path to be appended to "parent", such as "/blah/sneh.txt", then + * the forward slashes will be removed and it will be assumed you meant "blahsneh.txt". + */ + public SanitizedFile(File parent, String name) { + super(parent, name.replaceAll("[^A-Za-z0-9-._]", "")); + } + + /** + * Used by the {@link org.fdroid.fdroid.data.SanitizedFile#knownSanitized(java.io.File)} + * method, but intentionally kept private so people don't think that any sanitization + * will occur by passing a file in - because it wont. + */ + private SanitizedFile(File file) { + super(file.getAbsolutePath()); + } + + /** + * This is dangerous, but there will be some cases when all we have is an absolute file + * path that wasn't given to us from user input. One example would be asking Android for + * the path to an installed .apk on disk. In such situations, we can't meaningfully + * sanitize it, but will still need to pass to a function which only allows SanitizedFile's + * as arguments (because they interact with, e.g. shells). + * + * To illustrate, imagine perfectly valid file path: "/tmp/../secret/file.txt", + * one cannot distinguish between: + * + * "/tmp/" (known safe directory) + "../secret/file.txt" (suspicious looking file name) + * + * and + * + * "/tmp/../secret/" (known safe directory) + "file.txt" (known safe file name) + * + * I guess the best this method offers us is the ability to uniquely trace the different + * ways in which files are created and handled. It should make it easier to find and + * prevent suspect usages of methods which only expect SanitizedFile's, but are given + * a SanitizedFile returned from this method that really originated from user input. + */ + public static SanitizedFile knownSanitized(String path) { + return new SanitizedFile(new File(path)); + } + + /** + * @see {@link org.fdroid.fdroid.data.SanitizedFile#knownSanitized(String)} + */ + public static SanitizedFile knownSanitized(File file) { + return new SanitizedFile(file); + } + +} diff --git a/F-Droid/src/org/fdroid/fdroid/localrepo/LocalRepoManager.java b/F-Droid/src/org/fdroid/fdroid/localrepo/LocalRepoManager.java index 3fa7c36bc..45fa2e9e5 100644 --- a/F-Droid/src/org/fdroid/fdroid/localrepo/LocalRepoManager.java +++ b/F-Droid/src/org/fdroid/fdroid/localrepo/LocalRepoManager.java @@ -25,6 +25,7 @@ import org.fdroid.fdroid.Preferences; import org.fdroid.fdroid.R; import org.fdroid.fdroid.Utils; import org.fdroid.fdroid.data.App; +import org.fdroid.fdroid.data.SanitizedFile; import org.w3c.dom.Document; import org.w3c.dom.Element; @@ -81,13 +82,13 @@ public class LocalRepoManager { private Map apps = new HashMap(); - public final File xmlIndex; - private File xmlIndexJar = null; - private File xmlIndexJarUnsigned = null; - public final File webRoot; - public final File fdroidDir; - public final File repoDir; - public final File iconsDir; + public final SanitizedFile xmlIndex; + private SanitizedFile xmlIndexJar = null; + private SanitizedFile xmlIndexJarUnsigned = null; + public final SanitizedFile webRoot; + public final SanitizedFile fdroidDir; + public final SanitizedFile repoDir; + public final SanitizedFile iconsDir; private static LocalRepoManager localRepoManager; @@ -104,14 +105,14 @@ public class LocalRepoManager { prefs = PreferenceManager.getDefaultSharedPreferences(c); fdroidPackageName = c.getPackageName(); - webRoot = c.getFilesDir(); + webRoot = SanitizedFile.knownSanitized(c.getFilesDir()); /* /fdroid/repo is the standard path for user repos */ - fdroidDir = new File(webRoot, "fdroid"); - repoDir = new File(fdroidDir, "repo"); - iconsDir = new File(repoDir, "icons"); - xmlIndex = new File(repoDir, "index.xml"); - xmlIndexJar = new File(repoDir, "index.jar"); - xmlIndexJarUnsigned = new File(repoDir, "index.unsigned.jar"); + fdroidDir = new SanitizedFile(webRoot, "fdroid"); + repoDir = new SanitizedFile(fdroidDir, "repo"); + iconsDir = new SanitizedFile(repoDir, "icons"); + xmlIndex = new SanitizedFile(repoDir, "index.xml"); + xmlIndexJar = new SanitizedFile(repoDir, "index.jar"); + xmlIndexJarUnsigned = new SanitizedFile(repoDir, "index.unsigned.jar"); if (!fdroidDir.exists()) if (!fdroidDir.mkdir()) @@ -136,8 +137,8 @@ public class LocalRepoManager { try { appInfo = pm.getApplicationInfo(fdroidPackageName, PackageManager.GET_META_DATA); - File apkFile = new File(appInfo.publicSourceDir); - File fdroidApkLink = new File(webRoot, "fdroid.client.apk"); + SanitizedFile apkFile = SanitizedFile.knownSanitized(appInfo.publicSourceDir); + SanitizedFile fdroidApkLink = new SanitizedFile(webRoot, "fdroid.client.apk"); fdroidApkLink.delete(); if (Utils.symlinkOrCopyFile(apkFile, fdroidApkLink)) fdroidClientURL = "/" + fdroidApkLink.getName(); @@ -191,14 +192,14 @@ public class LocalRepoManager { } private void symlinkIndexPageElsewhere(String symlinkPrefix, File directory) { - File index = new File(directory, "index.html"); + SanitizedFile index = new SanitizedFile(directory, "index.html"); index.delete(); - Utils.symlinkOrCopyFile(new File(symlinkPrefix + "index.html"), index); + Utils.symlinkOrCopyFile(new SanitizedFile(new File(symlinkPrefix), "index.html"), index); for(String fileName : WEB_ROOT_ASSET_FILES) { - File file = new File(directory, fileName); + SanitizedFile file = new SanitizedFile(directory, fileName); file.delete(); - Utils.symlinkOrCopyFile(new File(symlinkPrefix + fileName), file); + Utils.symlinkOrCopyFile(new SanitizedFile(new File(symlinkPrefix), fileName), file); } } @@ -227,7 +228,7 @@ public class LocalRepoManager { App app = apps.get(packageName); if (app.installedApk != null) { - File outFile = new File(repoDir, app.installedApk.apkName); + SanitizedFile outFile = new SanitizedFile(repoDir, app.installedApk.apkName); if (Utils.symlinkOrCopyFile(app.installedApk.installedFile, outFile)) continue; } diff --git a/F-Droid/src/org/fdroid/fdroid/net/ApkDownloader.java b/F-Droid/src/org/fdroid/fdroid/net/ApkDownloader.java index c8c6bcc15..e989da6d6 100644 --- a/F-Droid/src/org/fdroid/fdroid/net/ApkDownloader.java +++ b/F-Droid/src/org/fdroid/fdroid/net/ApkDownloader.java @@ -25,7 +25,9 @@ import android.util.Log; import org.fdroid.fdroid.Hasher; import org.fdroid.fdroid.ProgressListener; +import org.fdroid.fdroid.compat.FileCompat; import org.fdroid.fdroid.data.Apk; +import org.fdroid.fdroid.data.SanitizedFile; import java.io.File; import java.io.IOException; @@ -59,7 +61,7 @@ public class ApkDownloader implements AsyncDownloadWrapper.Listener { private Apk curApk; private String repoAddress; - private File localFile; + private SanitizedFile localFile; private ProgressListener listener; private AsyncDownloadWrapper dlWrapper = null; @@ -80,13 +82,13 @@ public class ApkDownloader implements AsyncDownloadWrapper.Listener { public ApkDownloader(Apk apk, String repoAddress, File destDir) { curApk = apk; this.repoAddress = repoAddress; - localFile = new File(destDir, curApk.apkName); + localFile = new SanitizedFile(destDir, curApk.apkName); } /** * The downloaded APK. Valid only when getStatus() has returned STATUS.DONE. */ - public File localFile() { + public SanitizedFile localFile() { return localFile; } @@ -239,6 +241,13 @@ public class ApkDownloader implements AsyncDownloadWrapper.Listener { return; } + // Need the apk to be world readable, so that the installer is able to read it. + // Note that saving it into external storage for the purpose of letting the installer + // have access is insecure, because apps with permission to write to the external + // storage can overwrite the app between F-Droid asking for it to be installed and + // the installer actually installing it. + FileCompat.setReadable(localFile, true, false); + Log.d("FDroid", "Download finished: " + localFile); sendCompleteMessage(); } diff --git a/F-Droid/test/project.properties b/F-Droid/test/project.properties index 6e18427a4..e611232d2 100644 --- a/F-Droid/test/project.properties +++ b/F-Droid/test/project.properties @@ -12,3 +12,10 @@ # Project target. target=android-21 + +# With a target SDK of android-21 (5.0/Lollipop) and a Java 1.7 compiler, you +# can use Java 1.7 features like the <> diamond operator, multi-catch, strings +# in switches, etc. +java.encoding=UTF-8 +java.source=1.7 +java.target=1.7 diff --git a/F-Droid/test/src/org/fdroid/fdroid/FileCompatTest.java b/F-Droid/test/src/org/fdroid/fdroid/FileCompatTest.java new file mode 100644 index 000000000..4f1c48110 --- /dev/null +++ b/F-Droid/test/src/org/fdroid/fdroid/FileCompatTest.java @@ -0,0 +1,66 @@ +package org.fdroid.fdroid; + +import android.os.Build; +import android.test.InstrumentationTestCase; +import android.util.Log; +import org.fdroid.fdroid.compat.FileCompatForTest; +import org.fdroid.fdroid.data.SanitizedFile; + +import java.io.File; + +public class FileCompatTest extends InstrumentationTestCase { + + private static final String TAG = "org.fdroid.fdroid.FileCompatTest"; + + private File dir; + private SanitizedFile sourceFile; + private SanitizedFile destFile; + + public void setUp() { + dir = TestUtils.getWriteableDir(getInstrumentation()); + sourceFile = SanitizedFile.knownSanitized(TestUtils.copyAssetToDir(getInstrumentation().getContext(), "simpleIndex.jar", dir)); + destFile = new SanitizedFile(dir, "dest.txt"); + assertTrue(!destFile.exists()); + assertTrue(sourceFile.getAbsolutePath() + " should exist.", sourceFile.exists()); + } + + public void tearDown() { + assertTrue("Can't delete " + sourceFile.getAbsolutePath() + ".", sourceFile.delete()); + assertTrue("Can't delete " + destFile.getAbsolutePath() + ".", destFile.delete()); + } + + public void testSymlinkRuntime() { + SanitizedFile destFile = new SanitizedFile(dir, "dest.txt"); + assertFalse(destFile.exists()); + + FileCompatForTest.symlinkRuntimeTest(sourceFile, destFile); + assertTrue(destFile.getAbsolutePath() + " should exist after symlinking", destFile.exists()); + } + + public void testSymlinkLibcore() { + + if (Build.VERSION.SDK_INT >= 19) { + SanitizedFile destFile = new SanitizedFile(dir, "dest.txt"); + assertFalse(destFile.exists()); + + FileCompatForTest.symlinkLibcoreTest(sourceFile, destFile); + assertTrue(destFile.getAbsolutePath() + " should exist after symlinking", destFile.exists()); + } else { + Log.w(TAG, "Cannot test symlink-libcore on this device. Requires android-19, but this has android-" + Build.VERSION.SDK_INT); + } + } + + public void testSymlinkOs() { + + if (Build.VERSION.SDK_INT >= 21 ) { + SanitizedFile destFile = new SanitizedFile(dir, "dest.txt"); + assertFalse(destFile.exists()); + + FileCompatForTest.symlinkOsTest(sourceFile, destFile); + assertTrue(destFile.getAbsolutePath() + " should exist after symlinking", destFile.exists()); + } else { + Log.w(TAG, "Cannot test symlink-os on this device. Requires android-21, but only has android-" + Build.VERSION.SDK_INT); + } + } + +} diff --git a/F-Droid/test/src/org/fdroid/fdroid/SanitizedFileTest.java b/F-Droid/test/src/org/fdroid/fdroid/SanitizedFileTest.java new file mode 100644 index 000000000..6cde747fc --- /dev/null +++ b/F-Droid/test/src/org/fdroid/fdroid/SanitizedFileTest.java @@ -0,0 +1,44 @@ +package org.fdroid.fdroid; + +import android.test.AndroidTestCase; +import org.fdroid.fdroid.data.SanitizedFile; + +import java.io.File; + +public class SanitizedFileTest extends AndroidTestCase { + + public void testSanitizedFile() { + + File directory = new File("/tmp/blah"); + + String safeFile = "safe"; + String nonEvilFile = "$%^safe-and_bleh.boo*@~"; + String evilFile = ";rm /etc/shadow;"; + + File safeNotSanitized = new File(directory, safeFile); + File nonEvilNotSanitized = new File(directory, nonEvilFile); + File evilNotSanitized = new File(directory, evilFile); + + assertEquals("/tmp/blah/safe", safeNotSanitized.getAbsolutePath()); + assertEquals("/tmp/blah/$%^safe-and_bleh.boo*@~", nonEvilNotSanitized.getAbsolutePath()); + assertEquals("/tmp/blah/;rm /etc/shadow;", evilNotSanitized.getAbsolutePath()); + + assertEquals("safe", safeNotSanitized.getName()); + assertEquals("$%^safe-and_bleh.boo*@~", nonEvilNotSanitized.getName()); + assertEquals("shadow;", evilNotSanitized.getName()); // Should be ;rm /etc/shadow; but the forward slashes are naughty. + + SanitizedFile safeSanitized = new SanitizedFile(directory, safeFile); + SanitizedFile nonEvilSanitized = new SanitizedFile(directory, nonEvilFile); + SanitizedFile evilSanitized = new SanitizedFile(directory, evilFile); + + assertEquals("/tmp/blah/safe", safeSanitized.getAbsolutePath()); + assertEquals("/tmp/blah/safe-and_bleh.boo", nonEvilSanitized.getAbsolutePath()); + assertEquals("/tmp/blah/rmetcshadow", evilSanitized.getAbsolutePath()); + + assertEquals("safe", safeSanitized.getName()); + assertEquals("safe-and_bleh.boo", nonEvilSanitized.getName()); + assertEquals("rmetcshadow", evilSanitized.getName()); + + } + +} diff --git a/F-Droid/test/src/org/fdroid/fdroid/TestUtils.java b/F-Droid/test/src/org/fdroid/fdroid/TestUtils.java index e4927a3fe..1f6b65c45 100644 --- a/F-Droid/test/src/org/fdroid/fdroid/TestUtils.java +++ b/F-Droid/test/src/org/fdroid/fdroid/TestUtils.java @@ -1,18 +1,24 @@ package org.fdroid.fdroid; +import android.app.Instrumentation; import android.content.*; import android.net.Uri; +import android.os.Environment; +import android.util.Log; import junit.framework.AssertionFailedError; import mock.MockInstallablePackageManager; import org.fdroid.fdroid.data.ApkProvider; import org.fdroid.fdroid.data.AppProvider; +import java.io.*; import java.util.ArrayList; import java.util.Collections; import java.util.List; public class TestUtils { + private static final String TAG = "org.fdroid.fdroid.TestUtils"; + public static void assertContainsOnly(List actualList, T[] expectedArray) { List expectedList = new ArrayList(expectedArray.length); Collections.addAll(expectedList, expectedArray); @@ -175,4 +181,68 @@ public class TestUtils { } + public static File copyAssetToDir(Context context, String assetName, File directory) { + File tempFile; + InputStream input = null; + OutputStream output = null; + try { + tempFile = File.createTempFile(assetName + "-", ".testasset", directory); + Log.d(TAG, "Copying asset file " + assetName + " to directory " + directory); + input = context.getResources().getAssets().open(assetName); + output = new FileOutputStream(tempFile); + Utils.copy(input, output); + } catch (IOException e) { + e.printStackTrace(); + return null; + } finally { + Utils.closeQuietly(output); + Utils.closeQuietly(input); + } + return tempFile; + } + + /** + * Prefer internal over external storage, because external tends to be FAT filesystems, + * which don't support symlinks (which we test using this method). + */ + public static File getWriteableDir(Instrumentation instrumentation) { + Context context = instrumentation.getContext(); + Context targetContext = instrumentation.getTargetContext(); + File dir = context.getCacheDir(); + Log.d(TAG, "Looking for writeable dir, trying context.getCacheDir()" ); + if (dir == null || !dir.canWrite()) { + Log.d(TAG, "Looking for writeable dir, trying context.getFilesDir()"); + dir = context.getFilesDir(); + } + if (dir == null || !dir.canWrite()) { + Log.d(TAG, "Looking for writeable dir, trying targetContext.getCacheDir()"); + dir = targetContext.getCacheDir(); + } + if (dir == null || !dir.canWrite()) { + Log.d(TAG, "Looking for writeable dir, trying targetContext.getFilesDir()"); + dir = targetContext.getFilesDir(); + } + if (dir == null || !dir.canWrite()) { + Log.d(TAG, "Looking for writeable dir, trying context.getExternalCacheDir()"); + dir = context.getExternalCacheDir(); + } + if (dir == null || !dir.canWrite()) { + Log.d(TAG, "Looking for writeable dir, trying context.getExternalFilesDir(null)"); + dir = context.getExternalFilesDir(null); + } + if (dir == null || !dir.canWrite()) { + Log.d(TAG, "Looking for writeable dir, trying targetContext.getExternalCacheDir()"); + dir = targetContext.getExternalCacheDir(); + } + if (dir == null || !dir.canWrite()) { + Log.d(TAG, "Looking for writeable dir, trying targetContext.getExternalFilesDir(null)"); + dir = targetContext.getExternalFilesDir(null); + } + if (dir == null || !dir.canWrite()) { + Log.d(TAG, "Looking for writeable dir, trying Environment.getExternalStorageDirectory()"); + dir = Environment.getExternalStorageDirectory(); + } + Log.d(TAG, "Writeable dir found: " + dir); + return dir; + } } diff --git a/F-Droid/test/src/org/fdroid/fdroid/compat/FileCompatForTest.java b/F-Droid/test/src/org/fdroid/fdroid/compat/FileCompatForTest.java new file mode 100644 index 000000000..f5ac4607f --- /dev/null +++ b/F-Droid/test/src/org/fdroid/fdroid/compat/FileCompatForTest.java @@ -0,0 +1,33 @@ +package org.fdroid.fdroid.compat; + +import android.annotation.TargetApi; +import android.os.Build; +import android.system.ErrnoException; +import org.fdroid.fdroid.Utils; +import org.fdroid.fdroid.data.SanitizedFile; + +import java.io.IOException; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; + +/** + * Used to expose the protected methods from FileCompat in a public manner so + * that they can be called from a test harness. + */ +public class FileCompatForTest extends FileCompat { + + @TargetApi(Build.VERSION_CODES.LOLLIPOP) + public static void symlinkOsTest(SanitizedFile source, SanitizedFile dest) { + symlinkOs(source, dest); + } + + public static void symlinkRuntimeTest(SanitizedFile source, SanitizedFile dest) { + symlinkRuntime(source, dest); + } + + public static void symlinkLibcoreTest(SanitizedFile source, SanitizedFile dest) { + symlinkLibcore(source, dest); + } + + +} diff --git a/F-Droid/test/src/org/fdroid/fdroid/updater/SignedRepoUpdaterTest.java b/F-Droid/test/src/org/fdroid/fdroid/updater/SignedRepoUpdaterTest.java index dc98ab5e8..e11f9639c 100644 --- a/F-Droid/test/src/org/fdroid/fdroid/updater/SignedRepoUpdaterTest.java +++ b/F-Droid/test/src/org/fdroid/fdroid/updater/SignedRepoUpdaterTest.java @@ -8,6 +8,7 @@ import android.test.InstrumentationTestCase; import android.util.Log; import org.apache.commons.io.FileUtils; +import org.fdroid.fdroid.TestUtils; import org.fdroid.fdroid.Utils; import org.fdroid.fdroid.data.Repo; import org.fdroid.fdroid.updater.RepoUpdater.UpdateException; @@ -31,56 +32,17 @@ public class SignedRepoUpdaterTest extends InstrumentationTestCase { @Override protected void setUp() { context = getInstrumentation().getContext(); - Context target = getInstrumentation().getTargetContext(); - /* find something that works, many of these often fail on emulators */ - testFilesDir = context.getFilesDir(); - if (testFilesDir == null || !testFilesDir.canWrite()) - testFilesDir = context.getCacheDir(); - if (testFilesDir == null || !testFilesDir.canWrite()) - testFilesDir = context.getExternalCacheDir(); - if (testFilesDir == null || !testFilesDir.canWrite()) - testFilesDir = context.getExternalFilesDir(null); - if (testFilesDir == null || !testFilesDir.canWrite()) - testFilesDir = target.getFilesDir(); - if (testFilesDir == null || !testFilesDir.canWrite()) - testFilesDir = target.getCacheDir(); - if (testFilesDir == null || !testFilesDir.canWrite()) - testFilesDir = target.getExternalCacheDir(); - if (testFilesDir == null || !testFilesDir.canWrite()) - testFilesDir = target.getExternalFilesDir(null); - if (testFilesDir == null || !testFilesDir.canWrite()) - testFilesDir = Environment.getExternalStorageDirectory(); - Log.i(TAG, "testFilesDir: " + testFilesDir); + testFilesDir = TestUtils.getWriteableDir(getInstrumentation()); Repo repo = new Repo(); repo.pubkey = this.simpleIndexPubkey; repoUpdater = RepoUpdater.createUpdaterFor(context, repo); } - private File getTestFile(String fileName) { - File indexFile; - InputStream input = null; - OutputStream output = null; - try { - indexFile = File.createTempFile(fileName + "-", ".xml", testFilesDir); - Log.i(TAG, "getTestFile indexFile " + indexFile); - input = context.getResources().getAssets().open(fileName); - output = new FileOutputStream(indexFile); - Utils.copy(input, output); - } catch (IOException e) { - e.printStackTrace(); - return null; - } finally { - Utils.closeQuietly(output); - Utils.closeQuietly(input); - } - return indexFile; - } - public void testExtractIndexFromJar() { if (!testFilesDir.canWrite()) return; - File simpleIndexXml = getTestFile("simpleIndex.xml"); - File simpleIndexJar = getTestFile("simpleIndex.jar"); + File simpleIndexXml = TestUtils.copyAssetToDir(context, "simpleIndex.xml", testFilesDir); + File simpleIndexJar = TestUtils.copyAssetToDir(context, "simpleIndex.jar", testFilesDir); File testFile = null; // these are supposed to succeed @@ -103,7 +65,7 @@ public class SignedRepoUpdaterTest extends InstrumentationTestCase { return; // this is supposed to fail try { - repoUpdater.getIndexFromFile(getTestFile("simpleIndexWithoutSignature.jar")); + repoUpdater.getIndexFromFile(TestUtils.copyAssetToDir(context, "simpleIndexWithoutSignature.jar", testFilesDir)); fail(); } catch (UpdateException e) { // success! @@ -115,7 +77,7 @@ public class SignedRepoUpdaterTest extends InstrumentationTestCase { return; // this is supposed to fail try { - repoUpdater.getIndexFromFile(getTestFile("simpleIndexWithCorruptedManifest.jar")); + repoUpdater.getIndexFromFile(TestUtils.copyAssetToDir(context, "simpleIndexWithCorruptedManifest.jar", testFilesDir)); fail(); } catch (UpdateException e) { e.printStackTrace(); @@ -130,7 +92,7 @@ public class SignedRepoUpdaterTest extends InstrumentationTestCase { return; // this is supposed to fail try { - repoUpdater.getIndexFromFile(getTestFile("simpleIndexWithCorruptedSignature.jar")); + repoUpdater.getIndexFromFile(TestUtils.copyAssetToDir(context, "simpleIndexWithCorruptedSignature.jar", testFilesDir)); fail(); } catch (UpdateException e) { e.printStackTrace(); @@ -145,7 +107,7 @@ public class SignedRepoUpdaterTest extends InstrumentationTestCase { return; // this is supposed to fail try { - repoUpdater.getIndexFromFile(getTestFile("simpleIndexWithCorruptedCertificate.jar")); + repoUpdater.getIndexFromFile(TestUtils.copyAssetToDir(context, "simpleIndexWithCorruptedCertificate.jar", testFilesDir)); fail(); } catch (UpdateException e) { e.printStackTrace(); @@ -160,7 +122,7 @@ public class SignedRepoUpdaterTest extends InstrumentationTestCase { return; // this is supposed to fail try { - repoUpdater.getIndexFromFile(getTestFile("simpleIndexWithCorruptedEverything.jar")); + repoUpdater.getIndexFromFile(TestUtils.copyAssetToDir(context, "simpleIndexWithCorruptedEverything.jar", testFilesDir)); fail(); } catch (UpdateException e) { e.printStackTrace(); @@ -175,11 +137,9 @@ public class SignedRepoUpdaterTest extends InstrumentationTestCase { return; // this is supposed to fail try { - repoUpdater.getIndexFromFile(getTestFile("masterKeyIndex.jar")); + repoUpdater.getIndexFromFile(TestUtils.copyAssetToDir(context, "masterKeyIndex.jar", testFilesDir)); fail(); - } catch (UpdateException e) { - // success! - } catch (SecurityException e) { + } catch (UpdateException | SecurityException e) { // success! } }