Extract path info from requests without decoding

Gitiles malfunctions in conjunction with jgit and guice
because of a recent Guice bug fix. Work around the problem
by parsing the URI directly, bypassing the unescaping
performed by the getPathInfo method.

This rest of this message is copied from
https://gerrit-review.googlesource.com/#/c/60820/ :

The fix for Guice issue #745[1] causes getPathInfo() within the
GuiceFilter to return decoded values, eliminating the difference
between "foo/bar" and "foo%2Fbar". This is in spec with the servlet
standard, whose javadoc for getPathInfo[2] states that the return
value be "decoded by the web container".

Work around this by extracting the path part directly from the request
URI, which is unmodified by the container. This is copying the Guice
behavior prior to the bugfix.

[1] https://github.com/google/guice/issues/745
[2] http://docs.oracle.com/javaee/7/api/javax/servlet/http/HttpServletRequest.html#getPathInfo()

Change-Id: I7fdb291bda377dab6160599ee537962d5f60f1e8
Signed-off-by: David Pletcher <dpletcher@google.com>

5 files changed, 96 insertions, 7 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 8d56d84c97..035b0578c8 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,6 +225,48 @@ 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 2ef71368d0..d81f7a0c92 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,6 +56,8 @@ 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.
  * <p>
@@ -109,14 +111,14 @@ class RegexPipeline extends UrlPipeline {
 	}
 
 	boolean match(final HttpServletRequest req) {
-		final String pathInfo = req.getPathInfo();
+		final String pathInfo = ServletUtils.getEncodedPathInfo(req);
 		return pathInfo != null && pattern.matcher(pathInfo).matches();
 	}
 
 	@Override
 	void service(HttpServletRequest req, HttpServletResponse rsp)
 			throws ServletException, IOException {
-		final String reqInfo = req.getPathInfo();
+		final String reqInfo = ServletUtils.getEncodedPathInfo(req);
 		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 b942016259..e9b0d6529d 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,6 +51,8 @@ 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.
  * <p>
@@ -88,14 +90,14 @@ class SuffixPipeline extends UrlPipeline {
 	}
 
 	boolean match(final HttpServletRequest req) {
-		final String pathInfo = req.getPathInfo();
+		final String pathInfo = ServletUtils.getEncodedPathInfo(req);
 		return pathInfo != null && pathInfo.endsWith(suffix);
 	}
 
 	@Override
 	void service(HttpServletRequest req, HttpServletResponse rsp)
 			throws ServletException, IOException {
-		String curInfo = req.getPathInfo();
+		String curInfo = ServletUtils.getEncodedPathInfo(req);
 		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 07bdfc751d..c62b5d7aec 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,8 +48,51 @@ 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));
 		assertFalse(ServletUtils.acceptsGzipEncoding(""));
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 64fbc01d4d..c73fd58a30 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());
 	}