refactor BackupOperation a bit, make more illegal states explicit

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/BackupOperation.java

@@ -20,7 +20,6 @@ package org.sufficientlysecure.keychain.operations;
 
 
 import java.io.BufferedOutputStream;
-import java.io.DataOutputStream;
 import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.io.InputStream;
@@ -59,6 +58,7 @@ import org.sufficientlysecure.keychain.provider.TemporaryFileProvider;
 import org.sufficientlysecure.keychain.service.BackupKeyringParcel;
 import org.sufficientlysecure.keychain.service.input.CryptoInputParcel;
 import org.sufficientlysecure.keychain.ui.util.KeyFormattingUtils;
+import org.sufficientlysecure.keychain.util.CountingOutputStream;
 import org.sufficientlysecure.keychain.util.InputData;
 import org.sufficientlysecure.keychain.util.Log;
 
@@ -112,74 +112,43 @@ public class BackupOperation extends BaseOperation<BackupKeyringParcel> {
             log.add(LogType.MSG_BACKUP_ALL, 0);
         }
 
-        if (backupInput.mIsEncrypted && cryptoInput == null) {
-            throw new IllegalStateException("Encrypted backup must supply cryptoInput parameter");
-        }
-
         try {
             Uri plainUri = null;
             OutputStream plainOut;
             if (backupInput.mIsEncrypted) {
+                if (cryptoInput == null) {
+                    throw new IllegalStateException("Encrypted backup must supply cryptoInput parameter");
+                }
+
                 plainUri = TemporaryFileProvider.createFile(mContext);
                 plainOut = mContext.getContentResolver().openOutputStream(plainUri);
             } else {
-                if (backupInput.mOutputUri == null) {
+                if (backupInput.mOutputUri == null || outputStream != null) {
                     throw new IllegalArgumentException("Unencrypted export to output stream is not supported!");
                 } else {
                     plainOut = mContext.getContentResolver().openOutputStream(backupInput.mOutputUri);
                 }
             }
 
-            int exportedDataSize;
+            CountingOutputStream outStream = new CountingOutputStream(new BufferedOutputStream(plainOut));
-            { // export key data, and possibly return if we don't encrypt
+            boolean backupSuccess = exportKeysToStream(
-                DataOutputStream outStream = new DataOutputStream(new BufferedOutputStream(plainOut));
+                    log, backupInput.mMasterKeyIds, backupInput.mExportSecret, outStream);
 
-                boolean backupSuccess = exportKeysToStream(
+            if (!backupSuccess) {
-                        log, backupInput.mMasterKeyIds, backupInput.mExportSecret, outStream);
+                // if there was an error, it will be in the log so we just have to return
-
+                return new ExportResult(ExportResult.RESULT_ERROR, log);
-                exportedDataSize = outStream.size();
-
-                if (!backupSuccess) {
-                    // if there was an error, it will be in the log so we just have to return
-                    return new ExportResult(ExportResult.RESULT_ERROR, log);
-                }
-
-                if (nonEncryptedOutput) {
-                    // log.add(LogType.MSG_EXPORT_NO_ENCRYPT, 1);
-                    log.add(LogType.MSG_BACKUP_SUCCESS, 1);
-                    return new ExportResult(ExportResult.RESULT_OK, log);
-                }
             }
 
-            PgpSignEncryptOperation pseOp = new PgpSignEncryptOperation(mContext, mProviderHelper, mProgressable, mCancelled);
+            if (!backupInput.mIsEncrypted) {
-
+                // log.add(LogType.MSG_EXPORT_NO_ENCRYPT, 1);
-            PgpSignEncryptData data = new PgpSignEncryptData();
+                log.add(LogType.MSG_BACKUP_SUCCESS, 1);
-            data.setSymmetricPassphrase(cryptoInput.getPassphrase());
+                return new ExportResult(ExportResult.RESULT_OK, log);
-            data.setEnableAsciiArmorOutput(true);
-            data.setAddBackupHeader(true);
-            PgpSignEncryptInputParcel inputParcel = new PgpSignEncryptInputParcel(data);
-
-            InputStream inStream = mContext.getContentResolver().openInputStream(plainUri);
-
-            String filename;
-            if (backupInput.mMasterKeyIds != null && backupInput.mMasterKeyIds.length == 1) {
-                filename = Constants.FILE_BACKUP_PREFIX + KeyFormattingUtils.convertKeyIdToHex(backupInput.mMasterKeyIds[0]);
-            } else {
-                filename = Constants.FILE_BACKUP_PREFIX + new SimpleDateFormat("yyyy-MM-dd", Locale.getDefault()).format(new Date());
             }
-            filename += backupInput.mExportSecret ? Constants.FILE_EXTENSION_BACKUP_SECRET : Constants.FILE_EXTENSION_BACKUP_PUBLIC;
 
-            InputData inputData = new InputData(inStream, exportedDataSize, filename);
+            long exportedDataSize = outStream.getCount();
+            PgpSignEncryptResult encryptResult =
+                    encryptBackupData(backupInput, cryptoInput, outputStream, plainUri, exportedDataSize);
 
-            OutputStream outStream;
-            if (backupInput.mOutputUri == null) {
-                outStream = outputStream;
-            } else {
-                outStream = mContext.getContentResolver().openOutputStream(backupInput.mOutputUri);
-            }
-            outStream = new BufferedOutputStream(outStream);
-
-            PgpSignEncryptResult encryptResult = pseOp.execute(inputParcel, new CryptoInputParcel(), inputData, outStream);
             if (!encryptResult.success()) {
                 log.addByMerge(encryptResult, 1);
                 // log.add(LogType.MSG_EXPORT_ERROR_ENCRYPT, 1);
@@ -193,13 +162,52 @@ public class BackupOperation extends BaseOperation<BackupKeyringParcel> {
         } catch (FileNotFoundException e) {
             log.add(LogType.MSG_BACKUP_ERROR_URI_OPEN, 1);
             return new ExportResult(ExportResult.RESULT_ERROR, log);
-
         }
 
     }
 
-    boolean exportKeysToStream(OperationLog log, long[] masterKeyIds, boolean exportSecret, OutputStream outStream) {
+    @NonNull
+    private PgpSignEncryptResult encryptBackupData(@NonNull BackupKeyringParcel backupInput,
+            @NonNull CryptoInputParcel cryptoInput, @Nullable OutputStream outputStream, Uri plainUri, long exportedDataSize)
+            throws FileNotFoundException {
+        PgpSignEncryptOperation signEncryptOperation = new PgpSignEncryptOperation(mContext, mProviderHelper, mProgressable, mCancelled);
 
+        PgpSignEncryptData data = new PgpSignEncryptData();
+        data.setSymmetricPassphrase(cryptoInput.getPassphrase());
+        data.setEnableAsciiArmorOutput(true);
+        data.setAddBackupHeader(true);
+        PgpSignEncryptInputParcel inputParcel = new PgpSignEncryptInputParcel(data);
+
+        InputStream inStream = mContext.getContentResolver().openInputStream(plainUri);
+
+        String filename;
+        if (backupInput.mMasterKeyIds != null && backupInput.mMasterKeyIds.length == 1) {
+            filename = Constants.FILE_BACKUP_PREFIX + KeyFormattingUtils.convertKeyIdToHex(backupInput.mMasterKeyIds[0]);
+        } else {
+            filename = Constants.FILE_BACKUP_PREFIX + new SimpleDateFormat("yyyy-MM-dd", Locale
+                    .getDefault()).format(new Date());
+        }
+        filename += backupInput.mExportSecret ? Constants.FILE_EXTENSION_BACKUP_SECRET : Constants.FILE_EXTENSION_BACKUP_PUBLIC;
+
+        InputData inputData = new InputData(inStream, exportedDataSize, filename);
+
+        OutputStream outStream;
+        if (backupInput.mOutputUri == null) {
+            if (outputStream == null) {
+                throw new IllegalArgumentException("If output uri is not set, outputStream must not be null!");
+            }
+            outStream = outputStream;
+        } else {
+            if (outputStream != null) {
+                throw new IllegalArgumentException("If output uri is set, outputStream must null!");
+            }
+            outStream = mContext.getContentResolver().openOutputStream(backupInput.mOutputUri);
+        }
+
+        return signEncryptOperation.execute(inputParcel, new CryptoInputParcel(), inputData, outStream);
+    }
+
+    boolean exportKeysToStream(OperationLog log, long[] masterKeyIds, boolean exportSecret, OutputStream outStream) {
         // noinspection unused TODO use these in a log entry
         int okSecret = 0, okPublic = 0;
 
@@ -257,12 +265,10 @@ public class BackupOperation extends BaseOperation<BackupKeyringParcel> {
         }
 
         return true;
-
     }
 
     private boolean writePublicKeyToStream(OperationLog log, OutputStream outStream, Cursor cursor)
             throws IOException {
-
         ArmoredOutputStream arOutStream = null;
 
         try {
@@ -283,7 +289,6 @@ public class BackupOperation extends BaseOperation<BackupKeyringParcel> {
 
     private boolean writeSecretKeyToStream(OperationLog log, OutputStream outStream, Cursor cursor)
             throws IOException {
-
         ArmoredOutputStream arOutStream = null;
 
         try {
@@ -303,7 +308,6 @@ public class BackupOperation extends BaseOperation<BackupKeyringParcel> {
     }
 
     private Cursor queryForKeys(long[] masterKeyIds) {
-
         String selection = null, selectionArgs[] = null;
 
         if (masterKeyIds != null) {
@@ -326,7 +330,6 @@ public class BackupOperation extends BaseOperation<BackupKeyringParcel> {
                 KeyRings.buildUnifiedKeyRingsUri(), PROJECTION, selection, selectionArgs,
                 Tables.KEYS + "." + KeyRings.MASTER_KEY_ID
         );
-
     }
 
 }

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/util/CountingOutputStream.java

@@ -0,0 +1,65 @@
+/*
+ * Copyright (C) 2007 The Guava Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
+ * in compliance with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package org.sufficientlysecure.keychain.util;
+
+
+import java.io.FilterOutputStream;
+import java.io.IOException;
+import java.io.OutputStream;
+
+import android.support.annotation.NonNull;
+
+
+public final class CountingOutputStream extends FilterOutputStream {
+
+    private long count;
+
+    /**
+     * Wraps another output stream, counting the number of bytes written.
+     *
+     * @param out
+     *         the output stream to be wrapped
+     */
+    public CountingOutputStream(@NonNull OutputStream out) {
+        super(out);
+    }
+
+    /**
+     * Returns the number of bytes written.
+     */
+    public long getCount() {
+        return count;
+    }
+
+    @Override
+    public void write(@NonNull byte[] b, int off, int len) throws IOException {
+        out.write(b, off, len);
+        count += len;
+    }
+
+    @Override
+    public void write(int b) throws IOException {
+        out.write(b);
+        count++;
+    }
+
+    // Overriding close() because FilterOutputStream's close() method pre-JDK8 has bad behavior:
+    // it silently ignores any exception thrown by flush(). Instead, just close the delegate stream.
+    // It should flush itself if necessary.
+    @Override
+    public void close() throws IOException {
+        out.close();
+    }
+}

OpenKeychain/src/test/java/org/sufficientlysecure/keychain/operations/ExportTest.java

@@ -242,7 +242,6 @@ public class ExportTest {
 
     @Test
     public void testExportUnencrypted() throws Exception {
-
         ContentResolver mockResolver = mock(ContentResolver.class);
 
         Uri fakeOutputUri = Uri.parse("content://fake/out/1");
@@ -256,7 +255,7 @@ public class ExportTest {
                 new ProviderHelper(RuntimeEnvironment.application), null);
 
         BackupKeyringParcel parcel = new BackupKeyringParcel(
-                new long[] { mStaticRing1.getMasterKeyId() }, false, fakeOutputUri);
+                new long[] { mStaticRing1.getMasterKeyId() }, false, false, fakeOutputUri);
 
         ExportResult result = op.execute(parcel, null);
 
@@ -284,8 +283,6 @@ public class ExportTest {
 
     @Test
     public void testExportEncrypted() throws Exception {
-
-
         Application spyApplication;
         ContentResolver mockResolver = mock(ContentResolver.class);
 
@@ -315,7 +312,7 @@ public class ExportTest {
                     new ProviderHelper(RuntimeEnvironment.application), null);
 
             BackupKeyringParcel parcel = new BackupKeyringParcel(
-                    new long[] { mStaticRing1.getMasterKeyId() }, false, fakeOutputUri);
+                    new long[] { mStaticRing1.getMasterKeyId() }, false, true, fakeOutputUri);
             CryptoInputParcel inputParcel = new CryptoInputParcel(passphrase);
             ExportResult result = op.execute(parcel, inputParcel);