Merge branch 'stable-4.9' into stable-4.10

* stable-4.9:
  Revert handling of ssh IdentityFile to pre-4.9 behavior

Change-Id: I7cd2f0fe93be5193d6de2a376a023bdece5d84a9

org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/OpenSshConfigTest.java

@@ -343,7 +343,8 @@ public class OpenSshConfigTest extends RepositoryTestCase {
 		assertEquals(h1.getConnectionAttempts(), h2.getConnectionAttempts());
 		final ConfigRepository.Config c = osc.getConfig("orcz");
 		assertNotNull(c);
-		assertSame(h1.getConfig(), h2.getConfig());
+		assertSame(c, h1.getConfig());
+		assertSame(c, h2.getConfig());
 	}
 
 	@Test

org.eclipse.jgit/src/org/eclipse/jgit/transport/JschConfigSessionFactory.java

@@ -59,7 +59,9 @@ import java.net.ConnectException;
 import java.net.UnknownHostException;
 import java.text.MessageFormat;
 import java.util.HashMap;
+import java.util.Locale;
 import java.util.Map;
+import java.util.concurrent.TimeUnit;
 
 import org.eclipse.jgit.errors.TransportException;
 import org.eclipse.jgit.internal.JGitText;
@@ -67,6 +69,7 @@ import org.eclipse.jgit.util.FS;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import com.jcraft.jsch.ConfigRepository;
 import com.jcraft.jsch.JSch;
 import com.jcraft.jsch.JSchException;
 import com.jcraft.jsch.Session;
@@ -88,6 +91,14 @@ public abstract class JschConfigSessionFactory extends SshSessionFactory {
 	private static final Logger LOG = LoggerFactory
 			.getLogger(JschConfigSessionFactory.class);
 
+	/**
+	 * We use different Jsch instances for hosts that have an IdentityFile
+	 * configured in ~/.ssh/config. Jsch by default would cache decrypted keys
+	 * only per session, which results in repeated password prompts. Using
+	 * different Jsch instances, we can cache the keys on these instances so
+	 * that they will be re-used for successive sessions, and thus the user is
+	 * prompted for a key password only once while Eclipse runs.
+	 */
 	private final Map<String, JSch> byIdentityFile = new HashMap<>();
 
 	private JSch defaultJSch;
@@ -298,7 +309,8 @@ public abstract class JschConfigSessionFactory extends SshSessionFactory {
 		if (defaultJSch == null) {
 			defaultJSch = createDefaultJSch(fs);
 			if (defaultJSch.getConfigRepository() == null) {
-				defaultJSch.setConfigRepository(config);
+				defaultJSch.setConfigRepository(
+						new JschBugFixingConfigRepository(config));
 			}
 			for (Object name : defaultJSch.getIdentityNames())
 				byIdentityFile.put((String) name, defaultJSch);
@@ -381,4 +393,91 @@ public abstract class JschConfigSessionFactory extends SshSessionFactory {
 			}
 		}
 	}
+
+	private static class JschBugFixingConfigRepository
+			implements ConfigRepository {
+
+		private final ConfigRepository base;
+
+		public JschBugFixingConfigRepository(ConfigRepository base) {
+			this.base = base;
+		}
+
+		@Override
+		public Config getConfig(String host) {
+			return new JschBugFixingConfig(base.getConfig(host));
+		}
+
+		/**
+		 * 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.
+		 * <p>
+		 * Additionally, this config hides the IdentityFile config entries from
+		 * Jsch; we manage those ourselves. Otherwise Jsch would cache passwords
+		 * (or rather, decrypted keys) only for a single session, resulting in
+		 * multiple password prompts for user operations that use several Jsch
+		 * sessions.
+		 */
+		private static class JschBugFixingConfig implements Config {
+
+			private static final String[] NO_IDENTITIES = {};
+
+			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 k = key.toUpperCase(Locale.ROOT);
+				if ("IDENTITYFILE".equals(k)) { //$NON-NLS-1$
+					return null;
+				}
+				String result = real.getValue(key);
+				if (result != null) {
+					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) {
+				String k = key.toUpperCase(Locale.ROOT);
+				if ("IDENTITYFILE".equals(k)) { //$NON-NLS-1$
+					return NO_IDENTITIES;
+				}
+				return real.getValues(key);
+			}
+		}
+	}
 }

org.eclipse.jgit/src/org/eclipse/jgit/transport/OpenSshConfig.java

@@ -59,7 +59,6 @@ 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;
@@ -973,7 +972,7 @@ public class OpenSshConfig implements ConfigRepository {
 	@Override
 	public Config getConfig(String hostName) {
 		Host host = lookup(hostName);
-		return new JschBugFixingConfig(host.getConfig());
+		return host.getConfig();
 	}
 
 	/** {@inheritDoc} */
@@ -983,61 +982,4 @@ 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);
-		}
-
-	}
 }