From f7fbc7fcd72becb06795e32d1ea263cf6185a670 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Tue, 12 Jun 2018 14:09:39 +0200 Subject: [PATCH] Propagate failure of ssh command to caller of SshSupport When SshSupport.runSshCommand fails since the executed external ssh command failed throw a CommandFailedException. If discovery of LFS server fails due to failure of the git-lfs-authenticate command chain the CommandFailureException to the LfsConfigInvalidException in order to allow root cause analysis in the application using that. Change-Id: I2f9ea2be11274549f6d845937164c248b3d840b2 Signed-off-by: Matthias Sohn --- .../jgit/lfs/internal/LfsText.properties | 1 - .../lfs/errors/LfsConfigInvalidException.java | 13 +++++ .../lfs/internal/LfsConnectionFactory.java | 47 ++++++++++--------- .../eclipse/jgit/lfs/internal/LfsText.java | 1 - .../eclipse/jgit/internal/JGitText.properties | 1 + .../org/eclipse/jgit/internal/JGitText.java | 1 + .../src/org/eclipse/jgit/util/SshSupport.java | 26 ++++++++-- 7 files changed, 62 insertions(+), 28 deletions(-) diff --git a/org.eclipse.jgit.lfs/resources/org/eclipse/jgit/lfs/internal/LfsText.properties b/org.eclipse.jgit.lfs/resources/org/eclipse/jgit/lfs/internal/LfsText.properties index 91f0bdb09..0e00f146a 100644 --- a/org.eclipse.jgit.lfs/resources/org/eclipse/jgit/lfs/internal/LfsText.properties +++ b/org.eclipse.jgit.lfs/resources/org/eclipse/jgit/lfs/internal/LfsText.properties @@ -1,4 +1,3 @@ -cannotDiscoverLfs=Cannot discover LFS URI corruptLongObject=The content hash ''{0}'' of the long object ''{1}'' doesn''t match its id, the corrupt object will be deleted. incorrectLONG_OBJECT_ID_LENGTH=Incorrect LONG_OBJECT_ID_LENGTH. inconsistentMediafileLength=Mediafile {0} has unexpected length; expected {1} but found {2}. diff --git a/org.eclipse.jgit.lfs/src/org/eclipse/jgit/lfs/errors/LfsConfigInvalidException.java b/org.eclipse.jgit.lfs/src/org/eclipse/jgit/lfs/errors/LfsConfigInvalidException.java index 5320af0b7..f90a003b4 100644 --- a/org.eclipse.jgit.lfs/src/org/eclipse/jgit/lfs/errors/LfsConfigInvalidException.java +++ b/org.eclipse.jgit.lfs/src/org/eclipse/jgit/lfs/errors/LfsConfigInvalidException.java @@ -63,4 +63,17 @@ public LfsConfigInvalidException(String msg) { super(msg); } + /** + * Constructor for LfsConfigInvalidException. + * + * @param msg + * the error description + * @param e + * cause of this exception + * @since 5.0 + */ + public LfsConfigInvalidException(String msg, Exception e) { + super(msg, e); + } + } diff --git a/org.eclipse.jgit.lfs/src/org/eclipse/jgit/lfs/internal/LfsConnectionFactory.java b/org.eclipse.jgit.lfs/src/org/eclipse/jgit/lfs/internal/LfsConnectionFactory.java index bac980bb8..955eca0c3 100644 --- a/org.eclipse.jgit.lfs/src/org/eclipse/jgit/lfs/internal/LfsConnectionFactory.java +++ b/org.eclipse.jgit.lfs/src/org/eclipse/jgit/lfs/internal/LfsConnectionFactory.java @@ -49,6 +49,7 @@ import java.io.IOException; import java.net.ProxySelector; +import java.net.URISyntaxException; import java.net.URL; import java.text.SimpleDateFormat; import java.util.LinkedList; @@ -56,6 +57,7 @@ import java.util.TreeMap; import org.eclipse.jgit.annotations.NonNull; +import org.eclipse.jgit.errors.CommandFailedException; import org.eclipse.jgit.lfs.LfsPointer; import org.eclipse.jgit.lfs.Protocol; import org.eclipse.jgit.lfs.errors.LfsConfigInvalidException; @@ -68,17 +70,12 @@ import org.eclipse.jgit.transport.http.HttpConnection; import org.eclipse.jgit.util.HttpSupport; import org.eclipse.jgit.util.SshSupport; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; /** * Provides means to get a valid LFS connection for a given repository. */ public class LfsConnectionFactory { - private static final Logger log = LoggerFactory - .getLogger(LfsConnectionFactory.class); - private static final int SSH_AUTH_TIMEOUT_SECONDS = 30; private static final String SCHEME_HTTPS = "https"; //$NON-NLS-1$ private static final String SCHEME_SSH = "ssh"; //$NON-NLS-1$ @@ -133,6 +130,7 @@ private static String getLfsUrl(Repository db, String purpose, String lfsUrl = config.getString(ConfigConstants.CONFIG_SECTION_LFS, null, ConfigConstants.CONFIG_KEY_URL); + Exception ex = null; if (lfsUrl == null) { String remoteUrl = null; for (String remote : db.getRemoteNames()) { @@ -151,39 +149,44 @@ private static String getLfsUrl(Repository db, String purpose, break; } if (lfsUrl == null && remoteUrl != null) { - lfsUrl = discoverLfsUrl(db, purpose, additionalHeaders, - remoteUrl); + try { + lfsUrl = discoverLfsUrl(db, purpose, additionalHeaders, + remoteUrl); + } catch (URISyntaxException | IOException + | CommandFailedException e) { + ex = e; + } } else { lfsUrl = lfsUrl + Protocol.INFO_LFS_ENDPOINT; } } if (lfsUrl == null) { + if (ex != null) { + throw new LfsConfigInvalidException( + LfsText.get().lfsNoDownloadUrl, ex); + } throw new LfsConfigInvalidException(LfsText.get().lfsNoDownloadUrl); } return lfsUrl; } private static String discoverLfsUrl(Repository db, String purpose, - Map additionalHeaders, String remoteUrl) { - try { - URIish u = new URIish(remoteUrl); - if (u.getScheme() == null || SCHEME_SSH.equals(u.getScheme())) { - Protocol.ExpiringAction action = getSshAuthentication( - db, purpose, remoteUrl, u); - additionalHeaders.putAll(action.header); - return action.href; - } else { - return remoteUrl + Protocol.INFO_LFS_ENDPOINT; - } - } catch (Exception e) { - log.error(LfsText.get().cannotDiscoverLfs, e); - return null; // could not discover + Map additionalHeaders, String remoteUrl) + throws URISyntaxException, IOException, CommandFailedException { + URIish u = new URIish(remoteUrl); + if (u.getScheme() == null || SCHEME_SSH.equals(u.getScheme())) { + Protocol.ExpiringAction action = getSshAuthentication(db, purpose, + remoteUrl, u); + additionalHeaders.putAll(action.header); + return action.href; + } else { + return remoteUrl + Protocol.INFO_LFS_ENDPOINT; } } private static Protocol.ExpiringAction getSshAuthentication( Repository db, String purpose, String remoteUrl, URIish u) - throws IOException { + throws IOException, CommandFailedException { AuthCache cached = sshAuthCache.get(remoteUrl); Protocol.ExpiringAction action = null; if (cached != null && cached.validUntil > System.currentTimeMillis()) { diff --git a/org.eclipse.jgit.lfs/src/org/eclipse/jgit/lfs/internal/LfsText.java b/org.eclipse.jgit.lfs/src/org/eclipse/jgit/lfs/internal/LfsText.java index d51203a74..d7d0fe186 100644 --- a/org.eclipse.jgit.lfs/src/org/eclipse/jgit/lfs/internal/LfsText.java +++ b/org.eclipse.jgit.lfs/src/org/eclipse/jgit/lfs/internal/LfsText.java @@ -60,7 +60,6 @@ public static LfsText get() { } // @formatter:off - /***/ public String cannotDiscoverLfs; /***/ public String corruptLongObject; /***/ public String inconsistentMediafileLength; /***/ public String inconsistentContentLength; 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 b608ca853..809bdf286 100644 --- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties +++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties @@ -629,6 +629,7 @@ sourceIsNotAWildcard=Source is not a wildcard. sourceRefDoesntResolveToAnyObject=Source ref {0} doesn''t resolve to any object. sourceRefNotSpecifiedForRefspec=Source ref not specified for refspec: {0} squashCommitNotUpdatingHEAD=Squash commit -- not updating HEAD +sshCommandFailed=Execution of ssh command ''{0}'' failed with error ''{1}'' sshUserNameError=Jsch error: failed to set SSH user name correctly to ''{0}''; using ''{1}'' picked up from SSH config file. sslFailureExceptionMessage=Secure connection to {0} could not be stablished because of SSL problems sslFailureInfo=A secure connection to {0}\ncould not be established because the server''s certificate could not be validated. 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 2ac75e1c2..f3029bf3a 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java @@ -690,6 +690,7 @@ public static JGitText get() { /***/ public String sourceRefDoesntResolveToAnyObject; /***/ public String sourceRefNotSpecifiedForRefspec; /***/ public String squashCommitNotUpdatingHEAD; + /***/ public String sshCommandFailed; /***/ public String sshUserNameError; /***/ public String sslFailureExceptionMessage; /***/ public String sslFailureInfo; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/SshSupport.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/SshSupport.java index 40a576ed2..913aa7286 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/util/SshSupport.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/SshSupport.java @@ -43,8 +43,11 @@ package org.eclipse.jgit.util; import java.io.IOException; +import java.text.MessageFormat; import org.eclipse.jgit.annotations.Nullable; +import org.eclipse.jgit.errors.CommandFailedException; +import org.eclipse.jgit.internal.JGitText; import org.eclipse.jgit.transport.CredentialsProvider; import org.eclipse.jgit.transport.RemoteSession; import org.eclipse.jgit.transport.SshSessionFactory; @@ -75,18 +78,23 @@ public class SshSupport { * @param timeout * a timeout in seconds. The timeout may be exceeded in corner * cases. - * @return The entire output read from stdout. Stderr is discarded. + * @return The entire output read from stdout. * @throws IOException + * @throws CommandFailedException + * if the ssh command execution failed, error message contains + * the content of stderr. */ public static String runSshCommand(URIish sshUri, @Nullable CredentialsProvider provider, FS fs, String command, - int timeout) throws IOException { + int timeout) throws IOException, CommandFailedException { RemoteSession session = null; Process process = null; StreamCopyThread errorThread = null; StreamCopyThread outThread = null; - try (MessageWriter stderr = new MessageWriter(); - MessageWriter stdout = new MessageWriter()) { + CommandFailedException failure = null; + @SuppressWarnings("resource") + MessageWriter stderr = new MessageWriter(); + try (MessageWriter stdout = new MessageWriter()) { session = SshSessionFactory.getInstance().getSession(sshUri, provider, fs, 1000 * timeout); process = session.exec(command, 0); @@ -127,11 +135,21 @@ public static String runSshCommand(URIish sshUri, } } if (process != null) { + if (process.exitValue() != 0) { + failure = new CommandFailedException(process.exitValue(), + MessageFormat.format( + JGitText.get().sshCommandFailed, command, + stderr.toString())); + } process.destroy(); } + stderr.close(); if (session != null) { SshSessionFactory.getInstance().releaseSession(session); } + if (failure != null) { + throw failure; + } } }