Merge pull request #5887 from nextcloud/transfer

close SSO streams

scripts/analysis/findbugs-results.txt

@@ -1 +1 @@
-384
+381

src/main/java/com/nextcloud/android/sso/InputStreamBinder.java

@@ -104,16 +104,6 @@ public class InputStreamBinder extends IInputStreamService.Stub {
         this.accountManager = accountManager;
     }
 
-    private NameValuePair[] convertMapToNVP(Map<String, String> map) {
-        NameValuePair[] nvp = new NameValuePair[map.size()];
-        int i = 0;
-        for (String key : map.keySet()) {
-            nvp[i] = new NameValuePair(key, map.get(key));
-            i++;
-        }
-        return nvp;
-    }
-
     public ParcelFileDescriptor performNextcloudRequestV2(ParcelFileDescriptor input) {
         return performNextcloudRequestAndBodyStreamV2(input, null);
     }
@@ -143,7 +133,9 @@ public class InputStreamBinder extends IInputStreamService.Stub {
             InputStream exceptionStream = serializeObjectToInputStreamV2(exception, response.getPlainHeadersString());
             InputStream resultStream = new java.io.SequenceInputStream(exceptionStream, response.getBody());
 
-            return ParcelFileDescriptorUtil.pipeFrom(resultStream, thread -> Log.d(TAG, "Done sending result"));
+            return ParcelFileDescriptorUtil.pipeFrom(resultStream,
+                                                     thread -> Log.d(TAG, "Done sending result"),
+                                                     response.getMethod());
         } catch (IOException e) {
             Log_OC.e(TAG, "Error while sending response back to client app", e);
         }
@@ -163,6 +155,7 @@ public class InputStreamBinder extends IInputStreamService.Stub {
         final InputStream requestBodyInputStream = requestBodyParcelFileDescriptor != null ?
             new ParcelFileDescriptor.AutoCloseInputStream(requestBodyParcelFileDescriptor) : null;
         Exception exception = null;
+        HttpMethodBase httpMethod = null;
         InputStream httpStream = new InputStream() {
             @Override
             public int read() {
@@ -173,7 +166,8 @@ public class InputStreamBinder extends IInputStreamService.Stub {
         try {
             // Start request and catch exceptions
             NextcloudRequest request = deserializeObjectAndCloseStream(is);
-            httpStream = processRequest(request, requestBodyInputStream);
+            httpMethod = processRequest(request, requestBodyInputStream);
+            httpStream = httpMethod.getResponseBodyAsStream();
         } catch (Exception e) {
             Log_OC.e(TAG, "Error during Nextcloud request", e);
             exception = e;
@@ -188,7 +182,9 @@ public class InputStreamBinder extends IInputStreamService.Stub {
             } else {
                 resultStream = exceptionStream;
             }
-            return ParcelFileDescriptorUtil.pipeFrom(resultStream, thread -> Log.d(TAG, "Done sending result"));
+            return ParcelFileDescriptorUtil.pipeFrom(resultStream,
+                                                     thread -> Log.d(TAG, "Done sending result"),
+                                                     httpMethod);
         } catch (IOException e) {
             Log_OC.e(TAG, "Error while sending response back to client app", e);
         }
@@ -309,7 +305,7 @@ public class InputStreamBinder extends IInputStreamService.Stub {
         return method;
     }
 
-    private InputStream processRequest(final NextcloudRequest request, final InputStream requestBodyInputStream)
+    private HttpMethodBase processRequest(final NextcloudRequest request, final InputStream requestBodyInputStream)
         throws UnsupportedOperationException,
         com.owncloud.android.lib.common.accounts.AccountUtils.AccountNotFoundException,
         OperationCanceledException, AuthenticatorException, IOException {
@@ -354,23 +350,21 @@ public class InputStreamBinder extends IInputStreamService.Stub {
 
         // Check if status code is 2xx --> https://en.wikipedia.org/wiki/List_of_HTTP_status_codes#2xx_Success
         if (status >= HTTP_STATUS_CODE_OK && status < HTTP_STATUS_CODE_MULTIPLE_CHOICES) {
-            return method.getResponseBodyAsStream();
+            return method;
         } else {
-            StringBuilder total = new StringBuilder();
             InputStream inputStream = method.getResponseBodyAsStream();
+            String total = "No response body";
+
             // If response body is available
             if (inputStream != null) {
-                BufferedReader reader = new BufferedReader(new InputStreamReader(inputStream));
-                String line = reader.readLine();
-                while (line != null) {
-                    total.append(line).append('\n');
-                    line = reader.readLine();
-                }
-                Log_OC.e(TAG, total.toString());
+                total = inputStreamToString(inputStream);
+                Log_OC.e(TAG, total);
             }
+
+            method.releaseConnection();
             throw new IllegalStateException(EXCEPTION_HTTP_REQUEST_FAILED,
                                             new IllegalStateException(String.valueOf(status),
-                                                                      new IllegalStateException(total.toString())));
+                                                                      new IllegalStateException(total)));
         }
     }
 
@@ -419,23 +413,21 @@ public class InputStreamBinder extends IInputStreamService.Stub {
 
         // Check if status code is 2xx --> https://en.wikipedia.org/wiki/List_of_HTTP_status_codes#2xx_Success
         if (status >= HTTP_STATUS_CODE_OK && status < HTTP_STATUS_CODE_MULTIPLE_CHOICES) {
-            return new Response(method.getResponseBodyAsStream(), method.getResponseHeaders());
+            return new Response(method);
         } else {
-            StringBuilder total = new StringBuilder();
             InputStream inputStream = method.getResponseBodyAsStream();
+            String total = "No response body";
+
             // If response body is available
             if (inputStream != null) {
-                BufferedReader reader = new BufferedReader(new InputStreamReader(inputStream));
-                String line = reader.readLine();
-                while (line != null) {
-                    total.append(line).append('\n');
-                    line = reader.readLine();
-                }
-                Log_OC.e(TAG, total.toString());
+                total = inputStreamToString(inputStream);
+                Log_OC.e(TAG, total);
             }
+
+            method.releaseConnection();
             throw new IllegalStateException(EXCEPTION_HTTP_REQUEST_FAILED,
                                             new IllegalStateException(String.valueOf(status),
-                                                                      new IllegalStateException(total.toString())));
+                                                                      new IllegalStateException(total)));
         }
     }
 
@@ -474,4 +466,29 @@ public class InputStreamBinder extends IInputStreamService.Stub {
         }
         return result == 0;
     }
