TransportHttp: abort on time-out or on SocketException

Avoid trying other authentication methods on SocketException or on
InterruptedIOException. SocketException is rather fatal, such as
nothing listening on the peer's port, connection reset, or it could
be a connection time-out.

Time-outs enforced by Timeout{Input,Output}Stream may result in
InterruptedIOException being thrown.

In both cases, it makes no sense to try other authentication methods,
and doing so may wrongly report "authentication not supported" or
"cannot open git-upload-pack" or some such instead of reporting a
time-out.

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

2 files changed, 90 insertions, 0 deletions

diff --git a/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/SmartClientSmartServerTest.java b/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/SmartClientSmartServerTest.java
index 51a7a8ddc1..8d1870a87e 100644
--- a/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/SmartClientSmartServerTest.java
+++ b/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/SmartClientSmartServerTest.java
@@ -112,6 +112,10 @@ public class SmartClientSmartServerTest extends AllFactoriesHttpTestCase {
 
 	private URIish authOnPostURI;
 
+	private URIish slowURI;
+
+	private URIish slowAuthURI;
+
 	private RevBlob A_txt;
 
 	private RevCommit A, B, unreachableCommit;
@@ -152,6 +156,10 @@ public class SmartClientSmartServerTest extends AllFactoriesHttpTestCase {
 
 		ServletContextHandler authOnPost = addAuthContext(gs, "pauth", "POST");
 
+		ServletContextHandler slow = addSlowContext(gs, "slow", false);
+
+		ServletContextHandler slowAuth = addSlowContext(gs, "slowAuth", true);
+
 		server.setUp();
 
 		remoteRepository = src.getRepository();
@@ -160,6 +168,8 @@ public class SmartClientSmartServerTest extends AllFactoriesHttpTestCase {
 		redirectURI = toURIish(redirect, srcName);
 		authURI = toURIish(auth, srcName);
 		authOnPostURI = toURIish(authOnPost, srcName);
+		slowURI = toURIish(slow, srcName);
+		slowAuthURI = toURIish(slowAuth, srcName);
 
 		A_txt = src.blob("A");
 		A = src.commit().add("A_txt", A_txt).create();
@@ -372,6 +382,43 @@ public class SmartClientSmartServerTest extends AllFactoriesHttpTestCase {
 		return redirect;
 	}
 
+	private ServletContextHandler addSlowContext(GitServlet gs, String path,
+			boolean auth) {
+		ServletContextHandler slow = server.addContext('/' + path);
+		slow.addFilter(new FilterHolder(new Filter() {
+
+			@Override
+			public void init(FilterConfig filterConfig)
+					throws ServletException {
+				// empty
+			}
+
+			// Simply delays the servlet for two seconds. Used for timeout
+			// tests, which use a one-second timeout.
+			@Override
+			public void doFilter(ServletRequest request,
+					ServletResponse response, FilterChain chain)
+					throws IOException, ServletException {
+				try {
+					Thread.sleep(2000);
+				} catch (InterruptedException e) {
+					throw new IOException(e);
+				}
+				chain.doFilter(request, response);
+			}
+
+			@Override
+			public void destroy() {
+				// empty
+			}
+		}), "/*", EnumSet.of(DispatcherType.REQUEST));
+		slow.addServlet(new ServletHolder(gs), "/*");
+		if (auth) {
+			return server.authBasic(slow);
+		}
+		return slow;
+	}
+
 	@Test
 	public void testListRemote() throws IOException {
 		assertEquals("http", remoteURI.getScheme());
@@ -489,6 +536,35 @@ public class SmartClientSmartServerTest extends AllFactoriesHttpTestCase {
 	}
 
 	@Test
+	public void testTimeoutExpired() throws Exception {
+		try (Repository dst = createBareRepository();
+				Transport t = Transport.open(dst, slowURI)) {
+			t.setTimeout(1);
+			TransportException expected = assertThrows(TransportException.class,
+					() -> t.fetch(NullProgressMonitor.INSTANCE,
+							mirror(master)));
+			assertTrue("Unexpected exception message: " + expected.toString(),
+					expected.getMessage().contains("time"));
+		}
+	}
+
+	@Test
+	public void testTimeoutExpiredWithAuth() throws Exception {
+		try (Repository dst = createBareRepository();
+				Transport t = Transport.open(dst, slowAuthURI)) {
+			t.setTimeout(1);
+			t.setCredentialsProvider(testCredentials);
+			TransportException expected = assertThrows(TransportException.class,
+					() -> t.fetch(NullProgressMonitor.INSTANCE,
+							mirror(master)));
+			assertTrue("Unexpected exception message: " + expected.toString(),
+					expected.getMessage().contains("time"));
+			assertFalse("Unexpected exception message: " + expected.toString(),
+					expected.getMessage().contains("auth"));
+		}
+	}
+
+	@Test
 	public void testInitialClone_Small() throws Exception {
 		try (Repository dst = createBareRepository();
 				Transport t = Transport.open(dst, remoteURI)) {
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransportHttp.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransportHttp.java
index c3c1f2a405..16169f028b 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransportHttp.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransportHttp.java
@@ -39,11 +39,13 @@ import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.InputStreamReader;
+import java.io.InterruptedIOException;
 import java.io.OutputStream;
 import java.net.HttpCookie;
 import java.net.MalformedURLException;
 import java.net.Proxy;
 import java.net.ProxySelector;
+import java.net.SocketException;
 import java.net.URI;
 import java.net.URISyntaxException;
 import java.net.URL;
@@ -573,6 +575,14 @@ public class TransportHttp extends HttpTransport implements WalkTransport,
 				}
 			} catch (NotSupportedException | TransportException e) {
 				throw e;
+			} catch (InterruptedIOException e) {
+				// Timeout!? Don't try other authentication methods.
+				throw new TransportException(uri, MessageFormat.format(
+						JGitText.get().connectionTimeOut, u.getHost()), e);
+			} catch (SocketException e) {
+				// Nothing on other end, timeout, connection reset, ...
+				throw new TransportException(uri,
+						JGitText.get().connectionFailed, e);
 			} catch (SSLHandshakeException e) {
 				handleSslFailure(e);
 				continue; // Re-try
@@ -1501,6 +1511,10 @@ public class TransportHttp extends HttpTransport implements WalkTransport,
 				} catch (SSLHandshakeException e) {
 					handleSslFailure(e);
 					continue; // Re-try
+				} catch (SocketException | InterruptedIOException e) {
+					// Timeout!? Must propagate; don't try other authentication
+					// methods.
+					throw e;
 				} catch (IOException e) {
 					if (authenticator == null || authMethod
 							.getType() != HttpAuthMethod.Type.NONE) {