Follow redirects in transport

Bug: 465167
Change-Id: I6da19c8106201c2a1ac69002bd633b7387f25d96
Signed-off-by: Bo Zhang <zhangbodut@gmail.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>

5 files changed, 174 insertions, 15 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 2b9105cfe7..a58db3e6d6 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,7 @@ import org.eclipse.jgit.transport.http.JDKHttpConnectionFactory;
 import org.eclipse.jgit.transport.http.apache.HttpClientConnectionFactory;
 import org.eclipse.jgit.transport.resolver.RepositoryResolver;
 import org.eclipse.jgit.transport.resolver.ServiceNotEnabledException;
+import org.eclipse.jgit.util.HttpSupport;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -128,6 +129,8 @@ public class SmartClientSmartServerTest extends HttpTestCase {
 
 	private URIish brokenURI;
 
+	private URIish redirectURI;
+
 	private RevBlob A_txt;
 
 	private RevCommit A, B;
@@ -155,13 +158,41 @@ public class SmartClientSmartServerTest extends HttpTestCase {
 				.setBoolean(ConfigConstants.CONFIG_CORE_SECTION, null,
 						ConfigConstants.CONFIG_KEY_LOGALLREFUPDATES, true);
 
-		ServletContextHandler app = server.addContext("/git");
 		GitServlet gs = new GitServlet();
+
+		ServletContextHandler app = addNormalContext(gs, src, srcName);
+
+		ServletContextHandler broken = addBrokenContext(gs, src, srcName);
+
+		ServletContextHandler redirect = addRedirectContext(gs, src, srcName);
+
+		server.setUp();
+
+		remoteRepository = src.getRepository();
+		remoteURI = toURIish(app, srcName);
+		brokenURI = toURIish(broken, srcName);
+		redirectURI = toURIish(redirect, srcName);
+
+		A_txt = src.blob("A");
+		A = src.commit().add("A_txt", A_txt).create();
+		B = src.commit().parent(A).add("A_txt", "C").add("B", "B").create();
+		src.update(master, B);
+
+		src.update("refs/garbage/a/very/long/ref/name/to/compress", B);
+	}
+
+	private ServletContextHandler addNormalContext(GitServlet gs, TestRepository<Repository> src, String srcName) {
+		ServletContextHandler app = server.addContext("/git");
 		gs.setRepositoryResolver(new TestRepoResolver(src, srcName));
 		app.addServlet(new ServletHolder(gs), "/*");
+		return app;
+	}
 
+	@SuppressWarnings("unused")
+	private ServletContextHandler addBrokenContext(GitServlet gs, TestRepository<Repository> src, String srcName) {
 		ServletContextHandler broken = server.addContext("/bad");
 		broken.addFilter(new FilterHolder(new Filter() {
+
 			public void doFilter(ServletRequest request,
 					ServletResponse response, FilterChain chain)
 					throws IOException, ServletException {
@@ -173,29 +204,56 @@ public class SmartClientSmartServerTest extends HttpTestCase {
 				w.close();
 			}
 
-			public void init(FilterConfig filterConfig) throws ServletException {
-				//
+			public void init(FilterConfig filterConfig)
+					throws ServletException {
+				// empty
 			}
 
 			public void destroy() {
-				//
+				// empty
 			}
 		}), "/" + srcName + "/git-upload-pack",
 				EnumSet.of(DispatcherType.REQUEST));
 		broken.addServlet(new ServletHolder(gs), "/*");
+		return broken;
+	}
 
-		server.setUp();
+	@SuppressWarnings("unused")
+	private ServletContextHandler addRedirectContext(GitServlet gs,
+			TestRepository<Repository> src, String srcName) {
+		ServletContextHandler redirect = server.addContext("/redirect");
+		redirect.addFilter(new FilterHolder(new Filter() {
 
-		remoteRepository = src.getRepository();
-		remoteURI = toURIish(app, srcName);
-		brokenURI = toURIish(broken, srcName);
+			@Override
+			public void init(FilterConfig filterConfig)
+					throws ServletException {
+				// empty
+			}
 
-		A_txt = src.blob("A");
-		A = src.commit().add("A_txt", A_txt).create();
-		B = src.commit().parent(A).add("A_txt", "C").add("B", "B").create();
-		src.update(master, B);
+			@Override
+			public void doFilter(ServletRequest request,
+					ServletResponse response, FilterChain chain)
+					throws IOException, ServletException {
+				final HttpServletResponse httpServletResponse = (HttpServletResponse) response;
+				final HttpServletRequest httpServletRequest = (HttpServletRequest) request;
+				final StringBuffer fullUrl = httpServletRequest.getRequestURL();
+				if (httpServletRequest.getQueryString() != null) {
+					fullUrl.append("?")
+							.append(httpServletRequest.getQueryString());
+				}
+				httpServletResponse
+						.setStatus(HttpServletResponse.SC_MOVED_PERMANENTLY);
+				httpServletResponse.setHeader(HttpSupport.HDR_LOCATION,
+						fullUrl.toString().replace("/redirect", "/git"));
+			}
 
-		src.update("refs/garbage/a/very/long/ref/name/to/compress", B);
+			@Override
+			public void destroy() {
+				// empty
+			}
+		}), "/*", EnumSet.of(DispatcherType.REQUEST));
+		redirect.addServlet(new ServletHolder(gs), "/*");
+		return redirect;
 	}
 
 	@Test
@@ -312,6 +370,52 @@ public class SmartClientSmartServerTest extends HttpTestCase {
 	}
 
 	@Test
+	public void testInitialClone_RedirectSmall() throws Exception {
+		Repository dst = createBareRepository();
+		assertFalse(dst.hasObject(A_txt));
+
+		try (Transport t = Transport.open(dst, redirectURI)) {
+			t.fetch(NullProgressMonitor.INSTANCE, mirror(master));
+		}
+
+		assertTrue(dst.hasObject(A_txt));
+		assertEquals(B, dst.exactRef(master).getObjectId());
+		fsck(dst, B);
+
+		List<AccessEvent> requests = getRequests();
+		assertEquals(4, requests.size());
+
+		AccessEvent firstRedirect = requests.get(0);
+		assertEquals(301, firstRedirect.getStatus());
+
+		AccessEvent info = requests.get(1);
+		assertEquals("GET", info.getMethod());
+		assertEquals(join(remoteURI, "info/refs"), info.getPath());
+		assertEquals(1, info.getParameters().size());
+		assertEquals("git-upload-pack", info.getParameter("service"));
+		assertEquals(200, info.getStatus());
+		assertEquals("application/x-git-upload-pack-advertisement",
+				info.getResponseHeader(HDR_CONTENT_TYPE));
+		assertEquals("gzip", info.getResponseHeader(HDR_CONTENT_ENCODING));
+
+		AccessEvent secondRedirect = requests.get(2);
+		assertEquals(301, secondRedirect.getStatus());
+
+		AccessEvent service = requests.get(3);
+		assertEquals("POST", service.getMethod());
+		assertEquals(join(remoteURI, "git-upload-pack"), service.getPath());
+		assertEquals(0, service.getParameters().size());
+		assertNotNull("has content-length",
+				service.getRequestHeader(HDR_CONTENT_LENGTH));
+		assertNull("not chunked",
+				service.getRequestHeader(HDR_TRANSFER_ENCODING));
+
+		assertEquals(200, service.getStatus());
+		assertEquals("application/x-git-upload-pack-result",
+				service.getResponseHeader(HDR_CONTENT_TYPE));
+	}
+
+	@Test
 	public void testFetch_FewLocalCommits() throws Exception {
 		// Bootstrap by doing the clone.
 		//
diff --git a/org.eclipse.jgit/.settings/.api_filters b/org.eclipse.jgit/.settings/.api_filters
new file mode 100644
index 0000000000..fa8a9cf9a8
--- /dev/null
+++ b/org.eclipse.jgit/.settings/.api_filters
@@ -0,0 +1,11 @@
+<?xml version="1.0" encoding="UTF-8" standalone="no"?>
+<component id="org.eclipse.jgit" version="2">
+    <resource path="src/org/eclipse/jgit/transport/http/HttpConnection.java" type="org.eclipse.jgit.transport.http.HttpConnection">
+        <filter comment="OSGi semantic versioning rules allow to break implementors in minor releases" id="403767336">
+            <message_arguments>
+                <message_argument value="org.eclipse.jgit.transport.http.HttpConnection"/>
+                <message_argument value="HTTP_MOVED_PERM"/>
+            </message_arguments>
+        </filter>
+    </resource>
+</component>
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 96a6fe1d01..fee9ae6a72 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransportHttp.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransportHttp.java
@@ -52,6 +52,7 @@ import static org.eclipse.jgit.util.HttpSupport.HDR_ACCEPT;
 import static org.eclipse.jgit.util.HttpSupport.HDR_ACCEPT_ENCODING;
 import static org.eclipse.jgit.util.HttpSupport.HDR_CONTENT_ENCODING;
 import static org.eclipse.jgit.util.HttpSupport.HDR_CONTENT_TYPE;
+import static org.eclipse.jgit.util.HttpSupport.HDR_LOCATION;
 import static org.eclipse.jgit.util.HttpSupport.HDR_PRAGMA;
 import static org.eclipse.jgit.util.HttpSupport.HDR_USER_AGENT;
 import static org.eclipse.jgit.util.HttpSupport.HDR_WWW_AUTHENTICATE;
@@ -898,9 +899,13 @@ public class TransportHttp extends HttpTransport implements WalkTransport,
 		}
 
 		void openStream() throws IOException {
+			openStream(null);
+		}
+
+		void openStream(final String redirectUrl) throws IOException {
 			conn = httpOpen(
 					METHOD_POST,
-					new URL(baseUrl, serviceName),
+					redirectUrl == null ? new URL(baseUrl, serviceName) : new URL(redirectUrl),
 					AcceptEncoding.GZIP);
 			conn.setInstanceFollowRedirects(false);
 			conn.setDoOutput(true);
@@ -909,6 +914,10 @@ public class TransportHttp extends HttpTransport implements WalkTransport,
 		}
 
 		void sendRequest() throws IOException {
+			sendRequest(null);
+		}
+
+		void sendRequest(final String redirectUrl) throws IOException {
 			// Try to compress the content, but only if that is smaller.
 			TemporaryBuffer buf = new TemporaryBuffer.Heap(http.postBuffer);
 			try {
@@ -923,7 +932,7 @@ public class TransportHttp extends HttpTransport implements WalkTransport,
 				buf = out;
 			}
 
-			openStream();
+			openStream(redirectUrl);
 			if (buf != out)
 				conn.setRequestProperty(HDR_CONTENT_ENCODING, ENCODING_GZIP);
 			conn.setFixedLengthStreamingMode((int) buf.length());
@@ -933,6 +942,12 @@ public class TransportHttp extends HttpTransport implements WalkTransport,
 			} finally {
 				httpOut.close();
 			}
+
+			final int status = HttpSupport.response(conn);
+			if (status == HttpConnection.HTTP_MOVED_PERM) {
+				String locationHeader = HttpSupport.responseHeader(conn, HDR_LOCATION);
+				sendRequest(locationHeader);
+			}
 		}
 
 		void openResponse() throws IOException {
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/http/HttpConnection.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/http/HttpConnection.java
index 09613fd7ac..58081c1c90 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/http/HttpConnection.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/http/HttpConnection.java
@@ -73,6 +73,12 @@ public interface HttpConnection {
 	public static final int HTTP_OK = java.net.HttpURLConnection.HTTP_OK;
 
 	/**
+	 * @see HttpURLConnection#HTTP_MOVED_PERM
+	 * @since 4.7
+	 */
+	public static final int HTTP_MOVED_PERM = java.net.HttpURLConnection.HTTP_MOVED_PERM;
+
+	/**
 	 * @see HttpURLConnection#HTTP_NOT_FOUND
 	 */
 	public static final int HTTP_NOT_FOUND = java.net.HttpURLConnection.HTTP_NOT_FOUND;
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/HttpSupport.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/HttpSupport.java
index 251381ab33..9f828adc20 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/util/HttpSupport.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/HttpSupport.java
@@ -141,6 +141,12 @@ public class HttpSupport {
 	/** The {@code Accept-Encoding} header. */
 	public static final String HDR_ACCEPT_ENCODING = "Accept-Encoding"; //$NON-NLS-1$
 
+	/**
+	 * The {@code Location} header.
+	 * @since 4.7
+	 */
+	public static final String HDR_LOCATION = "Location"; //$NON-NLS-1$
+
 	/** The {@code gzip} encoding value for {@link #HDR_ACCEPT_ENCODING}. */
 	public static final String ENCODING_GZIP = "gzip"; //$NON-NLS-1$
 
@@ -235,6 +241,23 @@ public class HttpSupport {
 	}
 
 	/**
+	 * Extract a HTTP header from the response.
+	 *
+	 * @param c
+	 *            connection the header should be obtained from.
+	 * @param headerName
+	 *            the header name
+	 * @return the header value
+	 * @throws IOException
+	 *             communications error prevented obtaining the header.
+	 * @since 4.7
+	 */
+	public static String responseHeader(final HttpConnection c,
+			final String headerName) throws IOException {
+		return c.getHeaderField(headerName);
+	}
+
+	/**
 	 * Determine the proxy server (if any) needed to obtain a URL.
 	 *
 	 * @param proxySelector