Fix the request wrapper for http requests served from plugins

A request wrapper is used to provide servlets contributed by plugins
with a context that is rooted in the plugins path segment of the request
URI. For example a plugin myPlugin that is invoked under
'/a/plugins/myPlugin' should be provided with the context path
'/a/plugins/myPlugin'. The request '/a/plugins/myPlugin/myResource'
should give the following info for the wrapped context:

contextPath: '/a/plugins/myPlugin'
servletPath: ''
pathInfo:    '/myResource'
requestURI:  '/a/plugins/myPlugin/myResource'

The request wrapper failed to comply with the servlet specification
(servlet specification 3.1, §3.5). For example the invariant

requestURI = contextPath + servletPath + pathInfo

was violated.

The request mapping mechanism is now extracted to a ContextMapper class.

Change-Id: Ie8d9a9ac151162efb080a6e676a121a53bc11852

gerrit-httpd/src/main/java/com/google/gerrit/httpd/plugins/ContextMapper.java

@@ -0,0 +1,74 @@
+// 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.plugins;
+
+package com.google.gerrit.httpd.plugins;
+
+import com.google.common.base.Strings;
+
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletRequestWrapper;
+
+class ContextMapper {
+  private static final String PLUGINS_PREFIX = "/plugins/";
+  private static final String AUTHORIZED_PREFIX = "/a" + PLUGINS_PREFIX;
+  private final String base;
+  private final String authorizedBase;
+
+  public ContextMapper(String contextPath) {
+    base = Strings.nullToEmpty(contextPath) + PLUGINS_PREFIX;
+    authorizedBase = Strings.nullToEmpty(contextPath) + AUTHORIZED_PREFIX;
+  }
+
+  private static boolean isAuthorizedCall(HttpServletRequest req) {
+    return !Strings.isNullOrEmpty(req.getServletPath())
+        && req.getServletPath().startsWith(AUTHORIZED_PREFIX);
+  }
+
+  HttpServletRequest create(HttpServletRequest req, String name) {
+    String contextPath = (isAuthorizedCall(req) ? authorizedBase : base) + name;
+
+    return new WrappedRequest(req, contextPath);
+  }
+
+  public String getFullPath(String name) {
+    return base + name;
+  }
+
+  private class WrappedRequest extends HttpServletRequestWrapper {
+    private final String contextPath;
+    private final String pathInfo;
+
+    private WrappedRequest(HttpServletRequest req, String contextPath) {
+      super(req);
+      this.contextPath = contextPath;
+      this.pathInfo = getRequestURI().substring(contextPath.length());
+    }
+
+    @Override
+    public String getServletPath() {
+      return "";
+    }
+
+    @Override
+    public String getContextPath() {
+      return contextPath;
+    }
+
+    @Override
+    public String getPathInfo() {
+      return pathInfo;
+    }
+  }
+
+}

gerrit-httpd/src/main/java/com/google/gerrit/httpd/plugins/HttpPluginServlet.java

@@ -73,7 +73,6 @@ import javax.servlet.ServletRequest;
 import javax.servlet.ServletResponse;
 import javax.servlet.http.HttpServlet;
 import javax.servlet.http.HttpServletRequest;
-import javax.servlet.http.HttpServletRequestWrapper;
 import javax.servlet.http.HttpServletResponse;
 
 @Singleton
@@ -83,8 +82,6 @@ class HttpPluginServlet extends HttpServlet
   private static final long serialVersionUID = 1L;
   private static final Logger log
       = LoggerFactory.getLogger(HttpPluginServlet.class);
-  private static final String PLUGINS_PREFIX = "/plugins/";
-  private static final String AUTHORIZED_PREFIX = "/a" + PLUGINS_PREFIX;
 
   private final MimeUtilFileTypeRegistry mimeUtil;
   private final Provider<String> webUrl;
@@ -94,8 +91,7 @@ class HttpPluginServlet extends HttpServlet
   private final RestApiServlet managerApi;
 
   private List<Plugin> pending = Lists.newArrayList();
-  private String base;
-  private String authorizedBase;
+  private ContextMapper wrapper;
   private final ConcurrentMap<String, PluginHolder> plugins
       = Maps.newConcurrentMap();
 
@@ -134,9 +130,7 @@ class HttpPluginServlet extends HttpServlet
   public synchronized void init(ServletConfig config) throws ServletException {
     super.init(config);
 
-    String path = config.getServletContext().getContextPath();
-    base = Strings.nullToEmpty(path) + PLUGINS_PREFIX;
-    authorizedBase = Strings.nullToEmpty(path) + AUTHORIZED_PREFIX;
+    wrapper = new ContextMapper(config.getServletContext().getContextPath());
     for (Plugin plugin : pending) {
       install(plugin);
     }
@@ -182,7 +176,8 @@ class HttpPluginServlet extends HttpServlet
       }
 
       try {
-        ServletContext ctx = PluginServletContext.create(plugin, base + name);
+        ServletContext ctx =
+            PluginServletContext.create(plugin, wrapper.getFullPath(name));
         filter.init(new WrappedFilterConfig(ctx));
       } catch (ServletException e) {
         log.warn(String.format("Plugin %s failed to initialize HTTP", name), e);
@@ -220,8 +215,7 @@ class HttpPluginServlet extends HttpServlet
       return;
     }
 
