From: Thomas Wolf <thomas.wolf@paranor.ch>
Date: Sat, 11 Nov 2017 10:41:10 +0000 (+0100)
Subject: Yet another work-around for a Jsch bug: timeouts
X-Git-Tag: v4.9.1.201712030800-r~5
X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=5284cc1bf764207343a43effccdcada02c05b2bd;p=jgit.git

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 d604751fef..8ce3f0f147 100644
--- 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 67a7db99f4..79fab01297 100644
--- 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);
+		}
+
+	}
 }