From d4bd09b78daa733933a15733bc6ebbaa0a0485f1 Mon Sep 17 00:00:00 2001 From: Bo Zhang Date: Mon, 16 Jan 2017 14:19:17 +0800 Subject: [PATCH] Follow redirects in transport Bug: 465167 Change-Id: I6da19c8106201c2a1ac69002bd633b7387f25d96 Signed-off-by: Bo Zhang Signed-off-by: Matthias Sohn --- .../http/test/SmartClientSmartServerTest.java | 130 ++++++++++++++++-- org.eclipse.jgit/.settings/.api_filters | 11 ++ .../eclipse/jgit/transport/TransportHttp.java | 19 ++- .../jgit/transport/http/HttpConnection.java | 6 + .../org/eclipse/jgit/util/HttpSupport.java | 23 ++++ 5 files changed, 174 insertions(+), 15 deletions(-) create mode 100644 org.eclipse.jgit/.settings/.api_filters 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 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 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 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 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 @@ + + + + + + + + + + + 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 @@ -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 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$ @@ -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. * -- 2.39.5