-    WrappedRequest wr = new WrappedRequest(req,
-        (isAuthorizedCall(req) ? authorizedBase : base) + name);
+    HttpServletRequest wr = wrapper.create(req, name);
     FilterChain chain = new FilterChain() {
       @Override
       public void doFilter(ServletRequest req, ServletResponse res)
@@ -236,11 +230,6 @@ class HttpPluginServlet extends HttpServlet
     }
   }
 
-  private boolean isAuthorizedCall(HttpServletRequest req) {
-    return !Strings.isNullOrEmpty(req.getServletPath())
-        && req.getServletPath().startsWith(AUTHORIZED_PREFIX);
-  }
-
   private static boolean isApiCall(HttpServletRequest req, List<String> parts) {
     String method = req.getMethod();
     int cnt = parts.size();
@@ -258,14 +247,13 @@ class HttpPluginServlet extends HttpServlet
       return;
     }
 
-    String uri = req.getRequestURI();
-    String ctx = req.getContextPath();
-    if (uri.length() <= ctx.length()) {
+    String pathInfo = req.getPathInfo();
+    if (pathInfo.length() < 1) {
       Resource.NOT_FOUND.send(req, res);
       return;
     }
 
-    String file = uri.substring(ctx.length() + 1);
+    String file = pathInfo.substring(1);
     ResourceKey key = new ResourceKey(holder.plugin, file);
     Resource rsc = resourceCache.getIfPresent(key);
     if (rsc != null) {
@@ -273,6 +261,7 @@ class HttpPluginServlet extends HttpServlet
       return;
     }
 
+    String uri = req.getRequestURI();
     if ("".equals(file)) {
       res.sendRedirect(uri + holder.docPrefix + "index.html");
       return;
@@ -675,32 +664,4 @@ class HttpPluginServlet extends HttpServlet
       }
     }
   }
-
-  private static class WrappedRequest extends HttpServletRequestWrapper {
-    private final String contextPath;
-
-    WrappedRequest(HttpServletRequest req, String contextPath) {
-      super(req);
-      this.contextPath = contextPath;
-    }
-
-    @Override
-    public String getContextPath() {
-      return contextPath;
-    }
-
-    @Override
-    public String getServletPath() {
-      return getRequestURI().substring(contextPath.length());
-    }
-
-    @Override
-    public String getRequestURI() {
-      String uri = super.getRequestURI();
-      if (uri.startsWith("/a/")) {
-        uri = uri.substring(2);
-      }
-      return uri;
-    }
-  }
 }

gerrit-httpd/src/test/java/com/google/gerrit/httpd/plugins/ContextMapperTest.java

@@ -0,0 +1,79 @@
+// 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.plugins;
+
+package com.google.gerrit.httpd.plugins;
+
+import static org.easymock.EasyMock.createNiceMock;
+import static org.easymock.EasyMock.expect;
+import static org.easymock.EasyMock.replay;
+import static org.junit.Assert.assertEquals;
+
+import org.junit.Test;
+
+import javax.servlet.http.HttpServletRequest;
+
+public class ContextMapperTest {
+
+  private static final String CONTEXT = "/context";
+  private static final String PLUGIN_NAME = "my-plugin";
+  private static final String RESOURCE = "my-resource";
+
+  @Test
+  public void testUnauthorized() throws Exception {
+    ContextMapper classUnderTest = new ContextMapper(CONTEXT);
+
+    HttpServletRequest originalRequest =
+        createMockRequest("/plugins/", PLUGIN_NAME + "/" + RESOURCE);
+
+    HttpServletRequest result =
+        classUnderTest.create(originalRequest, PLUGIN_NAME);
+
+    assertEquals(CONTEXT + "/plugins/" + PLUGIN_NAME, result.getContextPath());
+    assertEquals("", result.getServletPath());
+    assertEquals("/" + RESOURCE, result.getPathInfo());
+    assertEquals(CONTEXT + "/plugins/" + PLUGIN_NAME + "/" + RESOURCE,
+        result.getRequestURI());
+  }
+
+  @Test
+  public void testAuthorized() throws Exception {
+    ContextMapper classUnderTest = new ContextMapper(CONTEXT);
+
+    HttpServletRequest originalRequest =
+        createMockRequest("/a/plugins/", PLUGIN_NAME + "/" + RESOURCE);
+
+    HttpServletRequest result =
+        classUnderTest.create(originalRequest, PLUGIN_NAME);
+
+    assertEquals(CONTEXT + "/a/plugins/" + PLUGIN_NAME,
+        result.getContextPath());
+    assertEquals("", result.getServletPath());
+    assertEquals("/" + RESOURCE, result.getPathInfo());
+    assertEquals(CONTEXT + "/a/plugins/" + PLUGIN_NAME + "/" + RESOURCE,
+        result.getRequestURI());
+  }
+
+  private static HttpServletRequest createMockRequest(String servletPath,
+      String pathInfo) {
+    HttpServletRequest req = createNiceMock(HttpServletRequest.class);
+    expect(req.getContextPath()).andStubReturn(CONTEXT);
+    expect(req.getServletPath()).andStubReturn(servletPath);
+    expect(req.getPathInfo()).andStubReturn(pathInfo);
+    String uri = CONTEXT + servletPath + pathInfo;
+    expect(req.getRequestURI()).andStubReturn(uri);
+    replay(req);
+
+    return req;
+  }
+}