Add support to follow HTTP redirects

git-core follows HTTP redirects so JGit should also provide this.

Implement config setting http.followRedirects with possible values
"false" (= never), "true" (= always), and "initial" (only on GET, but
not on POST).[1]

We must do our own redirect handling and cannot rely on the support
that the underlying real connection may offer. At least the JDK's
HttpURLConnection has two features that get in the way:

* it does not allow cross-protocol redirects and thus fails on
  http->https redirects (for instance, on Github).
* it translates a redirect after a POST to a GET unless the system
  property "http.strictPostRedirect" is set to true. We don't want
  to manipulate that system setting nor require it.

Additionally, git has its own rules about what redirects it accepts;[2]
for instance, it does not allow a redirect that adds query arguments.

We handle response codes 301, 302, 303, and 307 as per RFC 2616.[3]
On POST we do not handle 303, and we follow redirects only if
http.followRedirects == true.

Redirects are followed only a certain number of times. There are two
ways to control that limit:

* by default, the limit is given by the http.maxRedirects system
  property that is also used by the JDK. If the system property is
  not set, the default is 5. (This is much lower than the JDK default
  of 20, but I don't see the value of following so many redirects.)
* this can be overwritten by a http.maxRedirects git config setting.

The JGit http.* git config settings are currently all global; JGit has
no support yet for URI-specific settings "http.<pattern>.name". Adding
support for that is well beyond the scope of this change.

Like git-core, we log every redirect attempt (LOG.info) so that users
may know about the redirection having occurred.

Extends the test framework to configure an AppServer with HTTPS support
so that we can test cloning via HTTPS and redirections involving HTTPS.

[1] https://git-scm.com/docs/git-config
[2] https://kernel.googlesource.com/pub/scm/git/git/+/6628eb41db5189c0cdfdced6d8697e7c813c5f0f
[3] https://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html

CQ: 13987
Bug: 465167
Change-Id: I86518cb76842f7d326b51f8715e3bbf8ada89859
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>

3 files changed, 176 insertions, 12 deletions

diff --git a/org.eclipse.jgit.junit.http/META-INF/MANIFEST.MF b/org.eclipse.jgit.junit.http/META-INF/MANIFEST.MF
index dd6cb48b16..f5e3033845 100644
--- a/org.eclipse.jgit.junit.http/META-INF/MANIFEST.MF
+++ b/org.eclipse.jgit.junit.http/META-INF/MANIFEST.MF
@@ -20,6 +20,7 @@ Import-Package: javax.servlet;version="[2.5.0,3.2.0)",
  org.eclipse.jetty.util.component;version="[9.4.5,10.0.0)",
  org.eclipse.jetty.util.log;version="[9.4.5,10.0.0)",
  org.eclipse.jetty.util.security;version="[9.4.5,10.0.0)",
+ org.eclipse.jetty.util.ssl;version="[9.4.5,10.0.0)",
  org.eclipse.jgit.errors;version="[4.9.0,4.10.0)",
  org.eclipse.jgit.http.server;version="[4.9.0,4.10.0)",
  org.eclipse.jgit.internal.storage.file;version="[4.9.0,4.10.0)",
diff --git a/org.eclipse.jgit.junit.http/src/org/eclipse/jgit/junit/http/AppServer.java b/org.eclipse.jgit.junit.http/src/org/eclipse/jgit/junit/http/AppServer.java
index 28c0f21111..69e2cd5957 100644
--- a/org.eclipse.jgit.junit.http/src/org/eclipse/jgit/junit/http/AppServer.java
+++ b/org.eclipse.jgit.junit.http/src/org/eclipse/jgit/junit/http/AppServer.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2010, 2012 Google Inc.
+ * Copyright (C) 2010, 2017 Google Inc.
  * and other copyright owners as documented in the project's IP log.
  *
  * This program and the accompanying materials are made available
@@ -46,15 +46,19 @@ package org.eclipse.jgit.junit.http;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
+import java.io.File;
+import java.io.IOException;
 import java.net.InetAddress;
 import java.net.URI;
 import java.net.URISyntaxException;
 import java.net.UnknownHostException;
+import java.nio.file.Files;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
 
+import org.eclipse.jetty.http.HttpVersion;
 import org.eclipse.jetty.security.AbstractLoginService;
 import org.eclipse.jetty.security.Authenticator;
 import org.eclipse.jetty.security.ConstraintMapping;
@@ -65,10 +69,12 @@ import org.eclipse.jetty.server.HttpConfiguration;
 import org.eclipse.jetty.server.HttpConnectionFactory;
 import org.eclipse.jetty.server.Server;
 import org.eclipse.jetty.server.ServerConnector;
+import org.eclipse.jetty.server.SslConnectionFactory;
 import org.eclipse.jetty.server.handler.ContextHandlerCollection;
 import org.eclipse.jetty.servlet.ServletContextHandler;
 import org.eclipse.jetty.util.security.Constraint;
 import org.eclipse.jetty.util.security.Password;
+import org.eclipse.jetty.util.ssl.SslContextFactory;
 import org.eclipse.jgit.transport.URIish;
 
 /**
@@ -88,6 +94,9 @@ public class AppServer {
 	/** Password for {@link #username} in secured access areas. */
 	public static final String password = "letmein";
 
+	/** SSL keystore password; must have at least 6 characters. */
+	private static final String keyPassword = "mykeys";
+
 	static {
 		// Install a logger that throws warning messages.
 		//
@@ -97,48 +106,141 @@ public class AppServer {
 
 	private final Server server;
 
+	private final HttpConfiguration config;
+
 	private final ServerConnector connector;
 
+	private final HttpConfiguration secureConfig;
+
+	private final ServerConnector secureConnector;
+
 	private final ContextHandlerCollection contexts;
 
 	private final TestRequestLog log;
 
+	private List<File> filesToDelete = new ArrayList<>();
+
 	public AppServer() {
-		this(0);
+		this(0, -1);
 	}
 
 	/**
 	 * @param port
-	 *            the http port number
+	 *            the http port number; may be zero to allocate a port
+	 *            dynamically
 	 * @since 4.2
 	 */
 	public AppServer(int port) {
+		this(port, -1);
+	}
+
+	/**
+	 * @param port
+	 *            for https, may be zero to allocate a port dynamically
+	 * @param sslPort
+	 *            for https,may be zero to allocate a port dynamically. If
+	 *            negative, the server will be set up without https support..
+	 * @since 4.9
+	 */
+	public AppServer(int port, int sslPort) {
 		server = new Server();
 
-		HttpConfiguration http_config = new HttpConfiguration();
-		http_config.setSecureScheme("https");
-		http_config.setSecurePort(8443);
-		http_config.setOutputBufferSize(32768);
+		config = new HttpConfiguration();
+		config.setSecureScheme("https");
+		config.setSecurePort(0);
+		config.setOutputBufferSize(32768);
 
 		connector = new ServerConnector(server,
-				new HttpConnectionFactory(http_config));
+				new HttpConnectionFactory(config));
 		connector.setPort(port);
+		String ip;
+		String hostName;
 		try {
 			final InetAddress me = InetAddress.getByName("localhost");
-			connector.setHost(me.getHostAddress());
+			ip = me.getHostAddress();
+			connector.setHost(ip);
+			hostName = InetAddress.getLocalHost().getCanonicalHostName();
 		} catch (UnknownHostException e) {
 			throw new RuntimeException("Cannot find localhost", e);
 		}
 
+		if (sslPort >= 0) {
+			SslContextFactory sslContextFactory = createTestSslContextFactory(
+					hostName);
+			secureConfig = new HttpConfiguration(config);
+			secureConnector = new ServerConnector(server,
+					new SslConnectionFactory(sslContextFactory,
+							HttpVersion.HTTP_1_1.asString()),
+					new HttpConnectionFactory(secureConfig));
+			secureConnector.setPort(sslPort);
+			secureConnector.setHost(ip);
+		} else {
+			secureConfig = null;
+			secureConnector = null;
+		}
+
 		contexts = new ContextHandlerCollection();
 
 		log = new TestRequestLog();
 		log.setHandler(contexts);
 
-		server.setConnectors(new Connector[] { connector });
+		if (secureConnector == null) {
+			server.setConnectors(new Connector[] { connector });
+		} else {
+			server.setConnectors(
+					new Connector[] { connector, secureConnector });
+		}
 		server.setHandler(log);
 	}
 
+	private SslContextFactory createTestSslContextFactory(String hostName) {
+		SslContextFactory factory = new SslContextFactory(true);
+
+		String dName = "CN=,OU=,O=,ST=,L=,C=";
+
+		try {
+			File tmpDir = Files.createTempDirectory("jks").toFile();
+			tmpDir.deleteOnExit();
+			makePrivate(tmpDir);
+			File keyStore = new File(tmpDir, "keystore.jks");
+			Runtime.getRuntime().exec(
+					new String[] {
+							"keytool", //
+							"-keystore", keyStore.getAbsolutePath(), //
+							"-storepass", keyPassword,
+							"-alias", hostName, //
+							"-genkeypair", //
+							"-keyalg", "RSA", //
+							"-keypass", keyPassword, //
+							"-dname", dName, //
+							"-validity", "2" //
+					}).waitFor();
+			keyStore.deleteOnExit();
+			makePrivate(keyStore);
+			filesToDelete.add(keyStore);
+			filesToDelete.add(tmpDir);
+			factory.setKeyStorePath(keyStore.getAbsolutePath());
+			factory.setKeyStorePassword(keyPassword);
+			factory.setKeyManagerPassword(keyPassword);
+			factory.setTrustStorePath(keyStore.getAbsolutePath());
+			factory.setTrustStorePassword(keyPassword);
+		} catch (InterruptedException | IOException e) {
+			throw new RuntimeException("Cannot create ssl key/certificate", e);
+		}
+		return factory;
+	}
+
+	private void makePrivate(File file) {
+		file.setReadable(false);
+		file.setWritable(false);
+		file.setExecutable(false);
+		file.setReadable(true, true);
+		file.setWritable(true, true);
+		if (file.isDirectory()) {
+			file.setExecutable(true, true);
+		}
+	}
+
 	/**
 	 * Create a new servlet context within the server.
 	 * <p>
@@ -231,6 +333,10 @@ public class AppServer {
 		RecordingLogger.clear();
 		log.clear();
 		server.start();
+		config.setSecurePort(getSecurePort());
+		if (secureConfig != null) {
+			secureConfig.setSecurePort(getSecurePort());
+		}
 	}
 
 	/**
@@ -243,6 +349,10 @@ public class AppServer {
 		RecordingLogger.clear();
 		log.clear();
 		server.stop();
+		for (File f : filesToDelete) {
+			f.delete();
+		}
+		filesToDelete.clear();
 	}
 
 	/**
@@ -272,6 +382,12 @@ public class AppServer {
 		return connector.getLocalPort();
 	}
 
+	/** @return the HTTPS port or -1 if not configured. */
+	public int getSecurePort() {
+		assertAlreadySetUp();
+		return secureConnector != null ? secureConnector.getLocalPort() : -1;
+	}
+
 	/** @return all requests since the server was started. */
 	public List<AccessEvent> getRequests() {
 		return new ArrayList<>(log.getEvents());
diff --git a/org.eclipse.jgit.junit.http/src/org/eclipse/jgit/junit/http/HttpTestCase.java b/org.eclipse.jgit.junit.http/src/org/eclipse/jgit/junit/http/HttpTestCase.java
index 1b94e02fa4..eabb0f2256 100644
--- a/org.eclipse.jgit.junit.http/src/org/eclipse/jgit/junit/http/HttpTestCase.java
+++ b/org.eclipse.jgit.junit.http/src/org/eclipse/jgit/junit/http/HttpTestCase.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2009-2010, Google Inc.
+ * Copyright (C) 2009-2017, Google Inc.
  * and other copyright owners as documented in the project's IP log.
  *
  * This program and the accompanying materials are made available
@@ -77,7 +77,7 @@ public abstract class HttpTestCase extends LocalDiskRepositoryTestCase {
 	@Override
 	public void setUp() throws Exception {
 		super.setUp();
-		server = new AppServer();
+		server = createServer();
 	}
 
 	@Override
@@ -86,6 +86,20 @@ public abstract class HttpTestCase extends LocalDiskRepositoryTestCase {
 		super.tearDown();
 	}
 
+	/**
+	 * Creates the {@linkAppServer}.This default implementation creates a server
+	 * without SSLsupport listening for HTTP connections on a dynamically chosen
+	 * port, which can be gotten once the server has been started via its
+	 * {@link AppServer#getPort()} method. Subclasses may override if they need
+	 * a more specialized server.
+	 *
+	 * @return the {@link AppServer}.
+	 * @since 4.9
+	 */
+	protected AppServer createServer() {
+		return new AppServer();
+	}
+
 	protected TestRepository<Repository> createTestRepository()
 			throws IOException {
 		return new TestRepository<>(createBareRepository());
@@ -165,4 +179,37 @@ public abstract class HttpTestCase extends LocalDiskRepositoryTestCase {
 			dir += "/";
 		return dir + path;
 	}
+
+	protected static String rewriteUrl(String url, String newProtocol,
+			int newPort) {
+		String newUrl = url;
+		if (newProtocol != null && !newProtocol.isEmpty()) {
+			int schemeEnd = newUrl.indexOf("://");
+			if (schemeEnd >= 0) {
+				newUrl = newProtocol + newUrl.substring(schemeEnd);
+			}
+		}
+		if (newPort > 0) {
+			newUrl = newUrl.replaceFirst(":\\d+/", ":" + newPort + "/");
+		} else {
+			// Remove the port, if any
+			newUrl = newUrl.replaceFirst(":\\d+/", "/");
+		}
+		return newUrl;
+	}
+
+	protected static URIish extendPath(URIish uri, String pathComponents)
+			throws URISyntaxException {
+		String raw = uri.toString();
+		String newComponents = pathComponents;
+		if (!newComponents.startsWith("/")) {
+			newComponents = '/' + newComponents;
+		}
+		if (!newComponents.endsWith("/")) {
+			newComponents += '/';
+		}
+		int i = raw.lastIndexOf('/');
+		raw = raw.substring(0, i) + newComponents + raw.substring(i + 1);
+		return new URIish(raw);
+	}
 }