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>

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 2b9105cfe7e2d3bea3c497e538fd96dbd4c8c53d..a58db3e6d6b2183fd2a9084590c399bb6cfaa6c6 100644 (file)
--- 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
@@ -311,6 +369,52 @@ public class SmartClientSmartServerTest extends HttpTestCase {
                                .getResponseHeader(HDR_CONTENT_TYPE));
        }
 
+       @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 (file)
index 0000000..fa8a9cf
--- /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 96a6fe1d0116c4266f1f1835edccdcbc17e94b86..fee9ae6a726102fd1321b237d6079c78b36086e7 100644 (file)
--- 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 09613fd7accbe1243f5bdf3f1a612f0498f4bfda..58081c1c90a93e456490e163cc26e1b554d48e5f 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/http/HttpConnection.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/http/HttpConnection.java
@@ -72,6 +72,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
         */

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 251381ab33d10caefa0e51c7efba37f33ddd53e2..9f828adc200c0950c7b80d5260e7b907fa307a38 100644 (file)
--- 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$
 
@@ -234,6 +240,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.
         *