Yet another work-around for a Jsch bug: timeouts

Jsch 0.1.54 passes on the values from ~/.ssh/config for
"ServerAliveInterval" and "ConnectTimeout" as read from
the config file to java.net.Socket.setSoTimeout(). That
method expects milliseconds, but the values in the config
file are seconds!

The missing conversion in Jsch means that the timeout is
set way too low, and if the server doesn't respond within
that very short time frame, Jsch kills the connection and
then throws an exception with a message such as "session is
down" or "timeout in waiting for rekeying process".

As a work-around, do the conversion to milliseconds in the
Jsch-facing Config interface of OpenSshConfig. That way Jsch
already gets these values as milliseconds.

Bug: 526867
Change-Id: Ibc9b93f7722fffe10f3e770dfe7fdabfb3b97e74
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/OpenSshConfigTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/OpenSshConfigTest.java
index d604751fefd635d8b84c9d7334f6593736ab0963..8ce3f0f147465ce221b276558bfda8ad5b518a87 100644 (file)
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/OpenSshConfigTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/OpenSshConfigTest.java
@@ -343,8 +343,7 @@ public class OpenSshConfigTest extends RepositoryTestCase {
                assertEquals(h1.getConnectionAttempts(), h2.getConnectionAttempts());
                final ConfigRepository.Config c = osc.getConfig("orcz");
                assertNotNull(c);
-               assertSame(c, h1.getConfig());
-               assertSame(c, h2.getConfig());
+               assertSame(h1.getConfig(), h2.getConfig());
        }
 
        @Test

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/OpenSshConfig.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/OpenSshConfig.java
index 67a7db99f44317e26f9561783c73a77e82ae0a6e..79fab01297ff7a4a7a5b38f46c57940c9f5acd52 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/OpenSshConfig.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/OpenSshConfig.java
@@ -59,6 +59,7 @@ import java.util.List;
 import java.util.Locale;
 import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.TimeUnit;
 
 import org.eclipse.jgit.errors.InvalidPatternException;
 import org.eclipse.jgit.fnmatch.FileNameMatcher;
@@ -961,7 +962,7 @@ public class OpenSshConfig implements ConfigRepository {
 
        /**
         * Retrieves the full {@link com.jcraft.jsch.ConfigRepository.Config Config}
-        * for the given host name.
+        * for the given host name. Should be called only by Jsch and tests.
         *
         * @param hostName
         *            to get the config for
@@ -971,7 +972,7 @@ public class OpenSshConfig implements ConfigRepository {
        @Override
        public Config getConfig(String hostName) {
                Host host = lookup(hostName);
-               return host.getConfig();
+               return new JschBugFixingConfig(host.getConfig());
        }
 
        @Override
@@ -980,4 +981,61 @@ public class OpenSshConfig implements ConfigRepository {
                return "OpenSshConfig [home=" + home + ", configFile=" + configFile
                                + ", lastModified=" + lastModified + ", state=" + state + "]";
        }
+
+       /**
+        * A {@link com.jcraft.jsch.ConfigRepository.Config} that transforms some
+        * values from the config file into the format Jsch 0.1.54 expects. This is
+        * a work-around for bugs in Jsch.
+        */
+       private static class JschBugFixingConfig implements Config {
+
+               private final Config real;
+
+               public JschBugFixingConfig(Config delegate) {
+                       real = delegate;
+               }
+
+               @Override
+               public String getHostname() {
+                       return real.getHostname();
+               }
+
+               @Override
+               public String getUser() {
+                       return real.getUser();
+               }
+
+               @Override
+               public int getPort() {
+                       return real.getPort();
+               }
+
+               @Override
+               public String getValue(String key) {
+                       String result = real.getValue(key);
+                       if (result != null) {
+                               String k = key.toUpperCase(Locale.ROOT);
+                               if ("SERVERALIVEINTERVAL".equals(k) //$NON-NLS-1$
+                                               || "CONNECTTIMEOUT".equals(k)) { //$NON-NLS-1$
+                                       // These values are in seconds. Jsch 0.1.54 passes them on
+                                       // as is to java.net.Socket.setSoTimeout(), which expects
+                                       // milliseconds. So convert here to milliseconds...
+                                       try {
+                                               int timeout = Integer.parseInt(result);
+                                               result = Long
+                                                               .toString(TimeUnit.SECONDS.toMillis(timeout));
+                                       } catch (NumberFormatException e) {
+                                               // Ignore
+                                       }
+                               }
+                       }
+                       return result;
+               }
+
+               @Override
+               public String[] getValues(String key) {
+                       return real.getValues(key);
+               }
+
+       }
 }