From 4b2b3294b8af9631ea66728baa2839fbcca630f5 Mon Sep 17 00:00:00 2001 From: Stefan Lay Date: Wed, 18 Jun 2014 11:33:34 +0200 Subject: [PATCH] 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 --- .../jgit/transport/OpenSshConfigTest.java | 34 ++++++++++++++- org.eclipse.jgit/.settings/.api_filters | 6 +++ .../eclipse/jgit/internal/JGitText.properties | 1 + .../org/eclipse/jgit/internal/JGitText.java | 1 + .../transport/JschConfigSessionFactory.java | 42 ++++++++++++++----- .../eclipse/jgit/transport/OpenSshConfig.java | 31 +++++++++++++- 6 files changed, 103 insertions(+), 12 deletions(-) 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 5836db1e6..8ec39bf56 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 class OpenSshConfigTest extends RepositoryTestCase { 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 class OpenSshConfigTest extends RepositoryTestCase { 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 87a931c87..56c9ee68a 100644 --- a/org.eclipse.jgit/.settings/.api_filters +++ b/org.eclipse.jgit/.settings/.api_filters @@ -7,6 +7,12 @@ + + + + + + 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 fd5801e6a..5b12a0c68 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 @@ transportProtoHTTP=HTTP 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 8acfb54b8..f075db3e5 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 class JGitText extends TranslationBundle { /***/ 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 65e9d4dc5..308741e93 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 abstract class JschConfigSessionFactory extends SshSessionFactory { 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 abstract class JschConfigSessionFactory extends SshSessionFactory { // 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 abstract class JschConfigSessionFactory extends SshSessionFactory { } + 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 01716da70..38f3a2ac7 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 class OpenSshConfig { 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 @@ public class OpenSshConfig { 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 class OpenSshConfig { String strictHostKeyChecking; + int connectionAttempts; + void copyFrom(final Host src) { if (hostName == null) hostName = src.hostName; @@ -346,6 +362,8 @@ public class OpenSshConfig { batchMode = src.batchMode; if (strictHostKeyChecking == null) strictHostKeyChecking = src.strictHostKeyChecking; + if (connectionAttempts == 0) + connectionAttempts = src.connectionAttempts; } /** @@ -402,5 +420,16 @@ public class OpenSshConfig { 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; + } } }