Close FileDescriptors and input, output streams when possible

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/OpenPgpService.java

@@ -55,6 +55,7 @@ import org.sufficientlysecure.keychain.ui.ViewKeyActivity;
 import org.sufficientlysecure.keychain.util.InputData;
 import org.sufficientlysecure.keychain.util.Log;
 
+import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.util.ArrayList;
@@ -224,6 +225,8 @@ public class OpenPgpService extends RemoteService {
     private Intent signImpl(Intent data, ParcelFileDescriptor input,
                             ParcelFileDescriptor output, AccountSettings accSettings,
                             boolean cleartextSign) {
+        InputStream is = null;
+        OutputStream os = null;
         try {
             boolean asciiArmor = cleartextSign || data.getBooleanExtra(OpenPgpApi.EXTRA_REQUEST_ASCII_ARMOR, true);
 
@@ -243,14 +246,12 @@ public class OpenPgpService extends RemoteService {
             }
 
             // Get Input- and OutputStream from ParcelFileDescriptor
-            InputStream is = new ParcelFileDescriptor.AutoCloseInputStream(input);
+            is = new ParcelFileDescriptor.AutoCloseInputStream(input);
-            OutputStream os = null;
             if (cleartextSign) {
                 // output stream only needed for cleartext signatures,
                 // detached signatures are returned as extra
                 os = new ParcelFileDescriptor.AutoCloseOutputStream(output);
             }
-            try {
             long inputLength = is.available();
             InputData inputData = new InputData(is, inputLength);
 
@@ -328,12 +329,6 @@ public class OpenPgpService extends RemoteService {
                 LogEntryParcel errorMsg = pgpResult.getLog().getLast();
                 throw new Exception(getString(errorMsg.mType.getMsgId()));
             }
-            } finally {
-                is.close();
-                if (os != null) {
-                    os.close();
-                }
-            }
         } catch (Exception e) {
             Log.d(Constants.TAG, "signImpl", e);
             Intent result = new Intent();
@@ -341,12 +336,29 @@ 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, AccountSettings accSettings,
                                       boolean sign) {
+        InputStream is = null;
+        OutputStream os = null;
         try {
             boolean asciiArmor = data.getBooleanExtra(OpenPgpApi.EXTRA_REQUEST_ASCII_ARMOR, true);
             String originalFilename = data.getStringExtra(OpenPgpApi.EXTRA_ORIGINAL_FILENAME);
@@ -372,9 +384,9 @@ public class OpenPgpService extends RemoteService {
 
             // build InputData and write into OutputStream
             // Get Input- and OutputStream from ParcelFileDescriptor
-            InputStream is = new ParcelFileDescriptor.AutoCloseInputStream(input);
+            is = new ParcelFileDescriptor.AutoCloseInputStream(input);
-            OutputStream os = new ParcelFileDescriptor.AutoCloseOutputStream(output);
+            os = new ParcelFileDescriptor.AutoCloseOutputStream(output);
-            try {
+
             long inputLength = is.available();
             InputData inputData = new InputData(is, inputLength);
 
@@ -458,11 +470,6 @@ public class OpenPgpService extends RemoteService {
                 LogEntryParcel errorMsg = pgpResult.getLog().getLast();
                 throw new Exception(getString(errorMsg.mType.getMsgId()));
             }
-
-            } finally {
-                is.close();
-                os.close();
-            }
         } catch (Exception e) {
             Log.d(Constants.TAG, "encryptAndSignImpl", e);
             Intent result = new Intent();
@@ -470,17 +477,33 @@ 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 input,
                                         ParcelFileDescriptor output, Set<Long> allowedKeyIds,
                                         boolean decryptMetadataOnly) {
+        InputStream is = null;
+        OutputStream os = null;
         try {
             // Get Input- and OutputStream from ParcelFileDescriptor
-            InputStream is = new ParcelFileDescriptor.AutoCloseInputStream(input);
+            is = new ParcelFileDescriptor.AutoCloseInputStream(input);
 
-            OutputStream os;
             // output is optional, e.g., for verifying detached signatures
             if (decryptMetadataOnly || output == null) {
                 os = null;
@@ -488,7 +511,6 @@ public class OpenPgpService extends RemoteService {
                 os = new ParcelFileDescriptor.AutoCloseOutputStream(output);
             }
 
-            try {
             String passphrase = data.getStringExtra(OpenPgpApi.EXTRA_PASSPHRASE);
             long inputLength = is.available();
             InputData inputData = new InputData(is, inputLength);
@@ -577,12 +599,7 @@ public class OpenPgpService extends RemoteService {
                 LogEntryParcel errorMsg = pgpResult.getLog().getLast();
                 throw new Exception(getString(errorMsg.mType.getMsgId()));
             }
-            } finally {
+
-                is.close();
-                if (os != null) {
-                    os.close();
-                }
-            }
         } catch (Exception e) {
             Log.d(Constants.TAG, "decryptAndVerifyImpl", e);
             Intent result = new Intent();
@@ -590,6 +607,21 @@ 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);
+                }
+            }
         }
     }
 
@@ -720,6 +752,7 @@ public class OpenPgpService extends RemoteService {
 
         @Override
         public Intent execute(Intent data, ParcelFileDescriptor input, ParcelFileDescriptor output) {
+            try {
                 Intent errorResult = checkRequirements(data);
                 if (errorResult != null) {
                     return errorResult;
@@ -763,6 +796,23 @@ public class OpenPgpService extends RemoteService {
                 } else {
                     return null;
                 }
+            } finally {
+                // always close input and output file descriptors even in error cases
+                if (input != null) {
+                    try {
+                        input.close();
+                    } catch (IOException e) {
+                        Log.e(Constants.TAG, "IOException when closing input ParcelFileDescriptor", e);
+                    }
+                }
+                if (output != null) {
+                    try {
+                        output.close();
+                    } catch (IOException e) {
+                        Log.e(Constants.TAG, "IOException when closing output ParcelFileDescriptor", e);
+                    }
+                }
+            }
         }
 
     };