Fix issues with LFS on GitHub (SSH)

* URIish seems to have a tiny feature (bug?). The path of the URI
   starts with a '/' only if the URI has a port set (it seems).
 * GitHub does not return SSH authorization on a single line as Gerrit
   does - need to account for that.
 * Increase the SSH git-lfs-authenticate timeout, as GitHub sometimes
   responds slower than expected.
 * Guard against NPE in case the download action does not contain any
   additional headers.

Change-Id: Icd1ead3d015479fd4b8bbd42ed42129b0abfb95c
Signed-off-by: Markus Duft <markus.duft@ssi-schaefer.com>
This commit is contained in:
Markus Duft 2018-06-11 17:12:00 +02:00 committed by Matthias Sohn
parent 747ad8b166
commit 01c52a58f6
5 changed files with 51 additions and 16 deletions

View File

@ -32,4 +32,5 @@ Import-Package: com.google.gson;version="[2.8.2,3.0.0)",
org.eclipse.jgit.treewalk;version="[5.0.0,5.1.0)", org.eclipse.jgit.treewalk;version="[5.0.0,5.1.0)",
org.eclipse.jgit.treewalk.filter;version="[5.0.0,5.1.0)", org.eclipse.jgit.treewalk.filter;version="[5.0.0,5.1.0)",
org.eclipse.jgit.util;version="[5.0.0,5.1.0)", org.eclipse.jgit.util;version="[5.0.0,5.1.0)",
org.eclipse.jgit.util.io;version="[5.0.0,5.1.0)" org.eclipse.jgit.util.io;version="[5.0.0,5.1.0)",
org.slf4j;version="[1.7.0,2.0.0)"

View File

@ -1,3 +1,4 @@
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. 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. incorrectLONG_OBJECT_ID_LENGTH=Incorrect LONG_OBJECT_ID_LENGTH.
inconsistentMediafileLength=Mediafile {0} has unexpected length; expected {1} but found {2}. inconsistentMediafileLength=Mediafile {0} has unexpected length; expected {1} but found {2}.

View File

