Allow retrying connecting SshSession in case of an exception
Connecting to an SshSession may fail due to different reasons. Jsch for
example often throws an com.jcraft.jsch.JschException: verify: false.[1]
The issue is still not fixed in JSch 0.1.51.
In such a case it is worth retrying to connect. The number of connection
attempts can be configured using ssh_config parameter
"ConnectionAttempts" [2].
Don't retry if the user canceled authentication.
[1] http://sourceforge.net/p/jsch/bugs/58/
[2] http://linux.die.net/man/5/ssh_config
Bug: 437656
Change-Id: I6dd2a3786b7d3f15f5a46821d8edac987a57e381
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
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 5836db1..8ec39bf 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
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2008, Google Inc.
+ * Copyright (C) 2008, 2014 Google Inc.
* and other copyright owners as documented in the project's IP log.
*
* This program and the accompanying materials are made available
@@ -96,6 +96,7 @@ public void testNoConfig() {
assertEquals("repo.or.cz", h.getHostName());
assertEquals("jex_junit", h.getUser());
assertEquals(22, h.getPort());
+ assertEquals(1, h.getConnectionAttempts());
assertNull(h.getIdentityFile());
}
@@ -249,4 +250,35 @@ public void testAlias_InheritBatchMode() throws Exception {
assertNotNull(h);
assertTrue(h.isBatchMode());
}
+
+ @Test
+ public void testAlias_ConnectionAttemptsDefault() throws Exception {
+ final Host h = osc.lookup("orcz");
+ assertNotNull(h);
+ assertEquals(1, h.getConnectionAttempts());
+ }
+
+ @Test
+ public void testAlias_ConnectionAttempts() throws Exception {
+ config("Host orcz\n" + "\tConnectionAttempts 5\n");
+ final Host h = osc.lookup("orcz");
+ assertNotNull(h);
+ assertEquals(5, h.getConnectionAttempts());
+ }
+
+ @Test
+ public void testAlias_invalidConnectionAttempts() throws Exception {
+ config("Host orcz\n" + "\tConnectionAttempts -1\n");
+ final Host h = osc.lookup("orcz");
+ assertNotNull(h);
+ assertEquals(1, h.getConnectionAttempts());
+ }
+
+ @Test
+ public void testAlias_badConnectionAttempts() throws Exception {
+ config("Host orcz\n" + "\tConnectionAttempts xxx\n");
+ final Host h = osc.lookup("orcz");
+ assertNotNull(h);
+ assertEquals(1, h.getConnectionAttempts());
+ }
}
diff --git a/org.eclipse.jgit/.settings/.api_filters b/org.eclipse.jgit/.settings/.api_filters
index 87a931c..56c9ee6 100644
--- a/org.eclipse.jgit/.settings/.api_filters
+++ b/org.eclipse.jgit/.settings/.api_filters
@@ -7,6 +7,12 @@
<message_argument value="3.2.0"/>
</message_arguments>
</filter>
+ <filter comment="minor addition" id="924844039">
+ <message_arguments>
+ <message_argument value="3.4.0"/>
+ <message_argument value="3.4.0"/>
+ </message_arguments>
+ </filter>
</resource>
<resource path="src/org/eclipse/jgit/transport/TransportHttp.java" type="org.eclipse.jgit.transport.TransportHttp">
<filter comment="Method is only used by implementers of TransportHttp's API, minor version are allowed to break implementer API according to OSGi semantic versioning (http://www.osgi.org/wiki/uploads/Links/SemanticVersioning.pdf)" id="338792546">
diff --git a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
index fd5801e..5b12a0c 100644
--- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
+++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
@@ -508,6 +508,7 @@
transportProtoLocal=Local Git Repository
transportProtoSFTP=SFTP
transportProtoSSH=SSH
+transportSSHRetryInterrupt=Interrupted while waiting for retry
treeEntryAlreadyExists=Tree entry "{0}" already exists.
treeFilterMarkerTooManyFilters=Too many markTreeFilters passed, maximum number is {0} (passed {1})
treeIteratorDoesNotSupportRemove=TreeIterator does not support remove()
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
index 8acfb54..f075db3 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
@@ -570,6 +570,7 @@ public static JGitText get() {
/***/ public String transportProtoLocal;
/***/ public String transportProtoSFTP;
/***/ public String transportProtoSSH;
+ /***/ public String transportSSHRetryInterrupt;
/***/ public String treeEntryAlreadyExists;
/***/ public String treeFilterMarkerTooManyFilters;
/***/ public String treeIteratorDoesNotSupportRemove;
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/JschConfigSessionFactory.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/JschConfigSessionFactory.java
index 65e9d4d..308741e 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/JschConfigSessionFactory.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/JschConfigSessionFactory.java
@@ -110,7 +110,7 @@ public synchronized RemoteSession getSession(URIish uri,
pass, host, port, hc);
int retries = 0;
- while (!session.isConnected() && retries < 3) {
+ while (!session.isConnected()) {
try {
retries++;
session.connect(tms);
@@ -120,16 +120,30 @@ public synchronized RemoteSession getSession(URIish uri,
// Make sure our known_hosts is not outdated
knownHosts(getJSch(hc, fs), fs);
- // if authentication failed maybe credentials changed at the
- // remote end therefore reset credentials and retry
- if (credentialsProvider != null && e.getCause() == null
- && e.getMessage().equals("Auth fail") //$NON-NLS-1$
- && retries < 3) {
- credentialsProvider.reset(uri);
- session = createSession(credentialsProvider, fs, user,
- pass, host, port, hc);
- } else {
+ if (isAuthenticationCanceled(e)) {
throw e;
+ } else if (isAuthenticationFailed(e)
+ && credentialsProvider != null) {
+ // if authentication failed maybe credentials changed at
+ // the remote end therefore reset credentials and retry
+ if (retries < 3) {
+ credentialsProvider.reset(uri);
+ session = createSession(credentialsProvider, fs,
+ user, pass, host, port, hc);
+ } else
+ throw e;
+ } else if (retries >= hc.getConnectionAttempts()) {
+ throw e;
+ } else {
+ try {
+ Thread.sleep(1000);
+ session = createSession(credentialsProvider, fs,
+ user, pass, host, port, hc);
+ } catch (InterruptedException e1) {
+ throw new TransportException(
+ JGitText.get().transportSSHRetryInterrupt,
+ e1);
+ }
}
}
}
@@ -147,6 +161,14 @@ public synchronized RemoteSession getSession(URIish uri,
}
+ private static boolean isAuthenticationFailed(JSchException e) {
+ return e.getCause() == null && e.getMessage().equals("Auth fail"); //$NON-NLS-1$
+ }
+
+ private static boolean isAuthenticationCanceled(JSchException e) {
+ return e.getCause() == null && e.getMessage().equals("Auth cancel"); //$NON-NLS-1$
+ }
+
private Session createSession(CredentialsProvider credentialsProvider,
FS fs, String user, final String pass, String host, int port,
final OpenSshConfig.Host hc) throws JSchException {
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 01716da..38f3a2a 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/OpenSshConfig.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/OpenSshConfig.java
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2008-2009, Google Inc.
+ * Copyright (C) 2008, 2014, Google Inc.
* and other copyright owners as documented in the project's IP log.
*
* This program and the accompanying materials are made available
@@ -148,6 +148,8 @@ public Host lookup(final String hostName) {
h.user = OpenSshConfig.userName();
if (h.port == 0)
h.port = OpenSshConfig.SSH_PORT;
+ if (h.connectionAttempts == 0)
+ h.connectionAttempts = 1;
h.patternsApplied = true;
return h;
}
@@ -244,6 +246,18 @@ private Map<String, Host> parse(final InputStream in) throws IOException {
for (final Host c : current)
if (c.strictHostKeyChecking == null)
c.strictHostKeyChecking = value;
+ } else if (StringUtils.equalsIgnoreCase(
+ "ConnectionAttempts", keyword)) { //$NON-NLS-1$
+ try {
+ final int connectionAttempts = Integer.parseInt(dequote(argValue));
+ if (connectionAttempts > 0) {
+ for (final Host c : current)
+ if (c.connectionAttempts == 0)
+ c.connectionAttempts = connectionAttempts;
+ }
+ } catch (NumberFormatException nfe) {
+ // ignore bad values
+ }
}
}
@@ -331,6 +345,8 @@ public static class Host {
String strictHostKeyChecking;
+ int connectionAttempts;
+
void copyFrom(final Host src) {
if (hostName == null)
hostName = src.hostName;
@@ -346,6 +362,8 @@ void copyFrom(final Host src) {
batchMode = src.batchMode;
if (strictHostKeyChecking == null)
strictHostKeyChecking = src.strictHostKeyChecking;
+ if (connectionAttempts == 0)
+ connectionAttempts = src.connectionAttempts;
}
/**
@@ -402,5 +420,16 @@ public String getPreferredAuthentications() {
public boolean isBatchMode() {
return batchMode != null && batchMode.booleanValue();
}
+
+ /**
+ * @return the number of tries (one per second) to connect before
+ * exiting. The argument must be an integer. This may be useful
+ * in scripts if the connection sometimes fails. The default is
+ * 1.
+ * @since 3.4
+ */
+ public int getConnectionAttempts() {
+ return connectionAttempts;
+ }
}
}
