From 4e198d2556858e36314cbf111125986b331b3564 Mon Sep 17 00:00:00 2001 From: Alexander Bakker Date: Fri, 20 Nov 2020 16:28:08 +0100 Subject: [PATCH] Move the creation of exports/backups to a background thread This writes exports/backups to the cache directory first and then hands off to a background thread to pipe the file to SAF. I'm cheating a bit, but it's the easiest fix with our current architecture. In the future, we should skip the first step by making VaultManager itself thread-safe. --- .../aegis/AegisApplication.java | 2 + .../aegis/ui/PreferencesFragment.java | 51 ++++++++-- .../aegis/ui/tasks/ExportTask.java | 64 ++++++++++++ .../aegis/vault/VaultBackupManager.java | 97 ++++++++++++------- .../aegis/vault/VaultManager.java | 37 +++++-- app/src/main/res/values/strings.xml | 3 +- 6 files changed, 201 insertions(+), 53 deletions(-) create mode 100644 app/src/main/java/com/beemdevelopment/aegis/ui/tasks/ExportTask.java diff --git a/app/src/main/java/com/beemdevelopment/aegis/AegisApplication.java b/app/src/main/java/com/beemdevelopment/aegis/AegisApplication.java index 5d6ecc5d..1ef86ffc 100644 --- a/app/src/main/java/com/beemdevelopment/aegis/AegisApplication.java +++ b/app/src/main/java/com/beemdevelopment/aegis/AegisApplication.java @@ -140,7 +140,9 @@ public class AegisApplication extends Application { * @param userInitiated whether or not the user initiated the lock in MainActivity. */ public void lock(boolean userInitiated) { + _manager.destroy(); _manager = null; + for (LockListener listener : _lockListeners) { listener.onLocked(userInitiated); } diff --git a/app/src/main/java/com/beemdevelopment/aegis/ui/PreferencesFragment.java b/app/src/main/java/com/beemdevelopment/aegis/ui/PreferencesFragment.java index 5450c750..be2779b7 100644 --- a/app/src/main/java/com/beemdevelopment/aegis/ui/PreferencesFragment.java +++ b/app/src/main/java/com/beemdevelopment/aegis/ui/PreferencesFragment.java @@ -44,6 +44,7 @@ import com.beemdevelopment.aegis.importers.DatabaseImporterException; import com.beemdevelopment.aegis.services.NotificationService; import com.beemdevelopment.aegis.ui.models.ImportEntry; import com.beemdevelopment.aegis.ui.preferences.SwitchPreference; +import com.beemdevelopment.aegis.ui.tasks.ExportTask; import com.beemdevelopment.aegis.ui.tasks.PasswordSlotDecryptTask; import com.beemdevelopment.aegis.util.UUIDMap; import com.beemdevelopment.aegis.vault.VaultBackupManager; @@ -747,11 +748,7 @@ public class PreferencesFragment extends PreferenceFragmentCompat { File file; try { VaultBackupManager.FileInfo fileInfo = getExportFileInfo(spinner.getSelectedItemPosition(), checkBoxEncrypt.isChecked()); - File dir = new File(getContext().getCacheDir(), "export"); - if (!dir.exists() && !dir.mkdir()) { - throw new IOException(String.format("Unable to create directory %s", dir)); - } - file = File.createTempFile(fileInfo.getFilename() + "-", "." + fileInfo.getExtension(), dir); + file = File.createTempFile(fileInfo.getFilename() + "-", "." + fileInfo.getExtension(), getExportCacheDir()); } catch (IOException e) { e.printStackTrace(); Dialogs.showErrorDialog(getContext(), R.string.exporting_vault_error, e); @@ -863,6 +860,15 @@ public class PreferencesFragment extends PreferenceFragmentCompat { return requestCode == CODE_EXPORT_GOOGLE_URI ? "text/plain" : "application/json"; } + private File getExportCacheDir() throws IOException { + File dir = new File(getContext().getCacheDir(), "export"); + if (!dir.exists() && !dir.mkdir()) { + throw new IOException(String.format("Unable to create directory %s", dir)); + } + + return dir; + } + private void startExportVault(int requestCode, StartExportCallback cb) { switch (requestCode) { case CODE_EXPORT: @@ -907,15 +913,28 @@ public class PreferencesFragment extends PreferenceFragmentCompat { return; } + startExportVault(requestCode, cb -> { - try (OutputStream stream = getContext().getContentResolver().openOutputStream(uri, "w")) { - cb.exportVault(stream); - } catch (IOException | VaultManagerException e) { + File file; + OutputStream outStream = null; + try { + file = File.createTempFile(VaultManager.FILENAME_PREFIX_EXPORT + "-", ".json", getExportCacheDir()); + outStream = new FileOutputStream(file); + cb.exportVault(outStream); + + new ExportTask(getContext(), new ExportResultListener()).execute(getLifecycle(), new ExportTask.Params(file, uri)); + } catch (VaultManagerException | IOException e) { e.printStackTrace(); Dialogs.showErrorDialog(getContext(), R.string.exporting_vault_error, e); + } finally { + try { + if (outStream != null) { + outStream.close(); + } + } catch (IOException e) { + e.printStackTrace(); + } } - - Toast.makeText(getActivity(), getString(R.string.exported_vault), Toast.LENGTH_SHORT).show(); }); } @@ -1132,6 +1151,18 @@ public class PreferencesFragment extends PreferenceFragmentCompat { } } + private class ExportResultListener implements ExportTask.Callback { + @Override + public void onTaskFinished(Exception e) { + if (e != null) { + e.printStackTrace(); + Dialogs.showErrorDialog(getContext(), R.string.exporting_vault_error, e); + } else { + Toast.makeText(getContext(), getString(R.string.exported_vault), Toast.LENGTH_SHORT).show(); + } + } + } + private interface FinishExportCallback { void exportVault(OutputStream stream) throws IOException, VaultManagerException; } diff --git a/app/src/main/java/com/beemdevelopment/aegis/ui/tasks/ExportTask.java b/app/src/main/java/com/beemdevelopment/aegis/ui/tasks/ExportTask.java new file mode 100644 index 00000000..3b198ef5 --- /dev/null +++ b/app/src/main/java/com/beemdevelopment/aegis/ui/tasks/ExportTask.java @@ -0,0 +1,64 @@ +package com.beemdevelopment.aegis.ui.tasks; + +import android.content.Context; +import android.net.Uri; + +import com.beemdevelopment.aegis.R; +import com.beemdevelopment.aegis.util.IOUtils; + +import java.io.File; +import java.io.FileInputStream; +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; + +public class ExportTask extends ProgressDialogTask { + private final Callback _cb; + + public ExportTask(Context context, Callback cb) { + super(context, context.getString(R.string.exporting_vault)); + _cb = cb; + } + + @Override + protected Exception doInBackground(ExportTask.Params... args) { + setPriority(); + + ExportTask.Params params = args[0]; + try (InputStream inStream = new FileInputStream(params.getFile()); + OutputStream outStream = getDialog().getContext().getContentResolver().openOutputStream(params.getDestUri(), "w")) { + IOUtils.copy(inStream, outStream); + return null; + } catch (IOException e) { + return e; + } + } + + @Override + protected void onPostExecute(Exception e) { + super.onPostExecute(e); + _cb.onTaskFinished(e); + } + + public static class Params { + private final File _file; + private final Uri _destUri; + + public Params(File file, Uri destUri) { + _file = file; + _destUri = destUri; + } + + public File getFile() { + return _file; + } + + public Uri getDestUri() { + return _destUri; + } + } + + public interface Callback { + void onTaskFinished(Exception e); + } +} diff --git a/app/src/main/java/com/beemdevelopment/aegis/vault/VaultBackupManager.java b/app/src/main/java/com/beemdevelopment/aegis/vault/VaultBackupManager.java index e29fe62c..bf020f7d 100644 --- a/app/src/main/java/com/beemdevelopment/aegis/vault/VaultBackupManager.java +++ b/app/src/main/java/com/beemdevelopment/aegis/vault/VaultBackupManager.java @@ -9,8 +9,10 @@ import android.util.Log; import androidx.annotation.NonNull; import androidx.documentfile.provider.DocumentFile; +import com.beemdevelopment.aegis.Preferences; import com.beemdevelopment.aegis.util.IOUtils; +import java.io.File; import java.io.FileInputStream; import java.io.IOException; import java.io.OutputStream; @@ -25,6 +27,8 @@ import java.util.Comparator; import java.util.Date; import java.util.List; import java.util.Locale; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; public class VaultBackupManager { private static final String TAG = VaultBackupManager.class.getSimpleName(); @@ -34,42 +38,67 @@ public class VaultBackupManager { public static final String FILENAME_PREFIX = "aegis-backup"; - private Context _context; + private final Context _context; + private final Preferences _prefs; + private final ExecutorService _executor; public VaultBackupManager(Context context) { _context = context; + _prefs = new Preferences(context); + _executor = Executors.newSingleThreadExecutor(); } - public void create(Uri dirUri, int versionsToKeep) throws VaultManagerException { + public void destroy() { + Log.i(TAG, "Shutting down backup manager thread"); + _executor.shutdown(); + } + + public void scheduleBackup(File tempFile, Uri dirUri, int versionsToKeep) { + _executor.execute(() -> { + try { + createBackup(tempFile, dirUri, versionsToKeep); + _prefs.setBackupsError(null); + } catch (VaultManagerException e) { + e.printStackTrace(); + _prefs.setBackupsError(e); + } + }); + } + + private void createBackup(File tempFile, Uri dirUri, int versionsToKeep) throws VaultManagerException { FileInfo fileInfo = new FileInfo(FILENAME_PREFIX); DocumentFile dir = DocumentFile.fromTreeUri(_context, dirUri); - Log.i(TAG, String.format("Creating backup at %s: %s", Uri.decode(dir.getUri().toString()), fileInfo.toString())); + try { + Log.i(TAG, String.format("Creating backup at %s: %s", Uri.decode(dir.getUri().toString()), fileInfo.toString())); - if (!hasPermissionsAt(dirUri)) { - Log.e(TAG, "Unable to create file for backup, no persisted URI permissions"); - throw new VaultManagerException("No persisted URI permissions"); - } + if (!hasPermissionsAt(dirUri)) { + throw new VaultManagerException("No persisted URI permissions"); + } - // If we create a file with a name that already exists, SAF will append a number - // to the filename and write to that instead. We can't overwrite existing files, so - // just avoid that altogether by checking beforehand. - if (dir.findFile(fileInfo.toString()) != null) { - throw new VaultManagerException("Backup file already exists"); - } + // If we create a file with a name that already exists, SAF will append a number + // to the filename and write to that instead. We can't overwrite existing files, so + // just avoid that altogether by checking beforehand. + if (dir.findFile(fileInfo.toString()) != null) { + throw new VaultManagerException("Backup file already exists"); + } - DocumentFile file = dir.createFile("application/json", fileInfo.toString()); - if (file == null) { - Log.e(TAG, "Unable to create file for backup, createFile returned null"); - throw new VaultManagerException("createFile returned null"); - } + DocumentFile file = dir.createFile("application/json", fileInfo.toString()); + if (file == null) { + throw new VaultManagerException("createFile returned null"); + } - try (FileInputStream inStream = _context.openFileInput(VaultManager.FILENAME); - OutputStream outStream = _context.getContentResolver().openOutputStream(file.getUri())) { - IOUtils.copy(inStream, outStream); - } catch (IOException e) { - Log.e(TAG, "Unable to create backup", e); - throw new VaultManagerException(e); + try (FileInputStream inStream = new FileInputStream(tempFile); + OutputStream outStream = _context.getContentResolver().openOutputStream(file.getUri())) { + IOUtils.copy(inStream, outStream); + } catch (IOException e) { + throw new VaultManagerException(e); + } + } catch (VaultManagerException e) { + Log.e(TAG, String.format("Unable to create backup: %s", e.toString())); + throw e; + } finally { + tempFile.delete(); } enforceVersioning(dir, versionsToKeep); @@ -88,22 +117,20 @@ public class VaultBackupManager { private void enforceVersioning(DocumentFile dir, int versionsToKeep) { Log.i(TAG, String.format("Scanning directory %s for backup files", Uri.decode(dir.getUri().toString()))); - List files = new ArrayList<>(); + List files = new ArrayList<>(); for (DocumentFile docFile : dir.listFiles()) { if (docFile.isFile() && !docFile.isVirtual()) { try { - files.add(new File(docFile)); + files.add(new BackupFile(docFile)); } catch (ParseException ignored) { } } } - Collections.sort(files, new FileComparator()); - for (File file : files) { - Log.i(TAG, file.getFile().getName()); - } + Log.i(TAG, String.format("Found %d backup files, keeping the %d most recent", files.size(), versionsToKeep)); + Collections.sort(files, new FileComparator()); if (files.size() > versionsToKeep) { - for (File file : files.subList(0, files.size() - versionsToKeep)) { + for (BackupFile file : files.subList(0, files.size() - versionsToKeep)) { Log.i(TAG, String.format("Deleting %s", file.getFile().getName())); if (!file.getFile().delete()) { Log.e(TAG, String.format("Unable to delete %s", file.getFile().getName())); @@ -188,11 +215,11 @@ public class VaultBackupManager { } } - private static class File { + private static class BackupFile { private DocumentFile _file; private FileInfo _info; - public File(DocumentFile file) throws ParseException { + public BackupFile(DocumentFile file) throws ParseException { _file = file; _info = FileInfo.parseFilename(file.getName()); } @@ -206,9 +233,9 @@ public class VaultBackupManager { } } - private static class FileComparator implements Comparator { + private static class FileComparator implements Comparator { @Override - public int compare(File o1, File o2) { + public int compare(BackupFile o1, BackupFile o2) { return o1.getInfo().getDate().compareTo(o2.getInfo().getDate()); } } diff --git a/app/src/main/java/com/beemdevelopment/aegis/vault/VaultManager.java b/app/src/main/java/com/beemdevelopment/aegis/vault/VaultManager.java index 72fa557f..24218a4b 100644 --- a/app/src/main/java/com/beemdevelopment/aegis/vault/VaultManager.java +++ b/app/src/main/java/com/beemdevelopment/aegis/vault/VaultManager.java @@ -6,12 +6,14 @@ import androidx.core.util.AtomicFile; import com.beemdevelopment.aegis.Preferences; import com.beemdevelopment.aegis.otp.GoogleAuthInfo; +import com.beemdevelopment.aegis.util.IOUtils; import org.json.JSONObject; import java.io.File; import java.io.FileOutputStream; import java.io.IOException; +import java.io.InputStream; import java.io.OutputStream; import java.io.PrintStream; import java.nio.charset.StandardCharsets; @@ -45,19 +47,21 @@ public class VaultManager { this(context, vault, null); } + private static AtomicFile getAtomicFile(Context context) { + return new AtomicFile(new File(context.getFilesDir(), FILENAME)); + } + public static boolean fileExists(Context context) { - File file = new File(context.getFilesDir(), FILENAME); + File file = getAtomicFile(context).getBaseFile(); return file.exists() && file.isFile(); } public static void deleteFile(Context context) { - AtomicFile file = new AtomicFile(new File(context.getFilesDir(), FILENAME)); - - file.delete(); + getAtomicFile(context).delete(); } public static VaultFile readFile(Context context) throws VaultManagerException { - AtomicFile file = new AtomicFile(new File(context.getFilesDir(), FILENAME)); + AtomicFile file = getAtomicFile(context); try { byte[] fileBytes = file.readFully(); @@ -91,7 +95,7 @@ public class VaultManager { public static void save(Context context, VaultFile vaultFile) throws VaultManagerException { byte[] bytes = vaultFile.toBytes(); - AtomicFile file = new AtomicFile(new File(context.getFilesDir(), FILENAME)); + AtomicFile file = getAtomicFile(context); FileOutputStream stream = null; try { @@ -106,6 +110,10 @@ public class VaultManager { } } + public void destroy() { + _backups.destroy(); + } + public void save(boolean backup) throws VaultManagerException { try { JSONObject obj = _vault.toJson(); @@ -176,7 +184,22 @@ public class VaultManager { } public void backup() throws VaultManagerException { - _backups.create(_prefs.getBackupsLocation(), _prefs.getBackupsVersionCount()); + try { + File dir = new File(_context.getCacheDir(), "backup"); + if (!dir.exists() && !dir.mkdir()) { + throw new IOException(String.format("Unable to create directory %s", dir)); + } + + File tempFile = File.createTempFile(VaultBackupManager.FILENAME_PREFIX, ".json", dir); + try (InputStream inStream = getAtomicFile(_context).openRead(); + OutputStream outStream = new FileOutputStream(tempFile)) { + IOUtils.copy(inStream, outStream); + } + + _backups.scheduleBackup(tempFile, _prefs.getBackupsLocation(), _prefs.getBackupsVersionCount()); + } catch (IOException e) { + throw new VaultManagerException(e); + } } public void addEntry(VaultEntry entry) { diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 94a15d68..8cc7c302 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -133,6 +133,7 @@ Passwords should be identical and non-empty Please select an authentication method Encrypting the vault + Exporting the vault Delete entry Are you sure you want to delete this entry? Delete entries @@ -165,7 +166,7 @@ Are you sure you want to disable encryption? This will cause the vault to be stored in plain text. An error occurred while enabling encryption An error occurred while disabling encryption - The backup was created successfully + The backup was scheduled successfully An error occurred while trying to create a backup Permission denied New format (v0.6.3 or newer)