Merge pull request #159 from owncloud/retry_transfers_after_lost_network

Improve support for unexpected loss of network connection.

ant.properties

@@ -0,0 +1,8 @@
+# This file contains custom properties used by the Ant build system.
+#
+# This file must be checked in Version Control Systems.
+#
+
+# Java version options
+java.source=1.7
+java.target=1.7

src/com/owncloud/android/lib/common/network/AdvancedSslSocketFactory.java

@@ -50,7 +50,6 @@ import org.apache.http.conn.ssl.X509HostnameVerifier;
 import com.owncloud.android.lib.common.utils.Log_OC;
 
 
-
 /**
  * AdvancedSSLProtocolSocketFactory allows to create SSL {@link Socket}s with
  * a custom SSLContext and an optional Hostname Verifier.
@@ -90,7 +89,7 @@ public class AdvancedSslSocketFactory implements SecureProtocolSocketFactory {
     }
 
     /**
-     * @see ProtocolSocketFactory#createSocket(java.lang.String,int,java.net.InetAddress,int)
+     * @see ProtocolSocketFactory#createSocket(java.lang.String, int, java.net.InetAddress, int)
      */
     @Override
     public Socket createSocket(String host, int port, InetAddress clientHost, int clientPort)
@@ -148,12 +147,10 @@ public class AdvancedSslSocketFactory implements SecureProtocolSocketFactory {
      *
      * @param host         the host name/IP
      * @param port         the port on the host
-     * @param clientHost the local host name/IP to bind the socket to
+     * @param localAddress the local host name/IP to bind the socket to
-     * @param clientPort the port on the local machine
+     * @param localPort    the port on the local machine
      * @param params       {@link HttpConnectionParams Http connection parameters}
-     * 
      * @return Socket a new socket
-     * 
      * @throws IOException          if an I/O error occurs while creating the socket
      * @throws UnknownHostException if the IP address of the host cannot be
      *                              determined
@@ -179,15 +176,16 @@ public class AdvancedSslSocketFactory implements SecureProtocolSocketFactory {
         SocketAddress localaddr = new InetSocketAddress(localAddress, localPort);
         SocketAddress remoteaddr = new InetSocketAddress(host, port);
         socket.setSoTimeout(params.getSoTimeout());
+        WriteTimeoutEnforcer.setSoWriteTimeout(params.getSoTimeout(), socket);
         socket.bind(localaddr);
-        ServerNameIndicator.setServerNameIndication(host, (SSLSocket)socket);
+        ServerNameIndicator.setServerNameIndication(host, (SSLSocket) socket);
         socket.connect(remoteaddr, timeout);
         verifyPeerIdentity(host, port, socket);
         return socket;
     }
 
     /**
-     * @see ProtocolSocketFactory#createSocket(java.lang.String,int)
+     * @see ProtocolSocketFactory#createSocket(java.lang.String, int)
      */
     @Override
     public Socket createSocket(String host, int port) throws IOException,
@@ -236,6 +234,7 @@ public class AdvancedSslSocketFactory implements SecureProtocolSocketFactory {
      *
      * Then, the host name is compared with the content of the server certificate using the current host name verifier,
      * if any.
+     *
      * @param socket
      */
     private void verifyPeerIdentity(String host, int port, Socket socket) throws IOException {
@@ -254,14 +253,14 @@ public class AdvancedSslSocketFactory implements SecureProtocolSocketFactory {
                 } else {
                     Throwable cause = e.getCause();
                     Throwable previousCause = null;
-                    while (	cause != null && 
+                    while (cause != null &&
                         cause != previousCause &&
                         !(cause instanceof CertificateCombinedException)) {
                         previousCause = cause;
                         cause = cause.getCause();
                     }
                     if (cause != null && cause instanceof CertificateCombinedException) {
-                        failInHandshake = (CertificateCombinedException)cause;
+                        failInHandshake = (CertificateCombinedException) cause;
                     }
                 }
                 if (failInHandshake == null) {
@@ -286,8 +285,8 @@ public class AdvancedSslSocketFactory implements SecureProtocolSocketFactory {
 
                 } else {
                     /// 2.2 : a new SSLSession instance was created in the handshake
-                    newSession = ((SSLSocket)socket).getSession();
+                    newSession = ((SSLSocket) socket).getSession();
-                    if (!mTrustManager.isKnownServer((X509Certificate)(newSession.getPeerCertificates()[0]))) {
+                    if (!mTrustManager.isKnownServer((X509Certificate) (newSession.getPeerCertificates()[0]))) {
                         verifiedHostname = mHostnameVerifier.verify(host, newSession);
                     }
                 }
@@ -338,7 +337,7 @@ public class AdvancedSslSocketFactory implements SecureProtocolSocketFactory {
      */
     private void enableSecureProtocols(Socket socket) {
         SSLParameters params = mSslContext.getSupportedSSLParameters();
-    	String [] supportedProtocols = params.getProtocols();
+        String[] supportedProtocols = params.getProtocols();
         ((SSLSocket) socket).setEnabledProtocols(supportedProtocols);
     }
 

src/com/owncloud/android/lib/common/network/WriteTimeoutEnforcer.java

@@ -0,0 +1,173 @@
+/* ownCloud Android Library is available under MIT license
+ *   Copyright (C) 2017 ownCloud GmbH.
+ *   
+ *   Permission is hereby granted, free of charge, to any person obtaining a copy
+ *   of this software and associated documentation files (the "Software"), to deal
+ *   in the Software without restriction, including without limitation the rights
+ *   to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ *   copies of the Software, and to permit persons to whom the Software is
+ *   furnished to do so, subject to the following conditions:
+ *   
+ *   The above copyright notice and this permission notice shall be included in
+ *   all copies or substantial portions of the Software.
+ *   
+ *   THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, 
+ *   EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ *   MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND 
+ *   NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS 
+ *   BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN 
+ *   ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN 
+ *   CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ *   THE SOFTWARE.
+ *
+ */
+
+package com.owncloud.android.lib.common.network;
+
+import com.owncloud.android.lib.common.utils.Log_OC;
+
+import java.lang.ref.WeakReference;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.net.Socket;
+import java.util.concurrent.atomic.AtomicReference;
+
+
+/**
+ * Enforces, if possible, a write timeout for a socket.
+ *
+ * Built as a singleton.
+ *
+ * Tries to hit something like this:
+ * https://android.googlesource.com/platform/external/conscrypt/+/lollipop-release/src/main/java/org/conscrypt/OpenSSLSocketImpl.java#1005
+ *
+ * Minimizes the chances of getting stalled in PUT/POST request if the network interface is lost while
+ * writing the entity into the outwards sockect.
+ *
+ * It happens. See https://github.com/owncloud/android/issues/1684#issuecomment-295306015
+ *
+ * @author David A. Velasco
+ */
+public class WriteTimeoutEnforcer {
+
+    private static final String TAG = WriteTimeoutEnforcer.class.getSimpleName();
+
+    private static final AtomicReference<WriteTimeoutEnforcer> mSingleInstance = new AtomicReference<>();
+
+    private static final String METHOD_NAME = "setSoWriteTimeout";
+
+
+    private final WeakReference<Class<?>> mSocketClassRef;
+    private final WeakReference<Method> mSetSoWriteTimeoutMethodRef;
+
+
+    /**
+     * Private constructor, class is a singleton.
+     *
+     * @param socketClass               Underlying implementation class of {@link Socket} used to connect
+     *                                  with the server.
+     * @param setSoWriteTimeoutMethod   Name of the method to call to set a write timeout in the socket.
+     */
+    private WriteTimeoutEnforcer(Class<?> socketClass, Method setSoWriteTimeoutMethod) {
+        mSocketClassRef = new WeakReference<Class<?>>(socketClass);
+        mSetSoWriteTimeoutMethodRef =
+            (setSoWriteTimeoutMethod == null) ?
+                null :
+                new WeakReference<>(setSoWriteTimeoutMethod)
+        ;
+    }
+
+
+    /**
+     * Calls the {@code #setSoWrite(int)} method of the underlying implementation
+     * of {@link Socket} if exists.
+
+     * Creates and initializes the single instance of the class when needed
+     *
+     * @param writeTimeoutMilliseconds  Write timeout to set, in milliseconds.
+     * @param socket                    Client socket to connect with the server.
+     */
+    public static void setSoWriteTimeout(int writeTimeoutMilliseconds, Socket socket) {
+        final Method setSoWriteTimeoutMethod = getMethod(socket);
+        if (setSoWriteTimeoutMethod != null) {
+            try {
+                setSoWriteTimeoutMethod.invoke(socket, writeTimeoutMilliseconds);
+                Log_OC.i(
+                    TAG,
+                    "Write timeout set in socket, writeTimeoutMilliseconds: "
+                        + writeTimeoutMilliseconds
+                );
+
+            } catch (IllegalArgumentException e) {
+                Log_OC.e(TAG, "Call to (SocketImpl)#setSoWriteTimeout(int) failed ", e);
+
+            } catch (IllegalAccessException e) {
+                Log_OC.e(TAG, "Call to (SocketImpl)#setSoWriteTimeout(int) failed ", e);
+
+            } catch (InvocationTargetException e) {
+                Log_OC.e(TAG, "Call to (SocketImpl)#setSoWriteTimeout(int) failed ", e);
+            }
+        } else {
+            Log_OC.i(TAG, "Write timeout for socket not supported");
+        }
+    }
+
+
+    /**
+     * Gets the method to invoke trying to minimize the cost of reflection reusing objects cached
+     * in static members.
+     *
+     * @param socket    Instance of the socket to use in connection with server.
+     * @return          Method to call to set a write timeout in the socket.
+     */
+    private static Method getMethod(Socket socket) {
+        final Class<?> socketClass = socket.getClass();
+        final WriteTimeoutEnforcer instance = mSingleInstance.get();
+        if (instance == null) {
+            return initFrom(socketClass);
+
+        } else if (instance.mSocketClassRef.get() != socketClass) {
+            // the underlying class changed
+            return initFrom(socketClass);
+
+        } else if (instance.mSetSoWriteTimeoutMethodRef == null) {
+            // method not supported
+            return null;
+
+        } else {
+            final Method cachedSetSoWriteTimeoutMethod = instance.mSetSoWriteTimeoutMethodRef.get();
+            return (cachedSetSoWriteTimeoutMethod == null) ?
+                initFrom(socketClass) :
+                cachedSetSoWriteTimeoutMethod
+            ;
+        }
+    }
+
+
+    /**
+     * Singleton initializer.
+     *
+     * Uses reflection to extract and 'cache' the method to invoke to set a write timouet in a socket.
+     *
+     * @param socketClass   Underlying class providing the implementation of {@link Socket}.
+     * @return              Method to call to set a write timeout in the socket.
+     */
+    private static Method initFrom(Class<?> socketClass) {
+        Log_OC.i(TAG, "Socket implementation: " + socketClass.getCanonicalName());
+        Method setSoWriteTimeoutMethod = null;
+        try {
+            setSoWriteTimeoutMethod = socketClass.getMethod(METHOD_NAME, int.class);
+        } catch (SecurityException e) {
+            Log_OC.e(TAG, "Could not access to (SocketImpl)#setSoWriteTimeout(int) method ", e);
+
+        } catch (NoSuchMethodException e) {
+            Log_OC.i(
+                TAG,
+                "Could not find (SocketImpl)#setSoWriteTimeout(int) method - write timeout not supported"
+            );
+        }
+        mSingleInstance.set(new WriteTimeoutEnforcer(socketClass, setSoWriteTimeoutMethod));
+        return setSoWriteTimeoutMethod;
+    }
+
+}

src/com/owncloud/android/lib/resources/files/DownloadRemoteFileOperation.java

@@ -35,7 +35,6 @@ import java.util.Set;
 import java.util.concurrent.atomic.AtomicBoolean;
 
 import org.apache.commons.httpclient.Header;
-import org.apache.commons.httpclient.HttpException;
 import org.apache.commons.httpclient.HttpStatus;
 import org.apache.commons.httpclient.methods.GetMethod;
 
@@ -60,7 +59,7 @@ public class DownloadRemoteFileOperation extends RemoteOperation {
     private static final int FORBIDDEN_ERROR = 403;
     private static final int SERVICE_UNAVAILABLE_ERROR = 503;
 
-    private Set<OnDatatransferProgressListener> mDataTransferListeners = new HashSet<OnDatatransferProgressListener>();
+    private Set<OnDatatransferProgressListener> mDataTransferListeners = new HashSet<>();
     private final AtomicBoolean mCancellationRequested = new AtomicBoolean(false);
     private long mModificationTimestamp = 0;
     private String mEtag = "";
@@ -76,7 +75,7 @@ public class DownloadRemoteFileOperation extends RemoteOperation {
 
     @Override
     protected RemoteOperationResult run(OwnCloudClient client) {
-        RemoteOperationResult result = null;
+        RemoteOperationResult result;
 
         /// download will be performed to a temporal file, then moved to the final location
         File tmpFile = new File(getTmpPath());
@@ -102,17 +101,18 @@ public class DownloadRemoteFileOperation extends RemoteOperation {
         IOException, OperationCancelledException {
 
         RemoteOperationResult result;
-        int status = -1;
+        int status;
         boolean savedFile = false;
         mGet = new GetMethod(client.getWebdavUri() + WebdavUtils.encodePath(mRemotePath));
-        Iterator<OnDatatransferProgressListener> it = null;
+        Iterator<OnDatatransferProgressListener> it;
 
         FileOutputStream fos = null;
+        BufferedInputStream bis = null;
         try {
             status = client.executeMethod(mGet);
             if (isSuccess(status)) {
                 targetFile.createNewFile();
-                BufferedInputStream bis = new BufferedInputStream(mGet.getResponseBodyAsStream());
+                bis = new BufferedInputStream(mGet.getResponseBodyAsStream());
                 fos = new FileOutputStream(targetFile);
                 long transferred = 0;
 
@@ -122,7 +122,7 @@ public class DownloadRemoteFileOperation extends RemoteOperation {
                     Long.parseLong(contentLength.getValue()) : 0;
 
                 byte[] bytes = new byte[4096];
-                int readResult = 0;
+                int readResult;
                 while ((readResult = bis.read(bytes)) != -1) {
                     synchronized (mCancellationRequested) {
                         if (mCancellationRequested.get()) {
@@ -147,7 +147,7 @@ public class DownloadRemoteFileOperation extends RemoteOperation {
                         modificationTime = mGet.getResponseHeader("last-modified");
                     }
                     if (modificationTime != null) {
-                        Date d = WebdavUtils.parseResponseDate((String) modificationTime.getValue());
+                        Date d = WebdavUtils.parseResponseDate(modificationTime.getValue());
                         mModificationTimestamp = (d != null) ? d.getTime() : 0;
                     } else {
                         Log_OC.e(TAG, "Could not read modification time from response downloading " + mRemotePath);
@@ -163,15 +163,16 @@ public class DownloadRemoteFileOperation extends RemoteOperation {
                     // TODO some kind of error control!
                 }
 
-            } else if (status != FORBIDDEN_ERROR && status != SERVICE_UNAVAILABLE_ERROR){
+            } else if (status != FORBIDDEN_ERROR && status != SERVICE_UNAVAILABLE_ERROR) {
                 client.exhaustResponse(mGet.getResponseBodyAsStream());
 
-            } // else, body read by RemoteOeprationResult constructor
+            } // else, body read by RemoteOperationResult constructor
 
             result = new RemoteOperationResult(isSuccess(status), mGet);
 
         } finally {
             if (fos != null) fos.close();
+            if (bis != null) bis.close();
             if (!savedFile && targetFile.exists()) {
                 targetFile.delete();
             }