Bug 1238785 - Add FileCleanupService. r=ahunt draft
authorMichael Comella <michael.l.comella@gmail.com>
Tue, 12 Apr 2016 17:10:35 -0700
changeset 350219 169d81de3d4a014c3145269328b296c20cbefbd5
parent 349534 5b37f138e5bfdb84fe3460dd115ec225933e0f5f
child 350220 2ab0e0f3e618f63b81dda92f83d4e5b669acd900
push id15272
push usermichael.l.comella@gmail.com
push dateWed, 13 Apr 2016 00:15:45 +0000
reviewersahunt
bugs1238785
milestone48.0a1
Bug 1238785 - Add FileCleanupService. r=ahunt This is intentionally kept minimal to ensure simplicity. MozReview-Commit-ID: IJRxrTbWN2P
mobile/android/base/AndroidManifest.xml.in
mobile/android/base/java/org/mozilla/gecko/cleanup/FileCleanupService.java
mobile/android/base/moz.build
mobile/android/tests/background/junit4/src/org/mozilla/gecko/cleanup/TestFileCleanupService.java
--- a/mobile/android/base/AndroidManifest.xml.in
+++ b/mobile/android/base/AndroidManifest.xml.in
@@ -348,16 +348,22 @@
             android:name="org.mozilla.gecko.dlc.DownloadContentService">
         </service>
 
         <service
             android:exported="false"
             android:name="org.mozilla.gecko.feeds.FeedService">
         </service>
 
+        <!-- DON'T EXPORT THIS, please! An attacker could delete arbitrary files. -->
+        <service
+            android:exported="false"
+            android:name="org.mozilla.gecko.cleanup.FileCleanupService">
+        </service>
+
         <receiver
             android:name="org.mozilla.gecko.feeds.FeedAlarmReceiver"
             android:exported="false" />
 
         <receiver
             android:name="org.mozilla.gecko.BootReceiver"
             android:exported="false">
             <intent-filter>