+
+    private static String inputStreamToString(InputStream inputStream) {
+        try {
+            StringBuilder total = new StringBuilder();
+            BufferedReader reader = new BufferedReader(new InputStreamReader(inputStream));
+            String line = reader.readLine();
+            while (line != null) {
+                total.append(line).append('\n');
+                line = reader.readLine();
+            }
+            return total.toString();
+        } catch (Exception e) {
+            return e.getMessage();
+        }
+    }
+
+    private static NameValuePair[] convertMapToNVP(Map<String, String> map) {
+        NameValuePair[] nvp = new NameValuePair[map.size()];
+        int i = 0;
+        for (String key : map.keySet()) {
+            nvp[i] = new NameValuePair(key, map.get(key));
+            i++;
+        }
+        return nvp;
+    }
 }

src/main/java/com/nextcloud/android/sso/Response.java

@@ -25,7 +25,9 @@ package com.nextcloud.android.sso;
 import com.google.gson.Gson;
 
 import org.apache.commons.httpclient.Header;
+import org.apache.commons.httpclient.HttpMethodBase;
 
+import java.io.IOException;
 import java.io.InputStream;
 import java.util.ArrayList;
 import java.util.List;
@@ -33,6 +35,7 @@ import java.util.List;
 public class Response {
     private InputStream body;
     private Header[] headers;
+    private HttpMethodBase method;
 
     public Response() {
         headers = new Header[0];
@@ -44,9 +47,10 @@ public class Response {
         };
     }
 
-    public Response(InputStream inputStream, Header[] headers) {
-        this.body = inputStream;
-        this.headers = headers;
+    public Response(HttpMethodBase methodBase) throws IOException {
+        this.method = methodBase;
+        this.body = methodBase.getResponseBodyAsStream();
+        this.headers = methodBase.getResponseHeaders();
     }
 
     public String getPlainHeadersString() {
@@ -63,4 +67,8 @@ public class Response {
     public InputStream getBody() {
         return this.body;
     }
+
+    public HttpMethodBase getMethod() {
+        return method;
+    }
 }

src/main/java/com/nextcloud/android/sso/aidl/ParcelFileDescriptorUtil.java

@@ -24,6 +24,8 @@ import android.util.Log;
 
 import com.owncloud.android.lib.common.utils.Log_OC;
 
+import org.apache.commons.httpclient.HttpMethodBase;
+
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
@@ -32,45 +34,37 @@ public final class ParcelFileDescriptorUtil {
 
     private ParcelFileDescriptorUtil() { }
 
-    public static ParcelFileDescriptor pipeFrom(InputStream inputStream, IThreadListener listener)
+    public static ParcelFileDescriptor pipeFrom(InputStream inputStream,
+                                                IThreadListener listener,
+                                                HttpMethodBase method)
             throws IOException {
         ParcelFileDescriptor[] pipe = ParcelFileDescriptor.createPipe();
         ParcelFileDescriptor readSide = pipe[0];
         ParcelFileDescriptor writeSide = pipe[1];
 
         // start the transfer thread
-        new TransferThread(inputStream, new ParcelFileDescriptor.AutoCloseOutputStream(writeSide),
-                listener)
+        new TransferThread(inputStream,
+                           new ParcelFileDescriptor.AutoCloseOutputStream(writeSide),
+                           listener,
+                           method)
                 .start();
 
         return readSide;
     }
 
-    public static ParcelFileDescriptor pipeTo(OutputStream outputStream, IThreadListener listener)
-            throws IOException {
-        ParcelFileDescriptor[] pipe = ParcelFileDescriptor.createPipe();
-        ParcelFileDescriptor readSide = pipe[0];
-        ParcelFileDescriptor writeSide = pipe[1];
-
-        // start the transfer thread
-        new TransferThread(new ParcelFileDescriptor.AutoCloseInputStream(readSide), outputStream,
-                listener)
-                .start();
-
-        return writeSide;
-    }
-
     public static class TransferThread extends Thread {
         private static final String TAG = TransferThread.class.getCanonicalName();
-        private final InputStream mIn;
-        private final OutputStream mOut;
-        private final IThreadListener mListener;
+        private final InputStream inputStream;
+        private final OutputStream outputStream;
+        private final IThreadListener threadListener;
+        private final HttpMethodBase httpMethod;
 
-        TransferThread(InputStream in, OutputStream out, IThreadListener listener) {
+        TransferThread(InputStream in, OutputStream out, IThreadListener listener, HttpMethodBase method) {
             super("ParcelFileDescriptor Transfer Thread");
-            mIn = in;
-            mOut = out;
-            mListener = listener;
+            inputStream = in;
+            outputStream = out;
+            threadListener = listener;
+            httpMethod = method;
             setDaemon(true);
         }
 
@@ -80,26 +74,31 @@ public final class ParcelFileDescriptorUtil {
             int len;
 
             try {
-                while ((len = mIn.read(buf)) > 0) {
-                    mOut.write(buf, 0, len);
+                while ((len = inputStream.read(buf)) > 0) {
+                    outputStream.write(buf, 0, len);
                 }
-                mOut.flush(); // just to be safe
+                outputStream.flush(); // just to be safe
             } catch (IOException e) {
-                Log.e("TransferThread", "writing failed");
+                Log_OC.e(TAG, "writing failed: " + e.getMessage());
             } finally {
                 try {
-                    mIn.close();
+                    inputStream.close();
                 } catch (IOException e) {
                     Log_OC.e(TAG, e.getMessage());
                 }
                 try {
-                    mOut.close();
+                    outputStream.close();
                 } catch (IOException e) {
                     Log_OC.e(TAG, e.getMessage());
                 }
             }
-            if (mListener != null) {
-                mListener.onThreadFinished(this);
+            if (threadListener != null) {
+                threadListener.onThreadFinished(this);
+            }
+
+            if (httpMethod != null) {
+                Log_OC.i(TAG, "releaseConnection");
+                httpMethod.releaseConnection();
             }
         }
     }