@ -68,13 +68,18 @@
import org.eclipse.jgit.transport.http.HttpConnection; import org.eclipse.jgit.transport.http.HttpConnection;
import org.eclipse.jgit.util.HttpSupport; import org.eclipse.jgit.util.HttpSupport;
import org.eclipse.jgit.util.SshSupport; 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. * Provides means to get a valid LFS connection for a given repository.
*/ */
public class LfsConnectionFactory { public class LfsConnectionFactory {
private static final int SSH_AUTH_TIMEOUT_SECONDS = 5; 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_HTTPS = "https"; //$NON-NLS-1$
private static final String SCHEME_SSH = "ssh"; //$NON-NLS-1$ private static final String SCHEME_SSH = "ssh"; //$NON-NLS-1$
private static final Map<String, AuthCache> sshAuthCache = new TreeMap<>(); private static final Map<String, AuthCache> sshAuthCache = new TreeMap<>();
@ -162,7 +167,7 @@ private static String discoverLfsUrl(Repository db, String purpose,
Map<String, String> additionalHeaders, String remoteUrl) { Map<String, String> additionalHeaders, String remoteUrl) {
try { try {
URIish u = new URIish(remoteUrl); URIish u = new URIish(remoteUrl);
if (SCHEME_SSH.equals(u.getScheme())) { if (u.getScheme() == null || SCHEME_SSH.equals(u.getScheme())) {
Protocol.ExpiringAction action = getSshAuthentication( Protocol.ExpiringAction action = getSshAuthentication(
db, purpose, remoteUrl, u); db, purpose, remoteUrl, u);
additionalHeaders.putAll(action.header); additionalHeaders.putAll(action.header);
@ -171,6 +176,7 @@ private static String discoverLfsUrl(Repository db, String purpose,
return remoteUrl + Protocol.INFO_LFS_ENDPOINT; return remoteUrl + Protocol.INFO_LFS_ENDPOINT;
} }
} catch (Exception e) { } catch (Exception e) {
log.error(LfsText.get().cannotDiscoverLfs, e);
return null; // could not discover return null; // could not discover
} }
} }
@ -226,8 +232,10 @@ private static Protocol.ExpiringAction getSshAuthentication(
.create(contentUrl, HttpSupport .create(contentUrl, HttpSupport
.proxyFor(ProxySelector.getDefault(), contentUrl)); .proxyFor(ProxySelector.getDefault(), contentUrl));
contentServerConn.setRequestMethod(method); contentServerConn.setRequestMethod(method);
action.header if (action.header != null) {
.forEach((k, v) -> contentServerConn.setRequestProperty(k, v)); action.header.forEach(
(k, v) -> contentServerConn.setRequestProperty(k, v));
}
if (contentUrl.getProtocol().equals(SCHEME_HTTPS) if (contentUrl.getProtocol().equals(SCHEME_HTTPS)
&& !repo.getConfig().getBoolean(HttpConfig.HTTP, && !repo.getConfig().getBoolean(HttpConfig.HTTP,
HttpConfig.SSL_VERIFY_KEY, true)) { HttpConfig.SSL_VERIFY_KEY, true)) {
@ -241,7 +249,13 @@ private static Protocol.ExpiringAction getSshAuthentication(
} }
private static String extractProjectName(URIish u) { private static String extractProjectName(URIish u) {
String path = u.getPath().substring(1); String path = u.getPath();
// begins with a slash if the url contains a port (gerrit vs. github).
if (path.startsWith("/")) { //$NON-NLS-1$
path = path.substring(1);
}
if (path.endsWith(org.eclipse.jgit.lib.Constants.DOT_GIT)) { if (path.endsWith(org.eclipse.jgit.lib.Constants.DOT_GIT)) {
return path.substring(0, path.length() - 4); return path.substring(0, path.length() - 4);
} else { } else {

View File

@ -60,6 +60,7 @@ public static LfsText get() {
} }
// @formatter:off // @formatter:off
/***/ public String cannotDiscoverLfs;
/***/ public String corruptLongObject; /***/ public String corruptLongObject;
/***/ public String inconsistentMediafileLength; /***/ public String inconsistentMediafileLength;
/***/ public String inconsistentContentLength; /***/ public String inconsistentContentLength;

View File

@ -42,12 +42,9 @@
*/ */
package org.eclipse.jgit.util; package org.eclipse.jgit.util;
import java.io.BufferedReader;
import java.io.IOException; import java.io.IOException;
import java.io.InputStreamReader;
import org.eclipse.jgit.annotations.Nullable; import org.eclipse.jgit.annotations.Nullable;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.transport.CredentialsProvider; import org.eclipse.jgit.transport.CredentialsProvider;
import org.eclipse.jgit.transport.RemoteSession; import org.eclipse.jgit.transport.RemoteSession;
import org.eclipse.jgit.transport.SshSessionFactory; import org.eclipse.jgit.transport.SshSessionFactory;
@ -76,8 +73,9 @@ public class SshSupport {
* @param command * @param command
* the remote command to execute. * the remote command to execute.
* @param timeout * @param timeout
* a timeout in seconds. * a timeout in seconds. The timeout may be exceeded in corner
* @return The first line of output read from stdout. Stderr is discarded. * cases.
* @return The entire output read from stdout. Stderr is discarded.
* @throws IOException * @throws IOException
*/ */
public static String runSshCommand(URIish sshUri, public static String runSshCommand(URIish sshUri,
@ -86,17 +84,28 @@ public static String runSshCommand(URIish sshUri,
RemoteSession session = null; RemoteSession session = null;
Process process = null; Process process = null;
StreamCopyThread errorThread = null; StreamCopyThread errorThread = null;
try (MessageWriter stderr = new MessageWriter()) { StreamCopyThread outThread = null;
try (MessageWriter stderr = new MessageWriter();
MessageWriter stdout = new MessageWriter()) {
session = SshSessionFactory.getInstance().getSession(sshUri, session = SshSessionFactory.getInstance().getSession(sshUri,
provider, fs, 1000 * timeout); provider, fs, 1000 * timeout);
process = session.exec(command, 0); process = session.exec(command, 0);
errorThread = new StreamCopyThread(process.getErrorStream(), errorThread = new StreamCopyThread(process.getErrorStream(),
stderr.getRawStream()); stderr.getRawStream());
errorThread.start(); errorThread.start();
try (BufferedReader reader = new BufferedReader( outThread = new StreamCopyThread(process.getInputStream(),
new InputStreamReader(process.getInputStream(), stdout.getRawStream());
Constants.CHARSET))) { outThread.start();
return reader.readLine(); try {
// waitFor with timeout has a bug - JSch' exitValue() throws the
// wrong exception type :(
if (process.waitFor() == 0) {
return stdout.toString();
} else {
return null; // still running after timeout
}
} catch (InterruptedException e) {
return null; // error
} }
} finally { } finally {
if (errorThread != null) { if (errorThread != null) {
@ -108,6 +117,15 @@ public static String runSshCommand(URIish sshUri,
errorThread = null; errorThread = null;
} }
} }
if (outThread != null) {
try {
outThread.halt();
} catch (InterruptedException e) {
// Stop waiting and return anyway.
} finally {
outThread = null;
}
}
if (process != null) { if (process != null) {
process.destroy(); process.destroy();
} }