Fix JSchProcess.waitFor() with time-out

SshSupport.runSshCommand() had a comment that wait with time-out
could not be used because JSchProcess.exitValue() threw the wrong
unchecked exception when the process was still running.

Fix this and make JSchProcess.exitValue() throw the right exception,
then wait with a time-out in SshSupport.

The Apache sshd client's SshdExecProcess has always used the correct
IllegalThreadStateException.

Add tests for SshSupport.runCommand().

Change-Id: Id30893174ae8be3b9a16119674049337b0cf4381
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>

2 files changed, 108 insertions, 4 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();
+		}
+	}
 }