Fixed exception in contextpath string manipulation.

Revision created by MOE tool push_codebase.
MOE_MIGRATION=4880
diff --git a/extensions/servlet/src/com/google/inject/servlet/FilterDefinition.java b/extensions/servlet/src/com/google/inject/servlet/FilterDefinition.java
index 7ea0d67..ff1e5b6 100755
--- a/extensions/servlet/src/com/google/inject/servlet/FilterDefinition.java
+++ b/extensions/servlet/src/com/google/inject/servlet/FilterDefinition.java
@@ -60,11 +60,11 @@
     this.initParams = Collections.unmodifiableMap(new HashMap<String, String>(initParams));
     this.filterInstance = filterInstance;
   }
-  
+
   public FilterDefinition get() {
     return this;
   }
-  
+
   public <B, V> V acceptExtensionVisitor(BindingTargetVisitor<B, V> visitor,
       ProviderInstanceBinding<? extends B> binding) {
     if(visitor instanceof ServletModuleTargetVisitor) {
@@ -73,7 +73,7 @@
             new InstanceFilterBindingImpl(initParams,
                 pattern,
                 filterInstance,
-                patternMatcher));        
+                patternMatcher));
       } else {
         return ((ServletModuleTargetVisitor<B, V>)visitor).visit(
             new LinkedFilterBindingImpl(initParams,
@@ -87,7 +87,7 @@
   }
 
   private boolean shouldFilter(String uri) {
-    return patternMatcher.matches(uri);
+    return uri != null && patternMatcher.matches(uri);
   }
 
   public void init(final ServletContext servletContext, Injector injector,
@@ -133,7 +133,7 @@
   public void destroy(Set<Filter> destroyedSoFar) {
     // filters are always singletons
     Filter reference = filter.get();
-    
+
     // Do nothing if this Filter was invalid (usually due to not being scoped
     // properly), or was already destroyed. According to Servlet Spec: it is
     // "out of service", and does not need to be destroyed.
@@ -152,8 +152,7 @@
 
   public Filter getFilterIfMatching(HttpServletRequest request) {
 
-    final String path = request.getRequestURI().substring(request.getContextPath().length());
-
+    final String path = ServletUtils.getContextRelativePath(request);
     if (shouldFilter(path)) {
       return filter.get();
     } else {
diff --git a/extensions/servlet/src/com/google/inject/servlet/ServletDefinition.java b/extensions/servlet/src/com/google/inject/servlet/ServletDefinition.java
index 00ac328..1111fde 100755
--- a/extensions/servlet/src/com/google/inject/servlet/ServletDefinition.java
+++ b/extensions/servlet/src/com/google/inject/servlet/ServletDefinition.java
@@ -68,11 +68,11 @@
     this.initParams = Collections.unmodifiableMap(new HashMap<String, String>(initParams));
     this.servletInstance = servletInstance;
   }
-  
+
   public ServletDefinition get() {
     return this;
   }
-  
+
   public <B, V> V acceptExtensionVisitor(BindingTargetVisitor<B, V> visitor,
       ProviderInstanceBinding<? extends B> binding) {
     if(visitor instanceof ServletModuleTargetVisitor) {
@@ -81,7 +81,7 @@
             new InstanceServletBindingImpl(initParams,
                 pattern,
                 servletInstance,
-                patternMatcher));        
+                patternMatcher));
       } else {
         return ((ServletModuleTargetVisitor<B, V>)visitor).visit(
             new LinkedServletBindingImpl(initParams,
@@ -95,7 +95,7 @@
   }
 
   boolean shouldServe(String uri) {
-    return patternMatcher.matches(uri);
+    return uri != null && patternMatcher.matches(uri);
   }
 
   public void init(final ServletContext servletContext, Injector injector,
@@ -163,7 +163,7 @@
    *
    * @return Returns true if this servlet triggered for the given request. Or false if
    *          guice-servlet should continue dispatching down the servlet pipeline.
-   * 
+   *
    * @throws IOException If thrown by underlying servlet
    * @throws ServletException If thrown by underlying servlet
    */
@@ -171,7 +171,7 @@
       ServletResponse servletResponse) throws IOException, ServletException {
 
     final HttpServletRequest request = (HttpServletRequest) servletRequest;
-    final String path = request.getRequestURI().substring(request.getContextPath().length());
+    final String path = ServletUtils.getContextRelativePath(request);
 
     final boolean serve = shouldServe(path);
 
diff --git a/extensions/servlet/src/com/google/inject/servlet/ServletUtils.java b/extensions/servlet/src/com/google/inject/servlet/ServletUtils.java
new file mode 100644
index 0000000..6a29425
--- /dev/null
+++ b/extensions/servlet/src/com/google/inject/servlet/ServletUtils.java
@@ -0,0 +1,38 @@
+// Copyright 2012 Google Inc. All Rights Reserved.
+
+package com.google.inject.servlet;
+
+import javax.servlet.http.HttpServletRequest;
+
+/**
+ * Some servlet utility methods.
+ *
+ * @author ntang@google.com (Michael Tang)
+ */
+final class ServletUtils {
+  private ServletUtils() {
+    // private to prevent instantiation.
+  }
+
+  /**
+   * Gets the context path relative path of the URI. Returns the path of the
+   * resource relative to the context path for a request's URI, or null if no
+   * path can be extracted.
+   */
+  // @Nullable
+  public static String getContextRelativePath(
+      // @Nullable
+      final HttpServletRequest request) {
+    if (request != null) {
+      String contextPath = request.getContextPath();
+      String requestURI = request.getRequestURI();
+      if (contextPath.length() < requestURI.length()) {
+        return requestURI.substring(contextPath.length());
+      } else if (requestURI != null && requestURI.trim().length() > 0 &&
+          contextPath.length() == requestURI.length()) {
+        return "/";
+      }
+    }
+    return null;
+  }
+}
diff --git a/extensions/servlet/test/com/google/inject/servlet/FilterDefinitionTest.java b/extensions/servlet/test/com/google/inject/servlet/FilterDefinitionTest.java
index f707195..c151102 100644
--- a/extensions/servlet/test/com/google/inject/servlet/FilterDefinitionTest.java
+++ b/extensions/servlet/test/com/google/inject/servlet/FilterDefinitionTest.java
@@ -6,6 +6,7 @@
 import static org.easymock.EasyMock.replay;
 import static org.easymock.EasyMock.verify;
 
+import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Maps;
 import com.google.common.collect.Sets;
 import com.google.inject.Binding;
@@ -28,6 +29,7 @@
 import javax.servlet.ServletRequest;
 import javax.servlet.ServletResponse;
 import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
 
 /**
  * Tests the lifecycle of the encapsulated {@link FilterDefinition} class.
@@ -56,14 +58,9 @@
 
     //some init params
     //noinspection SSBasedInspection
-    final Map<String, String> initParams = new HashMap<String, String>() {{
-      put("ahsd", "asdas24dok");
-      put("ahssd", "asdasd124ok");
-      put("ahfsasd", "asda124sdok");
-      put("ahsasgd", "a124sdasdok");
-      put("ahsd124124", "as124124124dasdok");
-    }};
-
+    final Map<String, String> initParams = new ImmutableMap.Builder<String, String>()
+      .put("ahsd", "asdas24dok")
+      .put("ahssd", "asdasd124ok").build();
 
     ServletContext servletContext = createMock(ServletContext.class);
     final String contextName = "thing__!@@44";
@@ -90,7 +87,7 @@
       assertTrue(initParams.containsKey(name));
       assertTrue(initParams.get(name).equals(filterConfig.getInitParameter(name)));
     }
-    
+
     verify(binding, injector, servletContext);
   }
 
@@ -167,7 +164,7 @@
         .andReturn(true);
     expect(injector.getBinding(Key.get(Filter.class)))
         .andReturn(binding);
-    
+
     expect(injector.getInstance(Key.get(Filter.class)))
         .andReturn(mockFilter)
         .anyTimes();
@@ -211,6 +208,78 @@
 
   }
 
+  public void testGetFilterIfMatching() throws ServletException {
+    String pattern = "/*";
+    final FilterDefinition filterDef = new FilterDefinition(pattern, Key.get(Filter.class),
+        UriPatternType.get(UriPatternType.SERVLET, pattern),
+        new HashMap<String, String>(), null);
+    HttpServletRequest servletRequest = createMock(HttpServletRequest.class);
+    ServletContext servletContext = createMock(ServletContext.class);
+    Injector injector = createMock(Injector.class);
+    Binding binding = createMock(Binding.class);
+
+    final MockFilter mockFilter = new MockFilter() {
+      @Override
+      public void doFilter(ServletRequest servletRequest, ServletResponse servletResponse,
+          FilterChain filterChain) {
+        //suppress rest of chain...
+      }
+    };
+    expect(injector.getBinding(Key.get(Filter.class)))
+        .andReturn(binding);
+    expect(binding.acceptScopingVisitor((BindingScopingVisitor) anyObject()))
+        .andReturn(true);
+    expect(injector.getInstance(Key.get(Filter.class)))
+        .andReturn(mockFilter)
+        .anyTimes();
+
+    expect(servletRequest.getContextPath()).andReturn("/a_context_path");
+    expect(servletRequest.getRequestURI()).andReturn("/a_context_path/test.html");
+
+    replay(servletRequest, binding, injector);
+    filterDef.init(servletContext, injector,
+        Sets.newSetFromMap(Maps.<Filter, Boolean>newIdentityHashMap()));
+    Filter filter = filterDef.getFilterIfMatching(servletRequest);
+    assertSame(filter, mockFilter);
+    verify(servletRequest, binding, injector);
+  }
+
+  public void testGetFilterIfMatchingNotMatching() throws ServletException {
+    String pattern = "/*";
+    final FilterDefinition filterDef = new FilterDefinition(pattern, Key.get(Filter.class),
+        UriPatternType.get(UriPatternType.SERVLET, pattern),
+        new HashMap<String, String>(), null);
+    HttpServletRequest servletRequest = createMock(HttpServletRequest.class);
+    ServletContext servletContext = createMock(ServletContext.class);
+    Injector injector = createMock(Injector.class);
+    Binding binding = createMock(Binding.class);
+
+    final MockFilter mockFilter = new MockFilter() {
+      @Override
+      public void doFilter(ServletRequest servletRequest, ServletResponse servletResponse,
+          FilterChain filterChain) {
+        //suppress rest of chain...
+      }
+    };
+    expect(injector.getBinding(Key.get(Filter.class)))
+        .andReturn(binding);
+    expect(binding.acceptScopingVisitor((BindingScopingVisitor) anyObject()))
+        .andReturn(true);
+    expect(injector.getInstance(Key.get(Filter.class)))
+        .andReturn(mockFilter)
+        .anyTimes();
+
+    expect(servletRequest.getContextPath()).andReturn("/a_context_path");
+    expect(servletRequest.getRequestURI()).andReturn("/test.html");
+
+    replay(servletRequest, binding, injector);
+    filterDef.init(servletContext, injector,
+        Sets.newSetFromMap(Maps.<Filter, Boolean>newIdentityHashMap()));
+    Filter filter = filterDef.getFilterIfMatching(servletRequest);
+    assertNull(filter);
+    verify(servletRequest, binding, injector);
+  }
+
   private static class MockFilter implements Filter {
     private boolean init;
     private boolean destroy;
diff --git a/extensions/servlet/test/com/google/inject/servlet/ServletDefinitionTest.java b/extensions/servlet/test/com/google/inject/servlet/ServletDefinitionTest.java
index 7ebc521..1a8224a 100644
--- a/extensions/servlet/test/com/google/inject/servlet/ServletDefinitionTest.java
+++ b/extensions/servlet/test/com/google/inject/servlet/ServletDefinitionTest.java
@@ -22,6 +22,7 @@
 import static org.easymock.EasyMock.replay;
 import static org.easymock.EasyMock.verify;
 
+import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Maps;
 import com.google.common.collect.Sets;
 import com.google.inject.Binding;
@@ -31,14 +32,16 @@
 
 import junit.framework.TestCase;
 
+import java.io.IOException;
 import java.util.Enumeration;
-import java.util.HashMap;
 import java.util.Map;
 
 import javax.servlet.ServletConfig;
 import javax.servlet.ServletContext;
 import javax.servlet.ServletException;
 import javax.servlet.http.HttpServlet;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
 
 /**
  * Basic unit test for lifecycle of a ServletDefinition (wrapper).
@@ -65,15 +68,9 @@
 
     //some init params
     //noinspection SSBasedInspection
-    final Map<String, String> initParams = new HashMap<String, String>() {
-      {
-        put("ahsd", "asdas24dok");
-        put("ahssd", "asdasd124ok");
-        put("ahfsasd", "asda124sdok");
-        put("ahsasgd", "a124sdasdok");
-        put("ahsd124124", "as124124124dasdok");
-      }
-    };
+    final Map<String, String> initParams = new ImmutableMap.Builder<String, String>()
+      .put("ahsd", "asdas24dok")
+      .put("ahssd", "asdasd124ok").build();
 
     String pattern = "/*";
     final ServletDefinition servletDefinition = new ServletDefinition(pattern,
@@ -101,7 +98,28 @@
       assertTrue(initParams.containsKey(name));
       assertEquals(initParams.get(name), servletConfig.getInitParameter(name));
     }
-    
+
     verify(injector, binding, servletContext);
   }
+
+  public void testServiceWithContextPath() throws IOException, ServletException   {
+    String pattern = "/*";
+    //some init params
+    Map<String, String> initParams = new ImmutableMap.Builder<String, String>()
+        .put("ahsd", "asdas24dok")
+        .put("ahssd", "asdasd124ok")
+        .build();
+
+    final ServletDefinition servletDefinition = new ServletDefinition(pattern,
+        Key.get(HttpServlet.class), UriPatternType.get(UriPatternType.SERVLET, pattern),
+        initParams, null);
+    HttpServletResponse servletResponse = createMock(HttpServletResponse.class);
+    HttpServletRequest servletRequest = createMock(HttpServletRequest.class);
+
+    expect(servletRequest.getContextPath()).andReturn("/a_context_path");
+    expect(servletRequest.getRequestURI()).andReturn("/test.html");
+    replay(servletRequest, servletResponse);
+    servletDefinition.service(servletRequest, servletResponse);
+    verify(servletRequest, servletResponse);
+  }
 }
diff --git a/extensions/servlet/test/com/google/inject/servlet/ServletUtilsTest.java b/extensions/servlet/test/com/google/inject/servlet/ServletUtilsTest.java
new file mode 100644
index 0000000..bd617ab
--- /dev/null
+++ b/extensions/servlet/test/com/google/inject/servlet/ServletUtilsTest.java
@@ -0,0 +1,58 @@
+// Copyright 2012 Google Inc. All Rights Reserved.
+
+package com.google.inject.servlet;
+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 junit.framework.TestCase;
+
+import javax.servlet.http.HttpServletRequest;
+
+/**
+ * Unit test for the servlet utility class.
+ *
+ * @author ntang@google.com (Michael Tang)
+ */
+public class ServletUtilsTest extends TestCase {
+  public void testGetContextRelativePath() {
+    HttpServletRequest servletRequest = createMock(HttpServletRequest.class);
+    expect(servletRequest.getContextPath()).andReturn("/a_context_path");
+    expect(servletRequest.getRequestURI()).andReturn("/a_context_path/test.html");
+    replay(servletRequest);
+    String path = ServletUtils.getContextRelativePath(servletRequest);
+    assertEquals("/test.html", path);
+    verify(servletRequest);
+  }
+
+  public void testGetContextRelativePathWithWrongPath() {
+    HttpServletRequest servletRequest = createMock(HttpServletRequest.class);
+    expect(servletRequest.getContextPath()).andReturn("/a_context_path");
+    expect(servletRequest.getRequestURI()).andReturn("/test.html");
+    replay(servletRequest);
+    String path = ServletUtils.getContextRelativePath(servletRequest);
+    assertNull(path);
+    verify(servletRequest);
+  }
+
+  public void testGetContextRelativePathWithRootPath() {
+    HttpServletRequest servletRequest = createMock(HttpServletRequest.class);
+    expect(servletRequest.getContextPath()).andReturn("/a_context_path");
+    expect(servletRequest.getRequestURI()).andReturn("/a_context_path");
+    replay(servletRequest);
+    String path = ServletUtils.getContextRelativePath(servletRequest);
+    assertEquals("/", path);
+    verify(servletRequest);
+  }
+
+  public void testGetContextRelativePathWithEmptyPath() {
+    HttpServletRequest servletRequest = createMock(HttpServletRequest.class);
+    expect(servletRequest.getContextPath()).andReturn("");
+    expect(servletRequest.getRequestURI()).andReturn("");
+    replay(servletRequest);
+    String path = ServletUtils.getContextRelativePath(servletRequest);
+    assertNull(path);
+    verify(servletRequest);
+  }
+}