From e17bfc96f293744cc5c0cef306e100f53d63bb3d Mon Sep 17 00:00:00 2001 From: Thomas Wolf Date: Wed, 22 Apr 2015 17:05:12 +0200 Subject: [PATCH] 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..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 Signed-off-by: Thomas Wolf --- org.eclipse.jgit.http.test/pom.xml | 3 - .../http/test/TestRepositoryResolver.java | 85 +++++ .../http/test/DumbClientSmartServerTest.java | 21 +- .../test/SmartClientSmartServerSslTest.java | 280 ++++++++++++++++ .../http/test/SmartClientSmartServerTest.java | 311 +++++++++++++++--- .../META-INF/MANIFEST.MF | 1 + .../eclipse/jgit/junit/http/AppServer.java | 136 +++++++- .../eclipse/jgit/junit/http/HttpTestCase.java | 51 ++- org.eclipse.jgit/.settings/.api_filters | 20 ++ .../eclipse/jgit/internal/JGitText.properties | 7 + .../org/eclipse/jgit/internal/JGitText.java | 7 + .../eclipse/jgit/transport/TransportHttp.java | 304 +++++++++++++---- .../jgit/transport/http/HttpConnection.java | 24 +- 13 files changed, 1108 insertions(+), 142 deletions(-) create mode 100644 org.eclipse.jgit.http.test/src/org/eclipse/jgit/http/test/TestRepositoryResolver.java create mode 100644 org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/SmartClientSmartServerSslTest.java diff --git a/org.eclipse.jgit.http.test/pom.xml b/org.eclipse.jgit.http.test/pom.xml index 7d9c17f6a..85b7c6844 100644 --- a/org.eclipse.jgit.http.test/pom.xml +++ b/org.eclipse.jgit.http.test/pom.xml @@ -87,14 +87,12 @@ org.eclipse.jgit org.eclipse.jgit.junit.http ${project.version} - test org.eclipse.jgit org.eclipse.jgit.junit ${project.version} - test @@ -107,7 +105,6 @@ org.eclipse.jetty jetty-servlet - test diff --git a/org.eclipse.jgit.http.test/src/org/eclipse/jgit/http/test/TestRepositoryResolver.java b/org.eclipse.jgit.http.test/src/org/eclipse/jgit/http/test/TestRepositoryResolver.java new file mode 100644 index 000000000..334e57c6d --- /dev/null +++ b/org.eclipse.jgit.http.test/src/org/eclipse/jgit/http/test/TestRepositoryResolver.java @@ -0,0 +1,85 @@ +/* + * Copyright (C) 2016, 2017 Google Inc. + * and other copyright owners as documented in the project's IP log. + * + * This program and the accompanying materials are made available + * under the terms of the Eclipse Distribution License v1.0 which + * accompanies this distribution, is reproduced below, and is + * available at http://www.eclipse.org/org/documents/edl-v10.php + * + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or + * without modification, are permitted provided that the following + * conditions are met: + * + * - Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * + * - Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following + * disclaimer in the documentation and/or other materials provided + * with the distribution. + * + * - Neither the name of the Eclipse Foundation, Inc. nor the + * names of its contributors may be used to endorse or promote + * products derived from this software without specific prior + * written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND + * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, + * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR + * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER + * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, + * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF + * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ +package org.eclipse.jgit.http.test; + +import javax.servlet.http.HttpServletRequest; + +import org.eclipse.jgit.errors.RepositoryNotFoundException; +import org.eclipse.jgit.junit.TestRepository; +import org.eclipse.jgit.lib.Repository; +import org.eclipse.jgit.transport.resolver.RepositoryResolver; +import org.eclipse.jgit.transport.resolver.ServiceNotEnabledException; + +/** A simple repository resolver for tests. */ +public final class TestRepositoryResolver + implements RepositoryResolver { + + private final TestRepository repo; + + private final String repoName; + + /** + * Creates a new {@link TestRepositoryResolver} that resolves the given name to + * the given repository. + * + * @param repo + * to resolve to + * @param repoName + * to match + */ + public TestRepositoryResolver(TestRepository repo, String repoName) { + this.repo = repo; + this.repoName = repoName; + } + + @Override + public Repository open(HttpServletRequest req, String name) + throws RepositoryNotFoundException, ServiceNotEnabledException { + if (!name.equals(repoName)) { + throw new RepositoryNotFoundException(name); + } + Repository db = repo.getRepository(); + db.incrementOpen(); + return db; + } +} diff --git a/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/DumbClientSmartServerTest.java b/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/DumbClientSmartServerTest.java index 06bfd7988..727f9bab0 100644 --- a/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/DumbClientSmartServerTest.java +++ b/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/DumbClientSmartServerTest.java @@ -1,5 +1,5 @@ /* - * Copyright (C) 2010, 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 @@ -60,12 +60,9 @@ import java.util.Collection; import java.util.List; import java.util.Map; -import javax.servlet.http.HttpServletRequest; - import org.eclipse.jetty.servlet.ServletContextHandler; import org.eclipse.jetty.servlet.ServletHolder; import org.eclipse.jgit.errors.NotSupportedException; -import org.eclipse.jgit.errors.RepositoryNotFoundException; import org.eclipse.jgit.http.server.GitServlet; import org.eclipse.jgit.junit.TestRepository; import org.eclipse.jgit.junit.http.AccessEvent; @@ -84,8 +81,6 @@ import org.eclipse.jgit.transport.URIish; import org.eclipse.jgit.transport.http.HttpConnectionFactory; 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.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -124,19 +119,7 @@ public class DumbClientSmartServerTest extends HttpTestCase { ServletContextHandler app = server.addContext("/git"); GitServlet gs = new GitServlet(); - gs.setRepositoryResolver(new RepositoryResolver() { - @Override - public Repository open(HttpServletRequest req, String name) - throws RepositoryNotFoundException, - ServiceNotEnabledException { - if (!name.equals(srcName)) - throw new RepositoryNotFoundException(name); - - final Repository db = src.getRepository(); - db.incrementOpen(); - return db; - } - }); + gs.setRepositoryResolver(new TestRepositoryResolver(src, srcName)); app.addServlet(new ServletHolder(gs), "/*"); server.setUp(); diff --git a/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/SmartClientSmartServerSslTest.java b/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/SmartClientSmartServerSslTest.java new file mode 100644 index 000000000..42064581c --- /dev/null +++ b/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/SmartClientSmartServerSslTest.java @@ -0,0 +1,280 @@ +/* + * Copyright (C) 2017 Thomas Wolf + * and other copyright owners as documented in the project's IP log. + * + * This program and the accompanying materials are made available + * under the terms of the Eclipse Distribution License v1.0 which + * accompanies this distribution, is reproduced below, and is + * available at http://www.eclipse.org/org/documents/edl-v10.php + * + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or + * without modification, are permitted provided that the following + * conditions are met: + * + * - Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * + * - Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following + * disclaimer in the documentation and/or other materials provided + * with the distribution. + * + * - Neither the name of the Eclipse Foundation, Inc. nor the + * names of its contributors may be used to endorse or promote + * products derived from this software without specific prior + * written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND + * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, + * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR + * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER + * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, + * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF + * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +package org.eclipse.jgit.http.test; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +import java.io.IOException; +import java.util.Arrays; +import java.util.Collection; +import java.util.EnumSet; +import java.util.List; + +import javax.servlet.DispatcherType; +import javax.servlet.Filter; +import javax.servlet.FilterChain; +import javax.servlet.FilterConfig; +import javax.servlet.ServletException; +import javax.servlet.ServletRequest; +import javax.servlet.ServletResponse; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +import org.eclipse.jetty.servlet.FilterHolder; +import org.eclipse.jetty.servlet.ServletContextHandler; +import org.eclipse.jetty.servlet.ServletHolder; +import org.eclipse.jgit.errors.TransportException; +import org.eclipse.jgit.http.server.GitServlet; +import org.eclipse.jgit.junit.TestRepository; +import org.eclipse.jgit.junit.http.AccessEvent; +import org.eclipse.jgit.junit.http.AppServer; +import org.eclipse.jgit.junit.http.HttpTestCase; +import org.eclipse.jgit.lib.ConfigConstants; +import org.eclipse.jgit.lib.NullProgressMonitor; +import org.eclipse.jgit.lib.Repository; +import org.eclipse.jgit.revwalk.RevBlob; +import org.eclipse.jgit.revwalk.RevCommit; +import org.eclipse.jgit.storage.file.FileBasedConfig; +import org.eclipse.jgit.transport.HttpTransport; +import org.eclipse.jgit.transport.Transport; +import org.eclipse.jgit.transport.URIish; +import org.eclipse.jgit.transport.http.HttpConnectionFactory; +import org.eclipse.jgit.transport.http.JDKHttpConnectionFactory; +import org.eclipse.jgit.transport.http.apache.HttpClientConnectionFactory; +import org.eclipse.jgit.util.FS; +import org.eclipse.jgit.util.HttpSupport; +import org.eclipse.jgit.util.SystemReader; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameters; + +@RunWith(Parameterized.class) +public class SmartClientSmartServerSslTest extends HttpTestCase { + + private URIish remoteURI; + + private URIish secureURI; + + private RevBlob A_txt; + + private RevCommit A, B; + + @Parameters + public static Collection data() { + // run all tests with both connection factories we have + return Arrays.asList(new Object[][] { + { new JDKHttpConnectionFactory() }, + { new HttpClientConnectionFactory() } }); + } + + public SmartClientSmartServerSslTest(HttpConnectionFactory cf) { + HttpTransport.setConnectionFactory(cf); + } + + @Override + protected AppServer createServer() { + return new AppServer(0, 0); + } + + @Override + @Before + public void setUp() throws Exception { + super.setUp(); + + final TestRepository src = createTestRepository(); + final String srcName = src.getRepository().getDirectory().getName(); + src.getRepository() + .getConfig() + .setBoolean(ConfigConstants.CONFIG_CORE_SECTION, null, + ConfigConstants.CONFIG_KEY_LOGALLREFUPDATES, true); + + GitServlet gs = new GitServlet(); + + ServletContextHandler app = addNormalContext(gs, src, srcName); + + server.setUp(); + + remoteURI = toURIish(app, srcName); + secureURI = new URIish(rewriteUrl(remoteURI.toString(), "https", + server.getSecurePort())); + + 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); + + FileBasedConfig userConfig = SystemReader.getInstance() + .openUserConfig(null, FS.DETECTED); + userConfig.setBoolean("http", null, "sslVerify", false); + userConfig.save(); + } + + private ServletContextHandler addNormalContext(GitServlet gs, TestRepository src, String srcName) { + ServletContextHandler app = server.addContext("/git"); + app.addFilter(new FilterHolder(new Filter() { + + @Override + public void init(FilterConfig filterConfig) + throws ServletException { + // empty + } + + // Redirects http to https for requests containing "/https/". + @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()); + } + String urlString = rewriteUrl(fullUrl.toString(), "https", + server.getSecurePort()); + httpServletResponse + .setStatus(HttpServletResponse.SC_MOVED_PERMANENTLY); + httpServletResponse.setHeader(HttpSupport.HDR_LOCATION, + urlString.replace("/https/", "/")); + } + + @Override + public void destroy() { + // empty + } + }), "/https/*", EnumSet.of(DispatcherType.REQUEST)); + app.addFilter(new FilterHolder(new Filter() { + + @Override + public void init(FilterConfig filterConfig) + throws ServletException { + // empty + } + + // Redirects https back to http for requests containing "/back/". + @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()); + } + String urlString = rewriteUrl(fullUrl.toString(), "http", + server.getPort()); + httpServletResponse + .setStatus(HttpServletResponse.SC_MOVED_PERMANENTLY); + httpServletResponse.setHeader(HttpSupport.HDR_LOCATION, + urlString.replace("/back/", "/")); + } + + @Override + public void destroy() { + // empty + } + }), "/back/*", EnumSet.of(DispatcherType.REQUEST)); + gs.setRepositoryResolver(new TestRepositoryResolver(src, srcName)); + app.addServlet(new ServletHolder(gs), "/*"); + return app; + } + + @Test + public void testInitialClone_ViaHttps() throws Exception { + Repository dst = createBareRepository(); + assertFalse(dst.hasObject(A_txt)); + + try (Transport t = Transport.open(dst, secureURI)) { + t.fetch(NullProgressMonitor.INSTANCE, mirror(master)); + } + assertTrue(dst.hasObject(A_txt)); + assertEquals(B, dst.exactRef(master).getObjectId()); + fsck(dst, B); + + List requests = getRequests(); + assertEquals(2, requests.size()); + } + + @Test + public void testInitialClone_RedirectToHttps() throws Exception { + Repository dst = createBareRepository(); + assertFalse(dst.hasObject(A_txt)); + + URIish cloneFrom = extendPath(remoteURI, "/https"); + try (Transport t = Transport.open(dst, cloneFrom)) { + t.fetch(NullProgressMonitor.INSTANCE, mirror(master)); + } + assertTrue(dst.hasObject(A_txt)); + assertEquals(B, dst.exactRef(master).getObjectId()); + fsck(dst, B); + + List requests = getRequests(); + assertEquals(3, requests.size()); + } + + @Test + public void testInitialClone_RedirectBackToHttp() throws Exception { + Repository dst = createBareRepository(); + assertFalse(dst.hasObject(A_txt)); + + URIish cloneFrom = extendPath(secureURI, "/back"); + try (Transport t = Transport.open(dst, cloneFrom)) { + t.fetch(NullProgressMonitor.INSTANCE, mirror(master)); + fail("Should have failed (redirect from https to http)"); + } catch (TransportException e) { + assertTrue(e.getMessage().contains("not allowed")); + } + } + +} 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 ed223c96e..8cadca523 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 @@ -1,5 +1,5 @@ /* - * Copyright (C) 2010, 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 @@ -57,17 +57,21 @@ import java.io.IOException; import java.io.PrintWriter; import java.net.URISyntaxException; import java.nio.charset.StandardCharsets; +import java.text.MessageFormat; import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.EnumSet; import java.util.List; import java.util.Map; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import javax.servlet.DispatcherType; import javax.servlet.Filter; import javax.servlet.FilterChain; import javax.servlet.FilterConfig; +import javax.servlet.RequestDispatcher; import javax.servlet.ServletException; import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; @@ -78,7 +82,6 @@ import org.eclipse.jetty.servlet.FilterHolder; import org.eclipse.jetty.servlet.ServletContextHandler; import org.eclipse.jetty.servlet.ServletHolder; import org.eclipse.jgit.errors.RemoteRepositoryException; -import org.eclipse.jgit.errors.RepositoryNotFoundException; import org.eclipse.jgit.errors.TransportException; import org.eclipse.jgit.http.server.GitServlet; import org.eclipse.jgit.internal.JGitText; @@ -101,6 +104,7 @@ import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.lib.StoredConfig; import org.eclipse.jgit.revwalk.RevBlob; import org.eclipse.jgit.revwalk.RevCommit; +import org.eclipse.jgit.storage.file.FileBasedConfig; import org.eclipse.jgit.transport.FetchConnection; import org.eclipse.jgit.transport.HttpTransport; import org.eclipse.jgit.transport.RemoteRefUpdate; @@ -110,9 +114,9 @@ import org.eclipse.jgit.transport.URIish; import org.eclipse.jgit.transport.http.HttpConnectionFactory; 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.FS; import org.eclipse.jgit.util.HttpSupport; +import org.eclipse.jgit.util.SystemReader; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -184,7 +188,52 @@ public class SmartClientSmartServerTest extends HttpTestCase { private ServletContextHandler addNormalContext(GitServlet gs, TestRepository src, String srcName) { ServletContextHandler app = server.addContext("/git"); - gs.setRepositoryResolver(new TestRepoResolver(src, srcName)); + app.addFilter(new FilterHolder(new Filter() { + + @Override + public void init(FilterConfig filterConfig) + throws ServletException { + // empty + } + + // Does an internal forward for GET requests containing "/post/", + // and issues a 301 redirect on POST requests for such URLs. Used + // in the POST redirect tests. + @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()); + } + String urlString = fullUrl.toString(); + if ("POST".equalsIgnoreCase(httpServletRequest.getMethod())) { + httpServletResponse.setStatus( + HttpServletResponse.SC_MOVED_PERMANENTLY); + httpServletResponse.setHeader(HttpSupport.HDR_LOCATION, + urlString.replace("/post/", "/")); + } else { + String path = httpServletRequest.getPathInfo(); + path = path.replace("/post/", "/"); + if (httpServletRequest.getQueryString() != null) { + path += '?' + httpServletRequest.getQueryString(); + } + RequestDispatcher dispatcher = httpServletRequest + .getRequestDispatcher(path); + dispatcher.forward(httpServletRequest, httpServletResponse); + } + } + + @Override + public void destroy() { + // empty + } + }), "/post/*", EnumSet.of(DispatcherType.REQUEST)); + gs.setRepositoryResolver(new TestRepositoryResolver(src, srcName)); app.addServlet(new ServletHolder(gs), "/*"); return app; } @@ -228,6 +277,12 @@ public class SmartClientSmartServerTest extends HttpTestCase { ServletContextHandler redirect = server.addContext("/redirect"); redirect.addFilter(new FilterHolder(new Filter() { + // Enables tests for different codes, and for multiple redirects. + // First parameter is the number of redirects, second one is the + // redirect status code that should be used + private Pattern responsePattern = Pattern + .compile("/response/(\\d+)/(30[1237])/"); + @Override public void init(FilterConfig filterConfig) throws ServletException { @@ -245,10 +300,43 @@ public class SmartClientSmartServerTest extends HttpTestCase { fullUrl.append("?") .append(httpServletRequest.getQueryString()); } - httpServletResponse - .setStatus(HttpServletResponse.SC_MOVED_PERMANENTLY); + String urlString = fullUrl.toString(); + if (urlString.contains("/loop/")) { + urlString = urlString.replace("/loop/", "/loop/x/"); + if (urlString.contains("/loop/x/x/x/x/x/x/x/x/")) { + // Go back to initial. + urlString = urlString.replace("/loop/x/x/x/x/x/x/x/x/", + "/loop/"); + } + httpServletResponse.setStatus( + HttpServletResponse.SC_MOVED_TEMPORARILY); + httpServletResponse.setHeader(HttpSupport.HDR_LOCATION, + urlString); + return; + } + int responseCode = HttpServletResponse.SC_MOVED_PERMANENTLY; + int nofRedirects = 0; + Matcher matcher = responsePattern.matcher(urlString); + if (matcher.find()) { + nofRedirects = Integer + .parseUnsignedInt(matcher.group(1)); + responseCode = Integer.parseUnsignedInt(matcher.group(2)); + if (--nofRedirects <= 0) { + urlString = fullUrl.substring(0, matcher.start()) + '/' + + fullUrl.substring(matcher.end()); + } else { + urlString = fullUrl.substring(0, matcher.start()) + + "/response/" + nofRedirects + "/" + + responseCode + '/' + + fullUrl.substring(matcher.end()); + } + } + httpServletResponse.setStatus(responseCode); + if (nofRedirects <= 0) { + urlString = urlString.replace("/redirect", "/git"); + } httpServletResponse.setHeader(HttpSupport.HDR_LOCATION, - fullUrl.toString().replace("/redirect", "/git")); + urlString); } @Override @@ -373,12 +461,17 @@ public class SmartClientSmartServerTest extends HttpTestCase { .getResponseHeader(HDR_CONTENT_TYPE)); } - @Test - public void testInitialClone_RedirectSmall() throws Exception { + private void initialClone_Redirect(int nofRedirects, int code) + throws Exception { Repository dst = createBareRepository(); assertFalse(dst.hasObject(A_txt)); - try (Transport t = Transport.open(dst, redirectURI)) { + URIish cloneFrom = redirectURI; + if (code != 301 || nofRedirects > 1) { + cloneFrom = extendPath(cloneFrom, + "/response/" + nofRedirects + "/" + code); + } + try (Transport t = Transport.open(dst, cloneFrom)) { t.fetch(NullProgressMonitor.INSTANCE, mirror(master)); } @@ -387,12 +480,15 @@ public class SmartClientSmartServerTest extends HttpTestCase { fsck(dst, B); List requests = getRequests(); - assertEquals(4, requests.size()); + assertEquals(2 + nofRedirects, requests.size()); - AccessEvent firstRedirect = requests.get(0); - assertEquals(301, firstRedirect.getStatus()); + int n = 0; + while (n < nofRedirects) { + AccessEvent redirect = requests.get(n++); + assertEquals(code, redirect.getStatus()); + } - AccessEvent info = requests.get(1); + AccessEvent info = requests.get(n++); assertEquals("GET", info.getMethod()); assertEquals(join(remoteURI, "info/refs"), info.getPath()); assertEquals(1, info.getParameters().size()); @@ -402,10 +498,130 @@ public class SmartClientSmartServerTest extends HttpTestCase { 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(n++); + 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 testInitialClone_Redirect301Small() throws Exception { + initialClone_Redirect(1, 301); + } + + @Test + public void testInitialClone_Redirect302Small() throws Exception { + initialClone_Redirect(1, 302); + } + + @Test + public void testInitialClone_Redirect303Small() throws Exception { + initialClone_Redirect(1, 303); + } + + @Test + public void testInitialClone_Redirect307Small() throws Exception { + initialClone_Redirect(1, 307); + } + + @Test + public void testInitialClone_RedirectMultiple() throws Exception { + initialClone_Redirect(4, 302); + } + + @Test + public void testInitialClone_RedirectMax() throws Exception { + FileBasedConfig userConfig = SystemReader.getInstance() + .openUserConfig(null, FS.DETECTED); + userConfig.setInt("http", null, "maxRedirects", 4); + userConfig.save(); + initialClone_Redirect(4, 302); + } + + @Test + public void testInitialClone_RedirectTooOften() throws Exception { + FileBasedConfig userConfig = SystemReader.getInstance() + .openUserConfig(null, FS.DETECTED); + userConfig.setInt("http", null, "maxRedirects", 3); + userConfig.save(); + Repository dst = createBareRepository(); + assertFalse(dst.hasObject(A_txt)); + + URIish cloneFrom = extendPath(redirectURI, "/response/4/302"); + String remoteUri = cloneFrom.toString(); + try (Transport t = Transport.open(dst, cloneFrom)) { + t.fetch(NullProgressMonitor.INSTANCE, mirror(master)); + fail("Should have failed (too many redirects)"); + } catch (TransportException e) { + String expectedMessageBegin = MessageFormat.format( + JGitText.get().redirectLimitExceeded, remoteUri, "3", + remoteUri.replace("/4/", "/1/") + '/', ""); + String message = e.getMessage(); + if (message.length() > expectedMessageBegin.length()) { + message = message.substring(0, expectedMessageBegin.length()); + } + assertEquals(expectedMessageBegin, message); + } + } + + @Test + public void testInitialClone_RedirectLoop() throws Exception { + Repository dst = createBareRepository(); + assertFalse(dst.hasObject(A_txt)); + + URIish cloneFrom = extendPath(redirectURI, "/loop"); + try (Transport t = Transport.open(dst, cloneFrom)) { + t.fetch(NullProgressMonitor.INSTANCE, mirror(master)); + fail("Should have failed (redirect loop)"); + } catch (TransportException e) { + assertTrue(e.getMessage().contains("redirected more than")); + } + } + + @Test + public void testInitialClone_RedirectOnPostAllowed() throws Exception { + FileBasedConfig userConfig = SystemReader.getInstance() + .openUserConfig(null, FS.DETECTED); + userConfig.setString("http", null, "followRedirects", "true"); + userConfig.save(); + Repository dst = createBareRepository(); + assertFalse(dst.hasObject(A_txt)); + + URIish cloneFrom = extendPath(remoteURI, "/post"); + try (Transport t = Transport.open(dst, cloneFrom)) { + t.fetch(NullProgressMonitor.INSTANCE, mirror(master)); + } + + assertTrue(dst.hasObject(A_txt)); + assertEquals(B, dst.exactRef(master).getObjectId()); + fsck(dst, B); + + List requests = getRequests(); + assertEquals(3, requests.size()); + + AccessEvent info = requests.get(0); + assertEquals("GET", info.getMethod()); + assertEquals(join(cloneFrom, "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 redirect = requests.get(1); + assertEquals("POST", redirect.getMethod()); + assertEquals(301, redirect.getStatus()); - AccessEvent service = requests.get(3); + AccessEvent service = requests.get(2); assertEquals("POST", service.getMethod()); assertEquals(join(remoteURI, "git-upload-pack"), service.getPath()); assertEquals(0, service.getParameters().size()); @@ -419,6 +635,39 @@ public class SmartClientSmartServerTest extends HttpTestCase { service.getResponseHeader(HDR_CONTENT_TYPE)); } + @Test + public void testInitialClone_RedirectOnPostForbidden() throws Exception { + Repository dst = createBareRepository(); + assertFalse(dst.hasObject(A_txt)); + + URIish cloneFrom = extendPath(remoteURI, "/post"); + try (Transport t = Transport.open(dst, cloneFrom)) { + t.fetch(NullProgressMonitor.INSTANCE, mirror(master)); + fail("Should have failed (redirect on POST)"); + } catch (TransportException e) { + assertTrue(e.getMessage().contains("301")); + } + } + + @Test + public void testInitialClone_RedirectForbidden() throws Exception { + FileBasedConfig userConfig = SystemReader.getInstance() + .openUserConfig(null, FS.DETECTED); + userConfig.setString("http", null, "followRedirects", "false"); + userConfig.save(); + + Repository dst = createBareRepository(); + assertFalse(dst.hasObject(A_txt)); + + try (Transport t = Transport.open(dst, redirectURI)) { + t.fetch(NullProgressMonitor.INSTANCE, mirror(master)); + fail("Should have failed (redirects forbidden)"); + } catch (TransportException e) { + assertTrue( + e.getMessage().contains("http.followRedirects is false")); + } + } + @Test public void testFetch_FewLocalCommits() throws Exception { // Bootstrap by doing the clone. @@ -619,7 +868,7 @@ public class SmartClientSmartServerTest extends HttpTestCase { ServletContextHandler app = noRefServer.addContext("/git"); GitServlet gs = new GitServlet(); - gs.setRepositoryResolver(new TestRepoResolver(repo, repoName)); + gs.setRepositoryResolver(new TestRepositoryResolver(repo, repoName)); app.addServlet(new ServletHolder(gs), "/*"); noRefServer.setUp(); @@ -822,28 +1071,4 @@ public class SmartClientSmartServerTest extends HttpTestCase { cfg.save(); } - private final class TestRepoResolver - implements RepositoryResolver { - - private final TestRepository repo; - - private final String repoName; - - private TestRepoResolver(TestRepository repo, - String repoName) { - this.repo = repo; - this.repoName = repoName; - } - - @Override - public Repository open(HttpServletRequest req, String name) - throws RepositoryNotFoundException, ServiceNotEnabledException { - if (!name.equals(repoName)) - throw new RepositoryNotFoundException(name); - - Repository db = repo.getRepository(); - db.incrementOpen(); - return db; - } - } } diff --git a/org.eclipse.jgit.junit.http/META-INF/MANIFEST.MF b/org.eclipse.jgit.junit.http/META-INF/MANIFEST.MF index dd6cb48b1..f5e303384 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 28c0f2111..69e2cd595 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 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. *

@@ -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 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 1b94e02fa..eabb0f225 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 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); + } } diff --git a/org.eclipse.jgit/.settings/.api_filters b/org.eclipse.jgit/.settings/.api_filters index 83da4c5e3..dbd754708 100644 --- a/org.eclipse.jgit/.settings/.api_filters +++ b/org.eclipse.jgit/.settings/.api_filters @@ -52,4 +52,24 @@ + + + + + + + + + + + + + + + + + + + + diff --git a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties index fc29f481d..22e60cbb5 100644 --- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties +++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties @@ -365,12 +365,14 @@ invalidPathContainsSeparator=Invalid path (contains separator ''{0}''): {1} invalidPathPeriodAtEndWindows=Invalid path (period at end is ignored by Windows): {0} invalidPathSpaceAtEndWindows=Invalid path (space at end is ignored by Windows): {0} invalidPathReservedOnWindows=Invalid path (''{0}'' is reserved on Windows): {1} +invalidRedirectLocation=Redirect or URI ''{0}'': invalid redirect location {1} -> {2} invalidReflogRevision=Invalid reflog revision: {0} invalidRefName=Invalid ref name: {0} invalidRemote=Invalid remote: {0} invalidRepositoryStateNoHead=Invalid repository --- cannot read HEAD invalidShallowObject=invalid shallow object {0}, expected commit invalidStageForPath=Invalid stage {0} for path {1} +invalidSystemProperty=Invalid system property ''{0}'': ''{1}''; using default value {2} invalidTagOption=Invalid tag option: {0} invalidTimeout=Invalid timeout: {0} invalidTimeUnitValue2=Invalid time unit value: {0}.{1}={2} @@ -528,6 +530,11 @@ receivePackObjectTooLarge2=Object too large ({0} bytes), rejecting the pack. Max receivePackInvalidLimit=Illegal limit parameter value {0} receivePackTooLarge=Pack exceeds the limit of {0} bytes, rejecting the pack receivingObjects=Receiving objects +redirectBlocked=URI ''{0}'' redirection blocked: redirect {1} -> {2} not allowed +redirectHttp=URI ''{0}'': following HTTP redirect #{1} {2} -> {3} +redirectLimitExceeded=URI ''{0}'' redirected more than {1} times; aborted at {2} -> {3} +redirectLocationMissing=Invalid redirect of ''{0}'': no redirect location for {1} +redirectsOff=Cannot redirect ''{0}'': http.followRedirects is false (HTTP status {1}) refAlreadyExists=already exists refAlreadyExists1=Ref {0} already exists reflogEntryNotFound=Entry {0} not found in reflog for ''{1}'' diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java index a68f839c5..610b99c14 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java @@ -424,11 +424,13 @@ public class JGitText extends TranslationBundle { /***/ public String invalidPathPeriodAtEndWindows; /***/ public String invalidPathSpaceAtEndWindows; /***/ public String invalidPathReservedOnWindows; + /***/ public String invalidRedirectLocation; /***/ public String invalidReflogRevision; /***/ public String invalidRefName; /***/ public String invalidRemote; /***/ public String invalidShallowObject; /***/ public String invalidStageForPath; + /***/ public String invalidSystemProperty; /***/ public String invalidTagOption; /***/ public String invalidTimeout; /***/ public String invalidTimeUnitValue2; @@ -587,6 +589,11 @@ public class JGitText extends TranslationBundle { /***/ public String receivePackInvalidLimit; /***/ public String receivePackTooLarge; /***/ public String receivingObjects; + /***/ public String redirectBlocked; + /***/ public String redirectHttp; + /***/ public String redirectLimitExceeded; + /***/ public String redirectLocationMissing; + /***/ public String redirectsOff; /***/ public String refAlreadyExists; /***/ public String refAlreadyExists1; /***/ public String reflogEntryNotFound; 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 1bdecdb18..327dc39c7 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransportHttp.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransportHttp.java @@ -2,6 +2,7 @@ * Copyright (C) 2008-2010, Google Inc. * Copyright (C) 2008, Shawn O. Pearce * Copyright (C) 2013, Matthias Sohn + * Copyright (C) 2017, Thomas Wolf * and other copyright owners as documented in the project's IP log. * * This program and the accompanying materials are made available @@ -69,6 +70,7 @@ import java.io.OutputStream; import java.net.MalformedURLException; import java.net.Proxy; import java.net.ProxySelector; +import java.net.URISyntaxException; import java.net.URL; import java.text.MessageFormat; import java.util.ArrayList; @@ -78,9 +80,11 @@ import java.util.Collections; import java.util.EnumSet; import java.util.HashSet; import java.util.LinkedHashSet; +import java.util.Locale; import java.util.Map; import java.util.Set; import java.util.TreeMap; +import java.util.function.Supplier; import java.util.zip.GZIPInputStream; import java.util.zip.GZIPOutputStream; @@ -103,9 +107,12 @@ import org.eclipse.jgit.transport.http.HttpConnection; import org.eclipse.jgit.util.HttpSupport; import org.eclipse.jgit.util.IO; import org.eclipse.jgit.util.RawParseUtils; +import org.eclipse.jgit.util.SystemReader; import org.eclipse.jgit.util.TemporaryBuffer; import org.eclipse.jgit.util.io.DisabledOutputStream; import org.eclipse.jgit.util.io.UnionInputStream; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * Transport over HTTP and FTP protocols. @@ -126,10 +133,37 @@ import org.eclipse.jgit.util.io.UnionInputStream; public class TransportHttp extends HttpTransport implements WalkTransport, PackTransport { + private static final Logger LOG = LoggerFactory + .getLogger(TransportHttp.class); + private static final String SVC_UPLOAD_PACK = "git-upload-pack"; //$NON-NLS-1$ private static final String SVC_RECEIVE_PACK = "git-receive-pack"; //$NON-NLS-1$ + private static final String MAX_REDIRECT_SYSTEM_PROPERTY = "http.maxRedirects"; //$NON-NLS-1$ + + private static final int DEFAULT_MAX_REDIRECTS = 5; + + private static final int MAX_REDIRECTS = (new Supplier() { + + @Override + public Integer get() { + String rawValue = SystemReader.getInstance() + .getProperty(MAX_REDIRECT_SYSTEM_PROPERTY); + Integer value = Integer.valueOf(DEFAULT_MAX_REDIRECTS); + if (rawValue != null) { + try { + value = Integer.valueOf(Integer.parseUnsignedInt(rawValue)); + } catch (NumberFormatException e) { + LOG.warn(MessageFormat.format( + JGitText.get().invalidSystemProperty, + MAX_REDIRECT_SYSTEM_PROPERTY, rawValue, value)); + } + } + return value; + } + }).get().intValue(); + /** * Accept-Encoding header in the HTTP request * (https://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html). @@ -230,14 +264,58 @@ public class TransportHttp extends HttpTransport implements WalkTransport, } }; + /** + * Config values for http.followRedirect + */ + private static enum HttpRedirectMode implements Config.ConfigEnum { + + /** Always follow redirects (up to the http.maxRedirects limit). */ + TRUE("true"), //$NON-NLS-1$ + /** + * Only follow redirects on the initial GET request. This is the + * default. + */ + INITIAL("initial"), //$NON-NLS-1$ + /** Never follow redirects. */ + FALSE("false"); //$NON-NLS-1$ + + private final String configValue; + + private HttpRedirectMode(String configValue) { + this.configValue = configValue; + } + + @Override + public String toConfigValue() { + return configValue; + } + + @Override + public boolean matchConfigValue(String s) { + return configValue.equals(s); + } + } + private static class HttpConfig { final int postBuffer; final boolean sslVerify; + final HttpRedirectMode followRedirects; + + final int maxRedirects; + HttpConfig(final Config rc) { postBuffer = rc.getInt("http", "postbuffer", 1 * 1024 * 1024); //$NON-NLS-1$ //$NON-NLS-2$ sslVerify = rc.getBoolean("http", "sslVerify", true); //$NON-NLS-1$ //$NON-NLS-2$ + followRedirects = rc.getEnum(HttpRedirectMode.values(), "http", //$NON-NLS-1$ + null, "followRedirects", HttpRedirectMode.INITIAL); //$NON-NLS-1$ + int redirectLimit = rc.getInt("http", "maxRedirects", //$NON-NLS-1$ //$NON-NLS-2$ + MAX_REDIRECTS); + if (redirectLimit < 0) { + redirectLimit = MAX_REDIRECTS; + } + maxRedirects = redirectLimit; } HttpConfig() { @@ -245,9 +323,9 @@ public class TransportHttp extends HttpTransport implements WalkTransport, } } - final URL baseUrl; + private URL baseUrl; - private final URL objectsUrl; + private URL objectsUrl; final HttpConfig http; @@ -262,17 +340,31 @@ public class TransportHttp extends HttpTransport implements WalkTransport, TransportHttp(final Repository local, final URIish uri) throws NotSupportedException { super(local, uri); + setURI(uri); + http = local.getConfig().get(HttpConfig::new); + proxySelector = ProxySelector.getDefault(); + } + + private URL toURL(URIish urish) throws MalformedURLException { + String uriString = urish.toString(); + if (!uriString.endsWith("/")) { //$NON-NLS-1$ + uriString += '/'; + } + return new URL(uriString); + } + + /** + * @param uri + * @throws NotSupportedException + * @since 4.9 + */ + protected void setURI(final URIish uri) throws NotSupportedException { try { - String uriString = uri.toString(); - if (!uriString.endsWith("/")) //$NON-NLS-1$ - uriString += "/"; //$NON-NLS-1$ - baseUrl = new URL(uriString); + baseUrl = toURL(uri); objectsUrl = new URL(baseUrl, "objects/"); //$NON-NLS-1$ } catch (MalformedURLException e) { throw new NotSupportedException(MessageFormat.format(JGitText.get().invalidURL, uri), e); } - http = local.getConfig().get(HttpConfig::new); - proxySelector = ProxySelector.getDefault(); } /** @@ -283,15 +375,7 @@ public class TransportHttp extends HttpTransport implements WalkTransport, */ TransportHttp(final URIish uri) throws NotSupportedException { super(uri); - try { - String uriString = uri.toString(); - if (!uriString.endsWith("/")) //$NON-NLS-1$ - uriString += "/"; //$NON-NLS-1$ - baseUrl = new URL(uriString); - objectsUrl = new URL(baseUrl, "objects/"); //$NON-NLS-1$ - } catch (MalformedURLException e) { - throw new NotSupportedException(MessageFormat.format(JGitText.get().invalidURL, uri), e); - } + setURI(uri); http = new HttpConfig(); proxySelector = ProxySelector.getDefault(); } @@ -461,28 +545,9 @@ public class TransportHttp extends HttpTransport implements WalkTransport, private HttpConnection connect(final String service) throws TransportException, NotSupportedException { - final URL u; - try { - final StringBuilder b = new StringBuilder(); - b.append(baseUrl); - - if (b.charAt(b.length() - 1) != '/') - b.append('/'); - b.append(Constants.INFO_REFS); - - if (useSmartHttp) { - b.append(b.indexOf("?") < 0 ? '?' : '&'); //$NON-NLS-1$ - b.append("service="); //$NON-NLS-1$ - b.append(service); - } - - u = new URL(b.toString()); - } catch (MalformedURLException e) { - throw new NotSupportedException(MessageFormat.format(JGitText.get().invalidURL, uri), e); - } - - + URL u = getServiceURL(service); int authAttempts = 1; + int redirects = 0; Collection ignoreTypes = null; for (;;) { try { @@ -532,6 +597,24 @@ public class TransportHttp extends HttpTransport implements WalkTransport, throw new TransportException(uri, MessageFormat.format( JGitText.get().serviceNotPermitted, service)); + case HttpConnection.HTTP_MOVED_PERM: + case HttpConnection.HTTP_MOVED_TEMP: + case HttpConnection.HTTP_SEE_OTHER: + case HttpConnection.HTTP_11_MOVED_TEMP: + // SEE_OTHER should actually never be sent by a git server, + // and in general should occur only on POST requests. But it + // doesn't hurt to accept it here as a redirect. + if (http.followRedirects == HttpRedirectMode.FALSE) { + throw new TransportException(MessageFormat.format( + JGitText.get().redirectsOff, uri, + Integer.valueOf(status))); + } + URIish newUri = redirect(conn.getHeaderField(HDR_LOCATION), + Constants.INFO_REFS, redirects++); + setURI(newUri); + u = getServiceURL(service); + authAttempts = 1; + break; default: String err = status + " " + conn.getResponseMessage(); //$NON-NLS-1$ throw new TransportException(uri, err); @@ -560,6 +643,86 @@ public class TransportHttp extends HttpTransport implements WalkTransport, } } + private URIish redirect(String location, String checkFor, int redirects) + throws TransportException { + if (location == null || location.isEmpty()) { + throw new TransportException(MessageFormat.format( + JGitText.get().redirectLocationMissing, uri, baseUrl)); + } + if (redirects >= http.maxRedirects) { + throw new TransportException(MessageFormat.format( + JGitText.get().redirectLimitExceeded, uri, + Integer.valueOf(http.maxRedirects), baseUrl, location)); + } + try { + if (!isValidRedirect(baseUrl, location, checkFor)) { + throw new TransportException( + MessageFormat.format(JGitText.get().redirectBlocked, + uri, baseUrl, location)); + } + location = location.substring(0, location.indexOf(checkFor)); + URIish result = new URIish(location); + if (LOG.isInfoEnabled()) { + LOG.info(MessageFormat.format(JGitText.get().redirectHttp, uri, + Integer.valueOf(redirects), baseUrl, result)); + } + return result; + } catch (URISyntaxException e) { + throw new TransportException(MessageFormat.format( + JGitText.get().invalidRedirectLocation, + uri, baseUrl, location), e); + } + } + + private boolean isValidRedirect(URL current, String next, String checkFor) { + // Protocols must be the same, or current is "http" and next "https". We + // do not follow redirects from https back to http. + String oldProtocol = current.getProtocol().toLowerCase(Locale.ROOT); + int schemeEnd = next.indexOf("://"); //$NON-NLS-1$ + if (schemeEnd < 0) { + return false; + } + String newProtocol = next.substring(0, schemeEnd) + .toLowerCase(Locale.ROOT); + if (!oldProtocol.equals(newProtocol)) { + if (!"https".equals(newProtocol)) { //$NON-NLS-1$ + return false; + } + } + // git allows only rewriting the root, i.e., everything before INFO_REFS + // or the service name + if (next.indexOf(checkFor) < 0) { + return false; + } + // Basically we should test here that whatever follows INFO_REFS is + // unchanged. But since we re-construct the query part + // anyway, it doesn't matter. + return true; + } + + private URL getServiceURL(final String service) + throws NotSupportedException { + try { + final StringBuilder b = new StringBuilder(); + b.append(baseUrl); + + if (b.charAt(b.length() - 1) != '/') { + b.append('/'); + } + b.append(Constants.INFO_REFS); + + if (useSmartHttp) { + b.append(b.indexOf("?") < 0 ? '?' : '&'); //$NON-NLS-1$ + b.append("service="); //$NON-NLS-1$ + b.append(service); + } + + return new URL(b.toString()); + } catch (MalformedURLException e) { + throw new NotSupportedException(MessageFormat.format(JGitText.get().invalidURL, uri), e); + } + } + /** * Open an HTTP connection, setting the accept-encoding request header to gzip. * @@ -598,6 +761,10 @@ public class TransportHttp extends HttpTransport implements WalkTransport, HttpSupport.disableSslVerify(conn); } + // We must do our own redirect handling to implement git rules and to + // handle http->https redirects + conn.setInstanceFollowRedirects(false); + conn.setRequestMethod(method); conn.setUseCaches(false); if (acceptEncoding == AcceptEncoding.GZIP) { @@ -906,13 +1073,7 @@ public class TransportHttp extends HttpTransport implements WalkTransport, } void openStream() throws IOException { - openStream(null); - } - - void openStream(final String redirectUrl) throws IOException { - conn = httpOpen( - METHOD_POST, - redirectUrl == null ? new URL(baseUrl, serviceName) : new URL(redirectUrl), + conn = httpOpen(METHOD_POST, new URL(baseUrl, serviceName), AcceptEncoding.GZIP); conn.setInstanceFollowRedirects(false); conn.setDoOutput(true); @@ -921,10 +1082,6 @@ 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 { @@ -939,21 +1096,42 @@ public class TransportHttp extends HttpTransport implements WalkTransport, buf = out; } - openStream(redirectUrl); - if (buf != out) - conn.setRequestProperty(HDR_CONTENT_ENCODING, ENCODING_GZIP); - conn.setFixedLengthStreamingMode((int) buf.length()); - final OutputStream httpOut = conn.getOutputStream(); - try { - buf.writeTo(httpOut, null); - } finally { - httpOut.close(); - } + int redirects = 0; + for (;;) { + openStream(); + if (buf != out) { + conn.setRequestProperty(HDR_CONTENT_ENCODING, ENCODING_GZIP); + } + conn.setFixedLengthStreamingMode((int) buf.length()); + try (OutputStream httpOut = conn.getOutputStream()) { + buf.writeTo(httpOut, null); + } - final int status = HttpSupport.response(conn); - if (status == HttpConnection.HTTP_MOVED_PERM) { - String locationHeader = HttpSupport.responseHeader(conn, HDR_LOCATION); - sendRequest(locationHeader); + if (http.followRedirects == HttpRedirectMode.TRUE) { + final int status = HttpSupport.response(conn); + switch (status) { + case HttpConnection.HTTP_MOVED_PERM: + case HttpConnection.HTTP_MOVED_TEMP: + case HttpConnection.HTTP_11_MOVED_TEMP: + // SEE_OTHER after a POST doesn't make sense for a git + // server, so we don't handle it here and thus we'll + // report an error in openResponse() later on. + URIish newUri = redirect( + conn.getHeaderField(HDR_LOCATION), + '/' + serviceName, redirects++); + try { + baseUrl = toURL(newUri); + } catch (MalformedURLException e) { + throw new TransportException(MessageFormat.format( + JGitText.get().invalidRedirectLocation, + uri, baseUrl, newUri), e); + } + continue; + default: + break; + } + } + break; } } 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 58081c1c9..35a1ee15e 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 @@ -1,5 +1,5 @@ /* - * Copyright (C) 2013 Christian Halstrick + * Copyright (C) 2013, 2017 Christian Halstrick * and other copyright owners as documented in the project's IP log. * * This program and the accompanying materials are made available @@ -78,6 +78,26 @@ public interface HttpConnection { */ public static final int HTTP_MOVED_PERM = java.net.HttpURLConnection.HTTP_MOVED_PERM; + /** + * @see HttpURLConnection#HTTP_MOVED_TEMP + * @since 4.9 + */ + public static final int HTTP_MOVED_TEMP = java.net.HttpURLConnection.HTTP_MOVED_TEMP; + + /** + * @see HttpURLConnection#HTTP_SEE_OTHER + * @since 4.9 + */ + public static final int HTTP_SEE_OTHER = java.net.HttpURLConnection.HTTP_SEE_OTHER; + + /** + * HTTP 1.1 additional MOVED_TEMP status code; value = 307. + * + * @see #HTTP_MOVED_TEMP + * @since 4.9 + */ + public static final int HTTP_11_MOVED_TEMP = 307; + /** * @see HttpURLConnection#HTTP_NOT_FOUND */ @@ -253,7 +273,7 @@ public interface HttpConnection { /** * Configure the connection so that it can be used for https communication. - * + * * @param km * the keymanager managing the key material used to authenticate * the local SSLSocket to its peer