Extract path info from requests without decoding
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. We do this with a static method rather
than using our own HttpServletRequestWrapper as we don't want to
produce a request wrapper that is not in spec.
[1] https://github.com/google/guice/issues/745
[2] http://docs.oracle.com/javaee/7/api/javax/servlet/http/HttpServletRequest.html#getPathInfo()
Change-Id: I48fb32710d45f573fb167c6344ad76541672858c
diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/RequestUtil.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/RequestUtil.java
new file mode 100644
index 0000000..c47ca30
--- /dev/null
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/RequestUtil.java
@@ -0,0 +1,48 @@
+// Copyright (C) 2014 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.httpd;
+
+import javax.servlet.http.HttpServletRequest;
+
+/** Utilities for manipulating HTTP request objects. */
+public class RequestUtil {
+ /**
+ * @return the same value as {@link HttpServletRequest#getPathInfo()}, but
+ * without decoding URL-encoded characters.
+ */
+ public static String getEncodedPathInfo(HttpServletRequest req) {
+ // Based on com.google.guice.ServletDefinition$1#getPathInfo() from:
+ // https://github.com/google/guice/blob/41c126f99d6309886a0ded2ac729033d755e1593/extensions/servlet/src/com/google/inject/servlet/ServletDefinition.java
+ String servletPath = req.getServletPath();
+ int servletPathLength = servletPath.length();
+ String requestUri = req.getRequestURI();
+ String pathInfo = requestUri.substring(req.getContextPath().length())
+ .replaceAll("[/]{2,}", "/");
+ if (pathInfo.startsWith(servletPath)) {
+ pathInfo = pathInfo.substring(servletPathLength);
+ // Corner case: when servlet path & request path match exactly (without
+ // trailing '/'), then pathinfo is null.
+ if (pathInfo.isEmpty() && servletPathLength > 0) {
+ pathInfo = null;
+ }
+ } else {
+ pathInfo = null;
+ }
+ return pathInfo;
+ }
+
+ private RequestUtil() {
+ }
+}
diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/plugins/HttpPluginServlet.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/plugins/HttpPluginServlet.java
index 90ce894..286f61b 100644
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/plugins/HttpPluginServlet.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/plugins/HttpPluginServlet.java
@@ -26,6 +26,7 @@
import com.google.common.collect.Maps;
import com.google.common.net.HttpHeaders;
import com.google.gerrit.extensions.registration.RegistrationHandle;
+import com.google.gerrit.httpd.RequestUtil;
import com.google.gerrit.httpd.resources.Resource;
import com.google.gerrit.httpd.resources.ResourceKey;
import com.google.gerrit.httpd.resources.SmallResource;
@@ -208,7 +209,7 @@
throws IOException, ServletException {
List<String> parts = Lists.newArrayList(
Splitter.on('/').limit(3).omitEmptyStrings()
- .split(Strings.nullToEmpty(req.getPathInfo())));
+ .split(Strings.nullToEmpty(RequestUtil.getEncodedPathInfo(req))));
if (isApiCall(req, parts)) {
managerApi.service(req, res);
@@ -255,7 +256,7 @@
return;
}
- String pathInfo = req.getPathInfo();
+ String pathInfo = RequestUtil.getEncodedPathInfo(req);
if (pathInfo.length() < 1) {
Resource.NOT_FOUND.send(req, res);
return;
diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
index 0d62392..979990a 100644
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
@@ -74,6 +74,7 @@
import com.google.gerrit.extensions.restapi.RestView;
import com.google.gerrit.extensions.restapi.TopLevelResource;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
+import com.google.gerrit.httpd.RequestUtil;
import com.google.gerrit.httpd.WebSession;
import com.google.gerrit.server.AccessPath;
import com.google.gerrit.server.AnonymousUser;
@@ -871,7 +872,7 @@
}
private static List<IdString> splitPath(HttpServletRequest req) {
- String path = req.getPathInfo();
+ String path = RequestUtil.getEncodedPathInfo(req);
if (Strings.isNullOrEmpty(path)) {
return Collections.emptyList();
}
diff --git a/gerrit-httpd/src/test/java/com/google/gerrit/httpd/RequestUtilTest.java b/gerrit-httpd/src/test/java/com/google/gerrit/httpd/RequestUtilTest.java
new file mode 100644
index 0000000..71a43bf
--- /dev/null
+++ b/gerrit-httpd/src/test/java/com/google/gerrit/httpd/RequestUtilTest.java
@@ -0,0 +1,91 @@
+// Copyright (C) 2014 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.httpd;
+
+import static org.easymock.EasyMock.createMock;
+import static org.easymock.EasyMock.expect;
+import static org.easymock.EasyMock.replay;
+import static org.easymock.EasyMock.verify;
+import static org.junit.Assert.assertEquals;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+
+import javax.servlet.http.HttpServletRequest;
+
+public class RequestUtilTest {
+ private List<Object> mocks;
+
+ @Before
+ public void setUp() {
+ mocks = Collections.synchronizedList(new ArrayList<>());
+ }
+
+ @After
+ public void tearDown() {
+ for (Object mock : mocks) {
+ verify(mock);
+ }
+ }
+
+ @Test
+ public void emptyContextPath() {
+ assertEquals("/foo/bar", RequestUtil.getEncodedPathInfo(
+ mockRequest("/s/foo/bar", "", "/s")));
+ assertEquals("/foo%2Fbar", RequestUtil.getEncodedPathInfo(
+ mockRequest("/s/foo%2Fbar", "", "/s")));
+ }
+
+ @Test
+ public void emptyServletPath() {
+ assertEquals("/foo/bar", RequestUtil.getEncodedPathInfo(
+ mockRequest("/c/foo/bar", "/c", "")));
+ assertEquals("/foo%2Fbar", RequestUtil.getEncodedPathInfo(
+ mockRequest("/c/foo%2Fbar", "/c", "")));
+ }
+
+ @Test
+ public void trailingSlashes() {
+ assertEquals("/foo/bar/", RequestUtil.getEncodedPathInfo(
+ mockRequest("/c/s/foo/bar/", "/c", "/s")));
+ assertEquals("/foo/bar/", RequestUtil.getEncodedPathInfo(
+ mockRequest("/c/s/foo/bar///", "/c", "/s")));
+ assertEquals("/foo%2Fbar/", RequestUtil.getEncodedPathInfo(
+ mockRequest("/c/s/foo%2Fbar/", "/c", "/s")));
+ assertEquals("/foo%2Fbar/", RequestUtil.getEncodedPathInfo(
+ mockRequest("/c/s/foo%2Fbar///", "/c", "/s")));
+ }
+
+ @Test
+ public void servletPathMatchesRequestPath() {
+ assertEquals(null, RequestUtil.getEncodedPathInfo(
+ mockRequest("/c/s", "/c", "/s")));
+ }
+
+ private HttpServletRequest mockRequest(String uri, String contextPath, String servletPath) {
+ HttpServletRequest req = createMock(HttpServletRequest.class);
+ expect(req.getRequestURI()).andStubReturn(uri);
+ expect(req.getContextPath()).andStubReturn(contextPath);
+ expect(req.getServletPath()).andStubReturn(servletPath);
+ replay(req);
+ mocks.add(req);
+ return req;
+ }
+}
