openpgpservice: move stream closing into delegating method

1 files changed, 90 insertions, 131 deletions

diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/OpenPgpService.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/OpenPgpService.java
index d6ef1be85..b810f5a6a 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/OpenPgpService.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/OpenPgpService.java
@@ -25,6 +25,8 @@ import android.net.Uri;
 import android.os.IBinder;
 import android.os.ParcelFileDescriptor;
 import android.os.Parcelable;
+import android.support.annotation.NonNull;
+import android.support.annotation.Nullable;
 import android.text.TextUtils;
 
 import org.openintents.openpgp.IOpenPgpService;
@@ -239,10 +241,8 @@ public class OpenPgpService extends RemoteService {
                 PendingIntent.FLAG_CANCEL_CURRENT);
     }
 
-    private Intent signImpl(Intent data, ParcelFileDescriptor input,
-                            ParcelFileDescriptor output, boolean cleartextSign) {
-        InputStream is = null;
-        OutputStream os = null;
+    private Intent signImpl(Intent data, InputStream inputStream,
+                            OutputStream outputStream, boolean cleartextSign) {
         try {
             boolean asciiArmor = cleartextSign || data.getBooleanExtra(OpenPgpApi.EXTRA_REQUEST_ASCII_ARMOR, true);
 
@@ -278,14 +278,13 @@ public class OpenPgpService extends RemoteService {
             }
 
             // Get Input- and OutputStream from ParcelFileDescriptor
-            is = new ParcelFileDescriptor.AutoCloseInputStream(input);
-            if (cleartextSign) {
+            if (!cleartextSign) {
                 // output stream only needed for cleartext signatures,
                 // detached signatures are returned as extra
-                os = new ParcelFileDescriptor.AutoCloseOutputStream(output);
+                outputStream = null;
             }
-            long inputLength = is.available();
-            InputData inputData = new InputData(is, inputLength);
+            long inputLength = inputStream.available();
+            InputData inputData = new InputData(inputStream, inputLength);
 
             CryptoInputParcel inputParcel = CryptoInputParcelCacheService.getCryptoInputParcel(this, data);
             if (inputParcel == null) {
@@ -299,7 +298,7 @@ public class OpenPgpService extends RemoteService {
 
             // execute PGP operation!
             PgpSignEncryptOperation pse = new PgpSignEncryptOperation(this, new ProviderHelper(getContext()), null);
-            PgpSignEncryptResult pgpResult = pse.execute(pseInput, inputParcel, inputData, os);
+            PgpSignEncryptResult pgpResult = pse.execute(pseInput, inputParcel, inputData, outputStream);
 
             if (pgpResult.isPending()) {
 
@@ -331,28 +330,11 @@ public class OpenPgpService extends RemoteService {
                     new OpenPgpError(OpenPgpError.GENERIC_ERROR, e.getMessage()));
             result.putExtra(OpenPgpApi.RESULT_CODE, OpenPgpApi.RESULT_CODE_ERROR);
             return result;
-        } finally {
-            if (is != null) {
-                try {
-                    is.close();
-                } catch (IOException e) {
-                    Log.e(Constants.TAG, "IOException when closing InputStream", e);
-                }
-            }
-            if (os != null) {
-                try {
-                    os.close();
-                } catch (IOException e) {
-                    Log.e(Constants.TAG, "IOException when closing OutputStream", e);
-                }
-            }
         }
     }
 
-    private Intent encryptAndSignImpl(Intent data, ParcelFileDescriptor input,
-                                      ParcelFileDescriptor output, boolean sign) {
-        InputStream is = null;
-        OutputStream os = null;
+    private Intent encryptAndSignImpl(Intent data, InputStream inputStream,
+                                      OutputStream outputStream, boolean sign) {
         try {
             boolean asciiArmor = data.getBooleanExtra(OpenPgpApi.EXTRA_REQUEST_ASCII_ARMOR, true);
             String originalFilename = data.getStringExtra(OpenPgpApi.EXTRA_ORIGINAL_FILENAME);
@@ -384,13 +366,9 @@ public class OpenPgpService extends RemoteService {
                 }
             }
 
-            // build InputData and write into OutputStream
-            // Get Input- and OutputStream from ParcelFileDescriptor
-            is = new ParcelFileDescriptor.AutoCloseInputStream(input);
-            os = new ParcelFileDescriptor.AutoCloseOutputStream(output);
-
-            long inputLength = is.available();
-            InputData inputData = new InputData(is, inputLength, originalFilename);
+            // TODO this is not correct!
+            long inputLength = inputStream.available();
+            InputData inputData = new InputData(inputStream, inputLength, originalFilename);
 
             PgpSignEncryptInputParcel pseInput = new PgpSignEncryptInputParcel();
             pseInput.setEnableAsciiArmorOutput(asciiArmor)
@@ -456,7 +434,7 @@ public class OpenPgpService extends RemoteService {
             PgpSignEncryptOperation op = new PgpSignEncryptOperation(this, new ProviderHelper(getContext()), null);
 
             // execute PGP operation!
-            PgpSignEncryptResult pgpResult = op.execute(pseInput, inputParcel, inputData, os);
+            PgpSignEncryptResult pgpResult = op.execute(pseInput, inputParcel, inputData, outputStream);
 
             if (pgpResult.isPending()) {
                 RequiredInputParcel requiredInput = pgpResult.getRequiredInputParcel();
@@ -483,37 +461,15 @@ public class OpenPgpService extends RemoteService {
                     new OpenPgpError(OpenPgpError.GENERIC_ERROR, e.getMessage()));
             result.putExtra(OpenPgpApi.RESULT_CODE, OpenPgpApi.RESULT_CODE_ERROR);
             return result;
-        } finally {
-            if (is != null) {
-                try {
-                    is.close();
-                } catch (IOException e) {
-                    Log.e(Constants.TAG, "IOException when closing InputStream", e);
-                }
-            }
-            if (os != null) {
-                try {
-                    os.close();
-                } catch (IOException e) {
-                    Log.e(Constants.TAG, "IOException when closing OutputStream", e);
-                }
-            }
         }
     }
 
-    private Intent decryptAndVerifyImpl(Intent data, ParcelFileDescriptor inputDescriptor,
-                                        ParcelFileDescriptor output, boolean decryptMetadataOnly) {
-        InputStream inputStream = null;
-        OutputStream outputStream = null;
+    private Intent decryptAndVerifyImpl(Intent data, InputStream inputStream,
+                                        OutputStream outputStream, boolean decryptMetadataOnly) {
         try {
-            // Get Input- and OutputStream from ParcelFileDescriptor
-            inputStream = new ParcelFileDescriptor.AutoCloseInputStream(inputDescriptor);
-
             // output is optional, e.g., for verifying detached signatures
-            if (decryptMetadataOnly || output == null) {
+            if (decryptMetadataOnly) {
                 outputStream = null;
-            } else {
-                outputStream = new ParcelFileDescriptor.AutoCloseOutputStream(output);
             }
 
             String currentPkg = getCurrentCallingPackage();
@@ -539,6 +495,7 @@ public class OpenPgpService extends RemoteService {
 
             PgpDecryptVerifyOperation op = new PgpDecryptVerifyOperation(this, mProviderHelper, null);
 
+            // TODO this is not correct!
             long inputLength = inputStream.available();
             InputData inputData = new InputData(inputStream, inputLength);
 
@@ -605,6 +562,7 @@ public class OpenPgpService extends RemoteService {
                         // case RESULT_NOT_ENCRYPTED, but a signature, fallback to deprecated signatureOnly variable
                         if (decryptionResult.getResult() == OpenPgpDecryptionResult.RESULT_NOT_ENCRYPTED
                                 && signatureResult.getResult() != OpenPgpSignatureResult.RESULT_NO_SIGNATURE) {
+                            // noinspection deprecation, TODO
                             signatureResult.setSignatureOnly(true);
                         }
 
@@ -666,33 +624,14 @@ public class OpenPgpService extends RemoteService {
             result.putExtra(OpenPgpApi.RESULT_ERROR, new OpenPgpError(OpenPgpError.GENERIC_ERROR, e.getMessage()));
             result.putExtra(OpenPgpApi.RESULT_CODE, OpenPgpApi.RESULT_CODE_ERROR);
             return result;
-        } finally {
-            if (inputStream != null) {
-                try {
-                    inputStream.close();
-                } catch (IOException e) {
-                    Log.e(Constants.TAG, "IOException when closing InputStream", e);
-                }
-            }
-            if (outputStream != null) {
-                try {
-                    outputStream.close();
-                } catch (IOException e) {
-                    Log.e(Constants.TAG, "IOException when closing OutputStream", e);
-                }
-            }
         }
 
     }
 
-    private Intent getKeyImpl(Intent data, ParcelFileDescriptor output) {
+    private Intent getKeyImpl(Intent data, OutputStream outputStream) {
         try {
             long masterKeyId = data.getLongExtra(OpenPgpApi.EXTRA_KEY_ID, 0);
 
-            // output is optional, for getting the key
-            OutputStream outputStream =
-                    (output != null) ? new ParcelFileDescriptor.AutoCloseOutputStream(output) : null;
-
             try {
                 // try to find key, throws NotFoundException if not in db!
                 CanonicalizedPublicKeyRing keyRing =
@@ -846,7 +785,7 @@ public class OpenPgpService extends RemoteService {
             OpenPgpError error = new OpenPgpError
                     (OpenPgpError.INCOMPATIBLE_API_VERSIONS, "Incompatible API versions!\n"
                             + "used API version: " + data.getIntExtra(OpenPgpApi.EXTRA_API_VERSION, -1) + "\n"
-                            + "supported API versions: " + supportedVersions.toString());
+                            + "supported API versions: " + supportedVersions);
             result.putExtra(OpenPgpApi.RESULT_ERROR, error);
             result.putExtra(OpenPgpApi.RESULT_CODE, OpenPgpApi.RESULT_CODE_ERROR);
             return result;
@@ -875,68 +814,88 @@ public class OpenPgpService extends RemoteService {
         return mBinder;
     }
 
+    @Nullable
+    protected Intent executeInternal(
+            @NonNull Intent data,
+            @Nullable ParcelFileDescriptor input,
+            @Nullable ParcelFileDescriptor output) {
 
-    protected Intent executeInternal(Intent data, ParcelFileDescriptor input, ParcelFileDescriptor output) {
-        try {
-            Intent errorResult = checkRequirements(data);
-            if (errorResult != null) {
-                return errorResult;
-            }
+        OutputStream outputStream =
+                (output != null) ? new ParcelFileDescriptor.AutoCloseOutputStream(output) : null;
+        InputStream inputStream =
+                (input != null) ? new ParcelFileDescriptor.AutoCloseInputStream(input) : null;
 
-            String action = data.getAction();
-            switch (action) {
-                case OpenPgpApi.ACTION_CLEARTEXT_SIGN: {
-                    return signImpl(data, input, output, true);
-                }
-                case OpenPgpApi.ACTION_SIGN: {
-                    // DEPRECATED: same as ACTION_CLEARTEXT_SIGN
-                    Log.w(Constants.TAG, "You are using a deprecated API call, please use ACTION_CLEARTEXT_SIGN instead of ACTION_SIGN!");
-                    return signImpl(data, input, output, true);
-                }
-                case OpenPgpApi.ACTION_DETACHED_SIGN: {
-                    return signImpl(data, input, output, false);
-                }
-                case OpenPgpApi.ACTION_ENCRYPT: {
-                    return encryptAndSignImpl(data, input, output, false);
-                }
-                case OpenPgpApi.ACTION_SIGN_AND_ENCRYPT: {
-                    return encryptAndSignImpl(data, input, output, true);
-                }
-                case OpenPgpApi.ACTION_DECRYPT_VERIFY: {
-                    return decryptAndVerifyImpl(data, input, output, false);
-                }
-                case OpenPgpApi.ACTION_DECRYPT_METADATA: {
-                    return decryptAndVerifyImpl(data, input, output, true);
-                }
-                case OpenPgpApi.ACTION_GET_SIGN_KEY_ID: {
-                    return getSignKeyIdImpl(data);
-                }
-                case OpenPgpApi.ACTION_GET_KEY_IDS: {
-                    return getKeyIdsImpl(data);
-                }
-                case OpenPgpApi.ACTION_GET_KEY: {
-                    return getKeyImpl(data, output);
-                }
-                default: {
-                    return null;
-                }
-            }
+        try {
+            return executeInternalWithStreams(data, inputStream, outputStream);
         } finally {
             // always close input and output file descriptors even in error cases
-            if (input != null) {
+            if (inputStream != null) {
                 try {
-                    input.close();
+                    inputStream.close();
                 } catch (IOException e) {
                     Log.e(Constants.TAG, "IOException when closing input ParcelFileDescriptor", e);
                 }
             }
-            if (output != null) {
+            if (outputStream != null) {
                 try {
-                    output.close();
+                    outputStream.close();
                 } catch (IOException e) {
                     Log.e(Constants.TAG, "IOException when closing output ParcelFileDescriptor", e);
                 }
             }
         }
     }
+
+    @Nullable
+    protected Intent executeInternalWithStreams(
+            @NonNull Intent data,
+            @Nullable InputStream inputStream,
+            @Nullable OutputStream outputStream) {
+
+        Intent errorResult = checkRequirements(data);
+        if (errorResult != null) {
+            return errorResult;
+        }
+
+        String action = data.getAction();
+        switch (action) {
+            case OpenPgpApi.ACTION_CLEARTEXT_SIGN: {
+                return signImpl(data, inputStream, outputStream, true);
+            }
+            case OpenPgpApi.ACTION_SIGN: {
+                // DEPRECATED: same as ACTION_CLEARTEXT_SIGN
+                Log.w(Constants.TAG, "You are using a deprecated API call, please use ACTION_CLEARTEXT_SIGN instead of ACTION_SIGN!");
+                return signImpl(data, inputStream, outputStream, true);
+            }
+            case OpenPgpApi.ACTION_DETACHED_SIGN: {
+                return signImpl(data, inputStream, outputStream, false);
+            }
+            case OpenPgpApi.ACTION_ENCRYPT: {
+                return encryptAndSignImpl(data, inputStream, outputStream, false);
+            }
+            case OpenPgpApi.ACTION_SIGN_AND_ENCRYPT: {
+                return encryptAndSignImpl(data, inputStream, outputStream, true);
+            }
+            case OpenPgpApi.ACTION_DECRYPT_VERIFY: {
+                return decryptAndVerifyImpl(data, inputStream, outputStream, false);
+            }
+            case OpenPgpApi.ACTION_DECRYPT_METADATA: {
+                return decryptAndVerifyImpl(data, inputStream, outputStream, true);
+            }
+            case OpenPgpApi.ACTION_GET_SIGN_KEY_ID: {
+                return getSignKeyIdImpl(data);
+            }
+            case OpenPgpApi.ACTION_GET_KEY_IDS: {
+                return getKeyIdsImpl(data);
+            }
+            case OpenPgpApi.ACTION_GET_KEY: {
+                return getKeyImpl(data, outputStream);
+            }
+            default: {
+                return null;
+            }
+        }
+
+    }
+
 }