new file mode 100644
--- /dev/null
+++ b/mobile/android/base/java/org/mozilla/gecko/cleanup/FileCleanupService.java
@@ -0,0 +1,80 @@
+/*
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, you can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+
+package org.mozilla.gecko.cleanup;
+
+import android.app.IntentService;
+import android.content.Intent;
+import android.util.Log;
+
+import java.io.File;
+import java.util.ArrayList;
+
+/**
+ * An IntentService to delete files.
+ *
+ * It takes an {@link ArrayList} of String file paths to delete via the extra
+ * {@link #EXTRA_FILE_PATHS_TO_DELETE}. If these file paths are directories, they will
+ * not be traversed recursively and will only be deleted if empty. This is to avoid accidentally
+ * trashing a users' profile if a folder is accidentally listed.
+ *
+ * An IntentService was chosen because:
+ *   * It generally won't be killed when the Activity is
+ *   * (unlike HandlerThread) The system handles scheduling, prioritizing,
+ * and shutting down the underlying background thread
+ *   * (unlike an existing background thread) We don't block our background operations
+ * for this, which doesn't directly affect the user.
+ *
+ * The major trade-off is that this Service is very dangerous if it's exported... so don't do that!
+ */
+public class FileCleanupService extends IntentService {
+    private static final String LOGTAG = "Gecko" + FileCleanupService.class.getSimpleName();
+    private static final String WORKER_THREAD_NAME = LOGTAG + "Worker";
+
+    public static final String ACTION_DELETE_FILES = "org.mozilla.gecko.intent.action.DELETE_FILES";
+    public static final String EXTRA_FILE_PATHS_TO_DELETE = "org.mozilla.gecko.file_paths_to_delete";
+
+    public FileCleanupService() {
+        super(WORKER_THREAD_NAME);
+
+        // We're likely to get scheduled again - let's wait until then in order to avoid:
+        //   * The coding complexity of re-running this
+        //   * Consuming system resources: we were probably killed for resource conservation purposes
+        setIntentRedelivery(false);
+    }
+
+    @Override
+    protected void onHandleIntent(final Intent intent) {
+        if (!isIntentValid(intent)) {
+            return;
+        }
+
+        final ArrayList<String> filesToDelete = intent.getStringArrayListExtra(EXTRA_FILE_PATHS_TO_DELETE);
+        for (final String path : filesToDelete) {
+            final File file = new File(path);
+            file.delete();
+        }
+    }
+
+    private static boolean isIntentValid(final Intent intent) {
+        if (intent == null) {
+            Log.w(LOGTAG, "Received null intent");
+            return false;
+        }
+
+        if (!intent.getAction().equals(ACTION_DELETE_FILES)) {
+            Log.w(LOGTAG, "Received unknown intent action: " + intent.getAction());
+            return false;
+        }
+
+        if (!intent.hasExtra(EXTRA_FILE_PATHS_TO_DELETE)) {
+            Log.w(LOGTAG, "Received intent with no files extra");
+            return false;
+        }
+
+        return true;
+    }
+}
--- a/mobile/android/base/moz.build
+++ b/mobile/android/base/moz.build
@@ -202,16 +202,17 @@ gbjar.sources += ['java/org/mozilla/geck
     'animation/Rotate3DAnimation.java',
     'animation/ViewHelper.java',
     'ANRReporter.java',
     'AppNotificationClient.java',
     'BaseGeckoInterface.java',
     'BootReceiver.java',
     'BrowserApp.java',
     'BrowserLocaleManager.java',
+    'cleanup/FileCleanupService.java',
     'ContactService.java',
     'ContextGetter.java',
     'CrashHandler.java',
     'CustomEditText.java',
     'DataReportingNotification.java',
     'db/AbstractPerProfileDatabaseProvider.java',
     'db/AbstractTransactionalProvider.java',
     'db/BaseTable.java',
new file mode 100644
--- /dev/null
+++ b/mobile/android/tests/background/junit4/src/org/mozilla/gecko/cleanup/TestFileCleanupService.java
@@ -0,0 +1,106 @@
+/*
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, you can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+
+package org.mozilla.gecko.cleanup;
+
+import android.content.Intent;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+import org.junit.runner.RunWith;
+import org.mozilla.gecko.background.testhelpers.TestRunner;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+
+import static org.junit.Assert.*;
+
+/**
+ * Tests the methods of {@link FileCleanupService}.
+ */
+@RunWith(TestRunner.class)
+public class TestFileCleanupService {
+    @Rule
+    public final TemporaryFolder tempFolder = new TemporaryFolder();
+
+    private void assertAllFilesExist(final List<File> fileList) {
+        for (final File file : fileList) {
+            assertTrue("File exists", file.exists());
+        }
+    }
+
+    private void assertAllFilesDoNotExist(final List<File> fileList) {
+        for (final File file : fileList) {
+            assertFalse("File does not exist", file.exists());
+        }
+    }
+
+    private void onHandleIntent(final ArrayList<String> filePaths) {
+        final FileCleanupService service = new FileCleanupService();
+        final Intent intent = new Intent(FileCleanupService.ACTION_DELETE_FILES);
+        intent.putExtra(FileCleanupService.EXTRA_FILE_PATHS_TO_DELETE, filePaths);
+        service.onHandleIntent(intent);
+    }
+
+    @Test
+    public void testOnHandleIntentDeleteSpecifiedFiles() throws Exception {
+        final int fileListCount = 3;
+        final ArrayList<File> filesToDelete = generateFileList(fileListCount);
+
+        final ArrayList<String> pathsToDelete = new ArrayList<>(fileListCount);
+        for (final File file : filesToDelete) {
+            pathsToDelete.add(file.getAbsolutePath());
+        }
+
+        assertAllFilesExist(filesToDelete);
+        onHandleIntent(pathsToDelete);
+        assertAllFilesDoNotExist(filesToDelete);
+    }
+
+    @Test
+    public void testOnHandleIntentDoesNotDeleteUnrelatedFiles() throws Exception {
+        final ArrayList<File> filesShouldNotBeDeleted = generateFileList(3);
+        assertAllFilesExist(filesShouldNotBeDeleted);
+        onHandleIntent(new ArrayList<String>());
+        assertAllFilesExist(filesShouldNotBeDeleted);
+    }
+
+    @Test
+    public void testOnHandleIntentDeletesEmptyDirectory() throws Exception {
+        final File dir = tempFolder.newFolder();
+        final ArrayList<String> filesToDelete = new ArrayList<>(1);
+        filesToDelete.add(dir.getAbsolutePath());
+
+        assertTrue("Empty directory exists", dir.exists());
+        onHandleIntent(filesToDelete);
+        assertFalse("Empty directory deleted by service", dir.exists());
+    }
+
+    @Test
+    public void testOnHandleIntentDoesNotDeleteNonEmptyDirectory() throws Exception {
+        final File dir = tempFolder.newFolder();
+        final ArrayList<String> filesCannotDelete = new ArrayList<>(1);
+        filesCannotDelete.add(dir.getAbsolutePath());
+        assertTrue("Directory exists", dir.exists());
+
+        final File fileInDir = new File(dir, "file_in_dir");
+        assertTrue("File in dir created", fileInDir.createNewFile());
+
+        onHandleIntent(filesCannotDelete);
+        assertTrue("Non-empty directory not deleted", dir.exists());
+        assertTrue("File in directory not deleted", fileInDir.exists());
+    }
+
+    private ArrayList<File> generateFileList(final int size) throws IOException {
+        final ArrayList<File> fileList = new ArrayList<>(size);
+        for (int i = 0; i < size; ++i) {
+            fileList.add(tempFolder.newFile());
+        }
+        return fileList;
+    }
+}