From f6cff781eedd48c626bd703afee83b0f40841ee3 Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Fri, 12 Dec 2014 16:53:18 -0500 Subject: [PATCH] Revert "Extract path info from requests without decoding" This reverts commit 19f869996f27adf59ec507e5f565d8b5619576f3. Leaving path info encoded confuses applications like Gitiles. Trying to fix this inside of JGit was maybe the wrong solution. Change-Id: I8df9ab6233ff513e427701c8a1a66022c19784eb --- .../jgit/http/server/ServletUtils.java | 42 ------------------ .../jgit/http/server/glue/RegexPipeline.java | 6 +-- .../jgit/http/server/glue/SuffixPipeline.java | 6 +-- .../jgit/http/server/ServletUtilsTest.java | 43 ------------------- .../jgit/http/test/RegexPipelineTest.java | 6 +-- 5 files changed, 7 insertions(+), 96 deletions(-) diff --git a/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/ServletUtils.java b/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/ServletUtils.java index 035b0578c..8d56d84c9 100644 --- a/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/ServletUtils.java +++ b/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/ServletUtils.java @@ -225,48 +225,6 @@ public final class ServletUtils { } } - /** - * Get the path info component of the request. The result is similar to - * {@link HttpServletRequest#getPathInfo()}, but URL-encoded characters are - * not decoded. - * - * @param req - * the incoming request. - * @return the same value as {@link HttpServletRequest#getPathInfo()}, but - * without decoding URL-encoded characters. - * @since 3.6 - */ - public static String getEncodedPathInfo(HttpServletRequest req) { - return getEncodedPathInfo(req.getContextPath(), req.getServletPath(), - req.getRequestURI()); - } - - /** - * Get the path info component of the request. The result is similar to - * {@link HttpServletRequest#getPathInfo()}, but URL-encoded characters are - * not decoded. - * - * @param contextPath - * the context path from the incoming request. - * @param servletPath - * the servlet path from the incoming request. - * @param requestUri - * the request URI from the incoming request. - * @return the same value as {@link HttpServletRequest#getPathInfo()}, but - * without decoding URL-encoded characters. - */ - static String getEncodedPathInfo(String contextPath, String servletPath, - String requestUri) { - String pathInfo = requestUri.substring(contextPath.length()) - .replaceAll("/{2,}", "/"); - if (!pathInfo.startsWith(servletPath)) - return null; - pathInfo = pathInfo.substring(servletPath.length()); - if (pathInfo.isEmpty() && !servletPath.isEmpty()) - return null; - return pathInfo; - } - private static byte[] sendInit(byte[] content, final HttpServletRequest req, final HttpServletResponse rsp) throws IOException { diff --git a/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/glue/RegexPipeline.java b/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/glue/RegexPipeline.java index d81f7a0c9..2ef71368d 100644 --- a/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/glue/RegexPipeline.java +++ b/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/glue/RegexPipeline.java @@ -56,8 +56,6 @@ import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import org.eclipse.jgit.http.server.ServletUtils; - /** * Selects requests by matching the URI against a regular expression. *

@@ -111,14 +109,14 @@ class RegexPipeline extends UrlPipeline { } boolean match(final HttpServletRequest req) { - final String pathInfo = ServletUtils.getEncodedPathInfo(req); + final String pathInfo = req.getPathInfo(); return pathInfo != null && pattern.matcher(pathInfo).matches(); } @Override void service(HttpServletRequest req, HttpServletResponse rsp) throws ServletException, IOException { - final String reqInfo = ServletUtils.getEncodedPathInfo(req); + final String reqInfo = req.getPathInfo(); if (reqInfo == null) { rsp.sendError(SC_NOT_FOUND); return; diff --git a/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/glue/SuffixPipeline.java b/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/glue/SuffixPipeline.java index e9b0d6529..b94201625 100644 --- a/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/glue/SuffixPipeline.java +++ b/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/glue/SuffixPipeline.java @@ -51,8 +51,6 @@ import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import org.eclipse.jgit.http.server.ServletUtils; - /** * Selects requests by matching the suffix of the URI. *

@@ -90,14 +88,14 @@ class SuffixPipeline extends UrlPipeline { } boolean match(final HttpServletRequest req) { - final String pathInfo = ServletUtils.getEncodedPathInfo(req); + final String pathInfo = req.getPathInfo(); return pathInfo != null && pathInfo.endsWith(suffix); } @Override void service(HttpServletRequest req, HttpServletResponse rsp) throws ServletException, IOException { - String curInfo = ServletUtils.getEncodedPathInfo(req); + String curInfo = req.getPathInfo(); String newPath = req.getServletPath() + curInfo; String newInfo = curInfo.substring(0, curInfo.length() - suffixLen); super.service(new WrappedRequest(req, newPath, newInfo), rsp); diff --git a/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/server/ServletUtilsTest.java b/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/server/ServletUtilsTest.java index c62b5d7ae..07bdfc751 100644 --- a/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/server/ServletUtilsTest.java +++ b/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/server/ServletUtilsTest.java @@ -48,50 +48,7 @@ import static org.junit.Assert.assertTrue; import org.junit.Test; -import static org.junit.Assert.assertEquals; - public class ServletUtilsTest { - @Test - public void emptyContextPath() { - assertEquals("/foo/bar", - ServletUtils.getEncodedPathInfo("", "/s", "/s/foo/bar")); - assertEquals("/foo%2Fbar", - ServletUtils.getEncodedPathInfo("", "/s", "/s/foo%2Fbar")); - } - - public void slashContextPath() { - assertEquals("/foo/bar", - ServletUtils.getEncodedPathInfo("/", "/s", "/s/foo/bar")); - assertEquals("/foo%2Fbar", - ServletUtils.getEncodedPathInfo("/", "/s", "/s/foo%2Fbar")); - } - - @Test - public void emptyServletPath() { - assertEquals("/foo/bar", - ServletUtils.getEncodedPathInfo("/c", "", "/c/foo/bar")); - assertEquals("/foo%2Fbar", - ServletUtils.getEncodedPathInfo("/c", "", "/c/foo%2Fbar")); - } - - @Test - public void trailingSlashes() { - assertEquals("/foo/bar/", - ServletUtils.getEncodedPathInfo("/c", "/s", "/c/s/foo/bar/")); - assertEquals("/foo/bar/", - ServletUtils.getEncodedPathInfo("/c", "/s", "/c/s/foo/bar///")); - assertEquals("/foo%2Fbar/", - ServletUtils.getEncodedPathInfo("/c", "/s", "/c/s/foo%2Fbar/")); - assertEquals("/foo%2Fbar/", ServletUtils.getEncodedPathInfo("/c", "/s", - "/c/s/foo%2Fbar///")); - } - - @Test - public void servletPathMatchesRequestPath() { - assertEquals((String) null, - ServletUtils.getEncodedPathInfo("/c", "/s", "/c/s")); - } - @Test public void testAcceptGzip() { assertFalse(ServletUtils.acceptsGzipEncoding((String) null)); diff --git a/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/RegexPipelineTest.java b/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/RegexPipelineTest.java index c73fd58a3..64fbc01d4 100644 --- a/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/RegexPipelineTest.java +++ b/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/RegexPipelineTest.java @@ -168,7 +168,7 @@ public class RegexPipelineTest extends HttpTestCase { s.serveRegex("^(/c)(/d)$") .through(new RegexGroupFilter(1)) .with(new Servlet("test2")); - s.serveRegex("^(/e)/f.*(/g)$") + s.serveRegex("^(/e)/f(/g)$") .through(new RegexGroupFilter(2)) .with(new Servlet("test3")); ctx.addServlet(new ServletHolder(s), "/*"); @@ -197,12 +197,12 @@ public class RegexPipelineTest extends HttpTestCase { assertEquals("/c", r.readLine()); assertEquals(null, r.readLine()); - c = ((HttpURLConnection) uri.resolve("/e/f/+/g").toURL() + c = ((HttpURLConnection) uri.resolve("/e/f/g").toURL() .openConnection()); assertEquals(200, c.getResponseCode()); r = new BufferedReader(new InputStreamReader(c.getInputStream())); assertEquals("test3", r.readLine()); - assertEquals("/e/f/+", r.readLine()); + assertEquals("/e/f", r.readLine()); assertEquals("/g", r.readLine()); assertEquals(null, r.readLine()); }