diff options
6 files changed, 161 insertions, 14 deletions
diff --git a/org.eclipse.jgit.junit.ssh/src/org/eclipse/jgit/junit/ssh/SshTestBase.java b/org.eclipse.jgit.junit.ssh/src/org/eclipse/jgit/junit/ssh/SshTestBase.java index 2d284cf1d5..3784741195 100644 --- a/org.eclipse.jgit.junit.ssh/src/org/eclipse/jgit/junit/ssh/SshTestBase.java +++ b/org.eclipse.jgit.junit.ssh/src/org/eclipse/jgit/junit/ssh/SshTestBase.java @@ -1,5 +1,5 @@ /* - * Copyright (C) 2018, Thomas Wolf <thomas.wolf@paranor.ch> and others + * Copyright (C) 2018, 2020 Thomas Wolf <thomas.wolf@paranor.ch> and others * * This program and the accompanying materials are made available under the * terms of the Eclipse Distribution License v. 1.0 which is available at @@ -14,6 +14,7 @@ import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; import static org.junit.Assume.assumeTrue; @@ -22,9 +23,14 @@ import java.io.IOException; import java.nio.file.Files; import java.util.List; import java.util.Locale; +import java.util.concurrent.TimeUnit; import org.eclipse.jgit.api.errors.TransportException; +import org.eclipse.jgit.errors.CommandFailedException; import org.eclipse.jgit.transport.CredentialItem; +import org.eclipse.jgit.transport.URIish; +import org.eclipse.jgit.util.FS; +import org.eclipse.jgit.util.SshSupport; import org.junit.Test; import org.junit.experimental.theories.DataPoints; import org.junit.experimental.theories.Theory; @@ -67,10 +73,45 @@ public abstract class SshTestBase extends SshTestHarness { defaultCloneDir = new File(getTemporaryDirectory(), "cloned"); } - @Test(expected = TransportException.class) + @Test public void testSshWithoutConfig() throws Exception { - cloneWith("ssh://" + TEST_USER + "@localhost:" + testPort - + "/doesntmatter", defaultCloneDir, null); + assertThrows(TransportException.class, + () -> cloneWith("ssh://" + TEST_USER + "@localhost:" + testPort + + "/doesntmatter", defaultCloneDir, null)); + } + + @Test + public void testSingleCommand() throws Exception { + installConfig("IdentityFile " + privateKey1.getAbsolutePath()); + String command = SshTestGitServer.ECHO_COMMAND + " 1 without timeout"; + long start = System.nanoTime(); + String reply = SshSupport.runSshCommand( + new URIish("ssh://" + TEST_USER + "@localhost:" + testPort), + null, FS.DETECTED, command, 0); // 0 == no timeout + long elapsed = System.nanoTime() - start; + assertEquals(command, reply); + // Now that we have an idea how long this takes on the test + // infrastructure, try again with a timeout. + command = SshTestGitServer.ECHO_COMMAND + " 1 expecting no timeout"; + // Still use a generous timeout. + int timeout = 10 * ((int) TimeUnit.NANOSECONDS.toSeconds(elapsed) + 1); + reply = SshSupport.runSshCommand( + new URIish("ssh://" + TEST_USER + "@localhost:" + testPort), + null, FS.DETECTED, command, timeout); + assertEquals(command, reply); + } + + @Test + public void testSingleCommandWithTimeoutExpired() throws Exception { + installConfig("IdentityFile " + privateKey1.getAbsolutePath()); + String command = SshTestGitServer.ECHO_COMMAND + " 2 EXPECTING TIMEOUT"; + + CommandFailedException e = assertThrows(CommandFailedException.class, + () -> SshSupport.runSshCommand(new URIish( + "ssh://" + TEST_USER + "@localhost:" + testPort), null, + FS.DETECTED, command, 1)); + assertTrue(e.getMessage().contains(command)); + assertTrue(e.getMessage().contains("time")); } @Test diff --git a/org.eclipse.jgit.junit.ssh/src/org/eclipse/jgit/junit/ssh/SshTestGitServer.java b/org.eclipse.jgit.junit.ssh/src/org/eclipse/jgit/junit/ssh/SshTestGitServer.java index 250a13876a..ab8e0c1ca0 100644 --- a/org.eclipse.jgit.junit.ssh/src/org/eclipse/jgit/junit/ssh/SshTestGitServer.java +++ b/org.eclipse.jgit.junit.ssh/src/org/eclipse/jgit/junit/ssh/SshTestGitServer.java @@ -12,6 +12,8 @@ package org.eclipse.jgit.junit.ssh; import java.io.ByteArrayInputStream; import java.io.IOException; import java.io.InputStream; +import java.io.OutputStream; +import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; import java.security.GeneralSecurityException; @@ -22,6 +24,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; import java.util.Locale; +import java.util.concurrent.TimeUnit; import org.apache.sshd.common.NamedResource; import org.apache.sshd.common.PropertyResolver; @@ -67,6 +70,16 @@ import org.eclipse.jgit.transport.UploadPack; */ public class SshTestGitServer { + /** + * Simple echo test command. Replies with the command string as passed. If + * of the form "echo [int] anything", takes the integer value as a delay in + * seconds before replying, which may be useful to test various + * timeout-related things. + * + * @since 5.9 + */ + public static final String ECHO_COMMAND = "echo"; + @NonNull protected final String testUser; @@ -166,6 +179,8 @@ public class SshTestGitServer { return new GitUploadPackCommand(command, executorService); } else if (command.startsWith(RemoteConfig.DEFAULT_RECEIVE_PACK)) { return new GitReceivePackCommand(command, executorService); + } else if (command.startsWith(ECHO_COMMAND)) { + return new EchoCommand(command, executorService); } return new UnknownCommand(command); }); @@ -475,4 +490,52 @@ public class SshTestGitServer { } } + + /** + * Simple echo command that echoes back the command string. If the first + * argument is a positive integer, it's taken as a delay (in seconds) before + * replying. Assumes UTF-8 character encoding. + */ + private static class EchoCommand extends AbstractCommandSupport { + + protected EchoCommand(String command, + CloseableExecutorService executorService) { + super(command, ThreadUtils.noClose(executorService)); + } + + @Override + public void run() { + String[] parts = getCommand().split(" "); + int timeout = 0; + if (parts.length >= 2) { + try { + timeout = Integer.parseInt(parts[1]); + } catch (NumberFormatException e) { + // No timeout. + } + if (timeout > 0) { + try { + Thread.sleep(TimeUnit.SECONDS.toMillis(timeout)); + } catch (InterruptedException e) { + // Ignore. + } + } + } + try { + doEcho(getCommand(), getOutputStream()); + onExit(0); + } catch (IOException e) { + log.warn( + MessageFormat.format("Could not run {0}", getCommand()), + e); + onExit(-1, e.toString()); + } + } + + private void doEcho(String text, OutputStream stream) + throws IOException { + stream.write(text.getBytes(StandardCharsets.UTF_8)); + stream.flush(); + } + } } diff --git a/org.eclipse.jgit.ssh.jsch/src/org/eclipse/jgit/transport/JschSession.java b/org.eclipse.jgit.ssh.jsch/src/org/eclipse/jgit/transport/JschSession.java index 300e03d798..858bdf3f7f 100644 --- a/org.eclipse.jgit.ssh.jsch/src/org/eclipse/jgit/transport/JschSession.java +++ b/org.eclipse.jgit.ssh.jsch/src/org/eclipse/jgit/transport/JschSession.java @@ -198,7 +198,7 @@ public class JschSession implements RemoteSession { @Override public int exitValue() { if (isRunning()) - throw new IllegalStateException(); + throw new IllegalThreadStateException(); return channel.getExitStatus(); } 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 8cbcb0f67a..899a799989 100644 --- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties +++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties @@ -621,6 +621,7 @@ 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}'' +sshCommandTimeout=Execution of ssh command ''{0}'' timed out after {1} seconds sslFailureExceptionMessage=Secure connection to {0} could not be established because of SSL problems sslFailureInfo=A secure connection to {0} could not be established because the server''s certificate could not be validated. sslFailureCause=SSL reported: {0} 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 782a3f872d..2976ab65fc 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java @@ -649,6 +649,7 @@ public class JGitText extends TranslationBundle { /***/ public String sourceRefNotSpecifiedForRefspec; /***/ public String squashCommitNotUpdatingHEAD; /***/ public String sshCommandFailed; + /***/ public String sshCommandTimeout; /***/ public String sslFailureExceptionMessage; /***/ public String sslFailureInfo; /***/ public String sslFailureCause; 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 a151cd336f..e29704158d 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/util/SshSupport.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/SshSupport.java @@ -11,6 +11,7 @@ package org.eclipse.jgit.util; import java.io.IOException; import java.text.MessageFormat; +import java.util.concurrent.TimeUnit; import org.eclipse.jgit.annotations.Nullable; import org.eclipse.jgit.errors.CommandFailedException; @@ -61,11 +62,21 @@ public class SshSupport { CommandFailedException failure = null; @SuppressWarnings("resource") MessageWriter stderr = new MessageWriter(); + @SuppressWarnings("resource") + MessageWriter stdout = new MessageWriter(); String out; - try (MessageWriter stdout = new MessageWriter()) { + try { + long start = System.nanoTime(); session = SshSessionFactory.getInstance().getSession(sshUri, provider, fs, 1000 * timeout); - process = session.exec(command, 0); + int commandTimeout = timeout; + if (timeout > 0) { + commandTimeout = checkTimeout(command, timeout, start); + } + process = session.exec(command, commandTimeout); + if (timeout > 0) { + commandTimeout = checkTimeout(command, timeout, start); + } errorThread = new StreamCopyThread(process.getErrorStream(), stderr.getRawStream()); errorThread.start(); @@ -73,9 +84,15 @@ public class SshSupport { stdout.getRawStream()); outThread.start(); try { - // waitFor with timeout has a bug - JSch' exitValue() throws the - // wrong exception type :( - if (process.waitFor() == 0) { + boolean finished = false; + if (timeout <= 0) { + process.waitFor(); + finished = true; + } else { + finished = process.waitFor(commandTimeout, + TimeUnit.SECONDS); + } + if (finished) { out = stdout.toString(); } else { out = null; // still running after timeout @@ -103,15 +120,26 @@ public class SshSupport { } } if (process != null) { - if (process.exitValue() != 0) { - failure = new CommandFailedException(process.exitValue(), + try { + if (process.exitValue() != 0) { + failure = new CommandFailedException( + process.exitValue(), + MessageFormat.format( + JGitText.get().sshCommandFailed, + command, stderr.toString())); + } + // It was successful after all + out = stdout.toString(); + } catch (IllegalThreadStateException e) { + failure = new CommandFailedException(0, MessageFormat.format( - JGitText.get().sshCommandFailed, command, - stderr.toString())); + JGitText.get().sshCommandTimeout, command, + Integer.valueOf(timeout))); } process.destroy(); } stderr.close(); + stdout.close(); if (session != null) { SshSessionFactory.getInstance().releaseSession(session); } @@ -122,4 +150,17 @@ public class SshSupport { return out; } + private static int checkTimeout(String command, int timeout, long since) + throws CommandFailedException { + long elapsed = System.nanoTime() - since; + int newTimeout = timeout + - (int) TimeUnit.NANOSECONDS.toSeconds(elapsed); + if (newTimeout <= 0) { + // All time used up for connecting the session + throw new CommandFailedException(0, + MessageFormat.format(JGitText.get().sshCommandTimeout, + command, Integer.valueOf(timeout))); + } + return newTimeout; + } } |