Merge pull request #211 from apache/SHIRO-753

[SHIRO-753] The context path is no longer used when determining the path application path
diff --git a/support/guice/src/test/java/org/apache/shiro/guice/web/FilterConfigTest.java b/support/guice/src/test/java/org/apache/shiro/guice/web/FilterConfigTest.java
index 75324ad..3919207 100644
--- a/support/guice/src/test/java/org/apache/shiro/guice/web/FilterConfigTest.java
+++ b/support/guice/src/test/java/org/apache/shiro/guice/web/FilterConfigTest.java
@@ -85,8 +85,7 @@
     private HttpServletRequest createMockRequest(String path) {

         HttpServletRequest request = createNiceMock(HttpServletRequest.class);

 

-        expect(request.getAttribute(WebUtils.INCLUDE_CONTEXT_PATH_ATTRIBUTE)).andReturn(null).anyTimes();

-        expect(request.getContextPath()).andReturn("");

+        expect(request.getServletPath()).andReturn("");

         expect(request.getPathInfo()).andReturn(path);

         replay(request);

         return request;

diff --git a/support/guice/src/test/java/org/apache/shiro/guice/web/ShiroWebModuleTest.java b/support/guice/src/test/java/org/apache/shiro/guice/web/ShiroWebModuleTest.java
index ff50d0f..9931d01 100644
--- a/support/guice/src/test/java/org/apache/shiro/guice/web/ShiroWebModuleTest.java
+++ b/support/guice/src/test/java/org/apache/shiro/guice/web/ShiroWebModuleTest.java
@@ -177,11 +177,13 @@
         servletContext.setAttribute(eq(EnvironmentLoader.ENVIRONMENT_ATTRIBUTE_KEY), EasyMock.anyObject());

         expect(request.getAttribute("javax.servlet.include.context_path")).andReturn("").anyTimes();

         expect(request.getCharacterEncoding()).andReturn("UTF-8").anyTimes();

-        expect(request.getAttribute("javax.servlet.include.request_uri")).andReturn("/test_authc");

-        expect(request.getAttribute("javax.servlet.include.request_uri")).andReturn("/test_custom_filter");

-        expect(request.getAttribute("javax.servlet.include.request_uri")).andReturn("/test_authc_basic");

-        expect(request.getAttribute("javax.servlet.include.request_uri")).andReturn("/test_perms");

-        expect(request.getAttribute("javax.servlet.include.request_uri")).andReturn("/multiple_configs");

+        expect(request.getAttribute("javax.servlet.include.path_info")).andReturn(null).anyTimes();

+        expect(request.getPathInfo()).andReturn(null).anyTimes();

+        expect(request.getAttribute("javax.servlet.include.servlet_path")).andReturn("/test_authc");

+        expect(request.getAttribute("javax.servlet.include.servlet_path")).andReturn("/test_custom_filter");

+        expect(request.getAttribute("javax.servlet.include.servlet_path")).andReturn("/test_authc_basic");

+        expect(request.getAttribute("javax.servlet.include.servlet_path")).andReturn("/test_perms");

+        expect(request.getAttribute("javax.servlet.include.servlet_path")).andReturn("/multiple_configs");

         replay(servletContext, request);

 

         Injector injector = Guice.createInjector(new ShiroWebModule(servletContext) {

diff --git a/support/guice/src/test/java/org/apache/shiro/guice/web/SimpleFilterChainResolverTest.java b/support/guice/src/test/java/org/apache/shiro/guice/web/SimpleFilterChainResolverTest.java
index d500da8..586a2b9 100644
--- a/support/guice/src/test/java/org/apache/shiro/guice/web/SimpleFilterChainResolverTest.java
+++ b/support/guice/src/test/java/org/apache/shiro/guice/web/SimpleFilterChainResolverTest.java
@@ -77,8 +77,8 @@
         ServletResponse response = ctrl.createMock(HttpServletResponse.class);

         FilterChain originalChain = ctrl.createMock(FilterChain.class);

 

-        expect(request.getAttribute(WebUtils.INCLUDE_CONTEXT_PATH_ATTRIBUTE)).andReturn("/context");

-        expect(request.getAttribute(WebUtils.INCLUDE_REQUEST_URI_ATTRIBUTE)).andReturn("/mychain");

+        expect(request.getAttribute(WebUtils.INCLUDE_SERVLET_PATH_ATTRIBUTE)).andReturn("/mychain");

+        expect(request.getAttribute(WebUtils.INCLUDE_PATH_INFO_ATTRIBUTE)).andReturn("");

 

         expect(request.getCharacterEncoding()).andStubReturn(null);

 

@@ -108,8 +108,8 @@
 

         ctrl.reset();

 

-        expect(request.getAttribute(WebUtils.INCLUDE_CONTEXT_PATH_ATTRIBUTE)).andReturn("/context");

-        expect(request.getAttribute(WebUtils.INCLUDE_REQUEST_URI_ATTRIBUTE)).andReturn("/nochain");

+        expect(request.getAttribute(WebUtils.INCLUDE_SERVLET_PATH_ATTRIBUTE)).andReturn("/nochain");

+        expect(request.getAttribute(WebUtils.INCLUDE_PATH_INFO_ATTRIBUTE)).andReturn("");

 

         expect(request.getCharacterEncoding()).andStubReturn(null);

 

diff --git a/web/src/main/java/org/apache/shiro/web/util/WebUtils.java b/web/src/main/java/org/apache/shiro/web/util/WebUtils.java
index 0700e67..b9c9f62 100644
--- a/web/src/main/java/org/apache/shiro/web/util/WebUtils.java
+++ b/web/src/main/java/org/apache/shiro/web/util/WebUtils.java
@@ -109,16 +109,7 @@
      * @return the path within the web application
      */
     public static String getPathWithinApplication(HttpServletRequest request) {
-        String contextPath = getContextPath(request);
-        String requestUri = getRequestUri(request);
-        if (StringUtils.startsWithIgnoreCase(requestUri, contextPath)) {
-            // Normal case: URI contains context path.
-            String path = requestUri.substring(contextPath.length());
-            return (StringUtils.hasText(path) ? path : "/");
-        } else {
-            // Special case: rather unusual.
-            return requestUri;
-        }
+        return normalize(removeSemicolon(getServletPath(request) + getPathInfo(request)));
     }
 
     /**
@@ -132,17 +123,27 @@
      *
      * @param request current HTTP request
      * @return the request URI
+     * @deprecated use getPathWithinApplication() to get the path minus the context path, or call HttpServletRequest.getRequestURI() directly from your code.
      */
+    @Deprecated
     public static String getRequestUri(HttpServletRequest request) {
         String uri = (String) request.getAttribute(INCLUDE_REQUEST_URI_ATTRIBUTE);
         if (uri == null) {
-            uri = valueOrEmpty(request.getContextPath()) + "/" +
-                  valueOrEmpty(request.getServletPath()) +
-                  valueOrEmpty(request.getPathInfo());
+            uri = request.getRequestURI();
         }
         return normalize(decodeAndCleanUriString(request, uri));
     }
 
+    private static String getServletPath(HttpServletRequest request) {
+        String servletPath = (String) request.getAttribute(INCLUDE_SERVLET_PATH_ATTRIBUTE);
+        return servletPath != null ? servletPath : valueOrEmpty(request.getServletPath());
+    }
+
+    private static String getPathInfo(HttpServletRequest request) {
+        String pathInfo = (String) request.getAttribute(INCLUDE_PATH_INFO_ATTRIBUTE);
+        return pathInfo != null ? pathInfo : valueOrEmpty(request.getPathInfo());
+    }
+
     private static String valueOrEmpty(String input) {
         if (input == null) {
             return "";
@@ -240,6 +241,10 @@
      */
     private static String decodeAndCleanUriString(HttpServletRequest request, String uri) {
         uri = decodeRequestString(request, uri);
+        return removeSemicolon(uri);
+    }
+
+    private static String removeSemicolon(String uri) {
         int semicolonIndex = uri.indexOf(';');
         return (semicolonIndex != -1 ? uri.substring(0, semicolonIndex) : uri);
     }
diff --git a/web/src/test/groovy/org/apache/shiro/web/util/WebUtilsTest.groovy b/web/src/test/groovy/org/apache/shiro/web/util/WebUtilsTest.groovy
index 26be858..7956a10 100644
--- a/web/src/test/groovy/org/apache/shiro/web/util/WebUtilsTest.groovy
+++ b/web/src/test/groovy/org/apache/shiro/web/util/WebUtilsTest.groovy
@@ -143,63 +143,85 @@
     @Test
     void testGetRequestUriWithServlet() {
 
-        dotTestGetPathWithinApplicationFromRequest("/", "/servlet", "/foobar", "/servlet/foobar")
-        dotTestGetPathWithinApplicationFromRequest("", "/servlet", "/foobar", "/servlet/foobar")
-        dotTestGetPathWithinApplicationFromRequest("", "servlet", "/foobar", "/servlet/foobar")
-        dotTestGetPathWithinApplicationFromRequest("/", "servlet", "/foobar", "/servlet/foobar")
-        dotTestGetPathWithinApplicationFromRequest("//", "servlet", "/foobar", "/servlet/foobar")
-        dotTestGetPathWithinApplicationFromRequest("//", "//servlet", "//foobar", "/servlet/foobar")
-        dotTestGetPathWithinApplicationFromRequest("/context-path", "/servlet", "/foobar", "/servlet/foobar")
-        dotTestGetPathWithinApplicationFromRequest("//context-path", "//servlet", "//foobar", "/servlet/foobar")
-        dotTestGetPathWithinApplicationFromRequest("//context-path", "/servlet", "/../servlet/other", "/servlet/other")
-        dotTestGetPathWithinApplicationFromRequest("//context-path", "/asdf", "/../servlet/other", "/servlet/other")
-        dotTestGetPathWithinApplicationFromRequest("//context-path", "/asdf", ";/../servlet/other", "/asdf")
-        dotTestGetPathWithinApplicationFromRequest("/context%2525path", "/servlet", "/foobar", "/servlet/foobar")
-        dotTestGetPathWithinApplicationFromRequest("/c%6Fntext%20path", "/servlet", "/foobar", "/servlet/foobar")
-        dotTestGetPathWithinApplicationFromRequest("/context path", "/servlet", "/foobar", "/servlet/foobar")
-        dotTestGetPathWithinApplicationFromRequest("", null, null, "/")
-        dotTestGetPathWithinApplicationFromRequest("", "index.jsp", null, "/index.jsp")
+        dotTestGetPathWithinApplicationFromRequest("/servlet", "/foobar", "/servlet/foobar")
+        dotTestGetPathWithinApplicationFromRequest("/servlet", "/foobar", "/servlet/foobar")
+        dotTestGetPathWithinApplicationFromRequest("servlet", "/foobar", "/servlet/foobar")
+        dotTestGetPathWithinApplicationFromRequest("servlet", "/foobar", "/servlet/foobar")
+        dotTestGetPathWithinApplicationFromRequest("servlet", "/foobar", "/servlet/foobar")
+        dotTestGetPathWithinApplicationFromRequest("//servlet", "//foobar", "/servlet/foobar")
+        dotTestGetPathWithinApplicationFromRequest("/servlet", "/foobar", "/servlet/foobar")
+        dotTestGetPathWithinApplicationFromRequest("//servlet", "//foobar", "/servlet/foobar")
+        dotTestGetPathWithinApplicationFromRequest("/servlet", "/../servlet/other", "/servlet/other")
+        dotTestGetPathWithinApplicationFromRequest("/asdf", "/../servlet/other", "/servlet/other")
+        dotTestGetPathWithinApplicationFromRequest("/asdf/foo", ";/../servlet/other", "/asdf/foo")
+        dotTestGetPathWithinApplicationFromRequest("/servlet", "/foobar", "/servlet/foobar")
+        dotTestGetPathWithinApplicationFromRequest("/servlet", "/foobar", "/servlet/foobar")
+        dotTestGetPathWithinApplicationFromRequest("/servlet", "/foobar", "/servlet/foobar")
+        dotTestGetPathWithinApplicationFromRequest(null, null, "/")
+        dotTestGetPathWithinApplicationFromRequest("index.jsp", null, "/index.jsp")
     }
 
     @Test
     void testGetPathWithinApplication() {
 
-        doTestGetPathWithinApplication("/", "/foobar", "/foobar");
-        doTestGetPathWithinApplication("", "/foobar", "/foobar");
-        doTestGetPathWithinApplication("", "foobar", "/foobar");
-        doTestGetPathWithinApplication("/", "foobar", "/foobar");
-        doTestGetPathWithinApplication("//", "foobar", "/foobar");
-        doTestGetPathWithinApplication("//", "//foobar", "/foobar");
-        doTestGetPathWithinApplication("/context-path", "/context-path/foobar", "/foobar");
-        doTestGetPathWithinApplication("/context-path", "/context-path/foobar/", "/foobar/");
-        doTestGetPathWithinApplication("//context-path", "//context-path/foobar", "/foobar");
-        doTestGetPathWithinApplication("//context-path", "//context-path//foobar", "/foobar");
-        doTestGetPathWithinApplication("//context-path", "//context-path/remove-one/remove-two/../../././/foobar", "/foobar");
-        doTestGetPathWithinApplication("//context-path", "//context-path//../../././/foobar", null);
-        doTestGetPathWithinApplication("/context%2525path", "/context%2525path/foobar", "/foobar");
-        doTestGetPathWithinApplication("/c%6Fntext%20path", "/c%6Fntext%20path/foobar", "/foobar");
-        doTestGetPathWithinApplication("/context path", "/context path/foobar", "/foobar");
-
+        doTestGetPathWithinApplication("/foobar", null, "/foobar");
+        doTestGetPathWithinApplication("/foobar", "", "/foobar");
+        doTestGetPathWithinApplication("", "/", "/");
+        doTestGetPathWithinApplication("", null, "/");
+        doTestGetPathWithinApplication("/foobar", "//", "/foobar/");
+        doTestGetPathWithinApplication("/foobar", "//extra", "/foobar/extra");
+        doTestGetPathWithinApplication("/foobar", "//extra///", "/foobar/extra/");
+        doTestGetPathWithinApplication("/foo bar", "/path info" ,"/foo bar/path info");
     }
 
-    void doTestGetPathWithinApplication(String contextPath, String requestUri, String expectedValue) {
+    @Test
+    void testGetRequestURI() {
+        doTestGetRequestURI("/foobar", "/foobar")
+        doTestGetRequestURI( "/foobar/", "/foobar/")
+        doTestGetRequestURI("",  "/");
+        doTestGetRequestURI("foobar", "/foobar");
+        doTestGetRequestURI("//foobar", "/foobar");
+        doTestGetRequestURI("//foobar///", "/foobar/");
+        doTestGetRequestURI("/context-path/foobar", "/context-path/foobar");
+        doTestGetRequestURI("/context-path/foobar/", "/context-path/foobar/");
+        doTestGetRequestURI("//context-path/foobar", "/context-path/foobar");
+        doTestGetRequestURI("//context-path//foobar", "/context-path/foobar");
+        doTestGetRequestURI("//context-path/remove-one/remove-two/../../././/foobar", "/context-path/foobar");
+        doTestGetRequestURI("//context-path//../../././/foobar", null);
+        doTestGetRequestURI("/context%2525path/foobar", "/context%25path/foobar");
+        doTestGetRequestURI("/c%6Fntext%20path/foobar", "/context path/foobar");
+        doTestGetRequestURI("/context path/foobar", "/context path/foobar");
+    }
+
+    void doTestGetPathWithinApplication(String servletPath, String pathInfo, String expectedValue) {
 
         def request = createMock(HttpServletRequest)
-        expect(request.getAttribute(WebUtils.INCLUDE_CONTEXT_PATH_ATTRIBUTE)).andReturn(contextPath)
-        expect(request.getAttribute(WebUtils.INCLUDE_REQUEST_URI_ATTRIBUTE)).andReturn(requestUri)
-        expect(request.getCharacterEncoding()).andReturn("UTF-8").times(2)
+        expect(request.getAttribute(WebUtils.INCLUDE_SERVLET_PATH_ATTRIBUTE)).andReturn(servletPath)
+        expect(request.getAttribute(WebUtils.INCLUDE_PATH_INFO_ATTRIBUTE)).andReturn(pathInfo)
+        if (pathInfo == null) {
+            expect(request.getPathInfo()).andReturn(null) // path info can be null
+        }
         replay request
         assertEquals expectedValue, WebUtils.getPathWithinApplication(request)
         verify request
     }
 
-    void dotTestGetPathWithinApplicationFromRequest(String contextPath, String servletPath, String pathInfo, String expectedValue) {
+    void doTestGetRequestURI(String rawRequestUri, String expectedValue) {
+
+        def request = createMock(HttpServletRequest)
+        expect(request.getAttribute(WebUtils.INCLUDE_REQUEST_URI_ATTRIBUTE)).andReturn(rawRequestUri)
+        expect(request.getCharacterEncoding()).andReturn("UTF-8").times(1)
+        replay request
+        assertEquals expectedValue, WebUtils.getRequestUri(request)
+        verify request
+    }
+
+    void dotTestGetPathWithinApplicationFromRequest(String servletPath, String pathInfo, String expectedValue) {
 
         HttpServletRequest request = createMock(HttpServletRequest)
-        expect(request.getAttribute(WebUtils.INCLUDE_CONTEXT_PATH_ATTRIBUTE)).andReturn(null)
-        expect(request.getAttribute(WebUtils.INCLUDE_REQUEST_URI_ATTRIBUTE)).andReturn(null)
+        expect(request.getAttribute(WebUtils.INCLUDE_SERVLET_PATH_ATTRIBUTE)).andReturn(null)
+        expect(request.getAttribute(WebUtils.INCLUDE_PATH_INFO_ATTRIBUTE)).andReturn(null)
         expect(request.getServletPath()).andReturn(servletPath)
-        expect(request.getContextPath()).andReturn(contextPath).times(2)
         expect(request.getPathInfo()).andReturn(pathInfo)
         expect(request.getCharacterEncoding()).andReturn("UTF-8").anyTimes()
         replay request
diff --git a/web/src/test/java/org/apache/shiro/web/filter/mgt/PathMatchingFilterChainResolverTest.java b/web/src/test/java/org/apache/shiro/web/filter/mgt/PathMatchingFilterChainResolverTest.java
index 99bac0d..963e89e 100644
--- a/web/src/test/java/org/apache/shiro/web/filter/mgt/PathMatchingFilterChainResolverTest.java
+++ b/web/src/test/java/org/apache/shiro/web/filter/mgt/PathMatchingFilterChainResolverTest.java
@@ -97,8 +97,6 @@
         //ensure at least one chain is defined:
         resolver.getFilterChainManager().addToChain("/index.html", "authcBasic");
 
-        expect(request.getAttribute(WebUtils.INCLUDE_CONTEXT_PATH_ATTRIBUTE)).andReturn(null).anyTimes();
-        expect(request.getContextPath()).andReturn("");
         expect(request.getServletPath()).andReturn("");
         expect(request.getPathInfo()).andReturn("/index.html");
         replay(request);
@@ -117,8 +115,6 @@
         //ensure at least one chain is defined:
         resolver.getFilterChainManager().addToChain("/index.html", "authcBasic");
 
-        expect(request.getAttribute(WebUtils.INCLUDE_CONTEXT_PATH_ATTRIBUTE)).andReturn(null).anyTimes();
-        expect(request.getContextPath()).andReturn("");
         expect(request.getServletPath()).andReturn("/");
         expect(request.getPathInfo()).andReturn("./index.html");
         replay(request);
@@ -136,9 +132,6 @@
 
         //ensure at least one chain is defined:
         resolver.getFilterChainManager().addToChain("/index.html", "authcBasic");
-
-        expect(request.getAttribute(WebUtils.INCLUDE_CONTEXT_PATH_ATTRIBUTE)).andReturn(null).anyTimes();
-        expect(request.getContextPath()).andReturn("");
         expect(request.getServletPath()).andReturn("/public/");
         expect(request.getPathInfo()).andReturn("../index.html");
         replay(request);
@@ -157,8 +150,6 @@
         //ensure at least one chain is defined:
         resolver.getFilterChainManager().addToChain("/index.html", "authcBasic");
 
-        expect(request.getAttribute(WebUtils.INCLUDE_CONTEXT_PATH_ATTRIBUTE)).andReturn(null).anyTimes();
-        expect(request.getContextPath()).andReturn("");
         expect(request.getServletPath()).andReturn("/");
         expect(request.getPathInfo()).andReturn(null);
         replay(request);
@@ -180,8 +171,6 @@
         //ensure at least one chain is defined:
         resolver.getFilterChainManager().addToChain("/resource/book", "authcBasic");
 
-        expect(request.getAttribute(WebUtils.INCLUDE_CONTEXT_PATH_ATTRIBUTE)).andReturn(null).anyTimes();
-        expect(request.getContextPath()).andReturn("");
         expect(request.getServletPath()).andReturn("");
         expect(request.getPathInfo()).andReturn("/resource/book");
         replay(request);
@@ -203,8 +192,6 @@
         //ensure at least one chain is defined:
         resolver.getFilterChainManager().addToChain("/", "authcBasic");
 
-        expect(request.getAttribute(WebUtils.INCLUDE_CONTEXT_PATH_ATTRIBUTE)).andReturn(null).anyTimes();
-        expect(request.getContextPath()).andReturn("");
         expect(request.getServletPath()).andReturn("/");
         expect(request.getPathInfo()).andReturn(null);
         replay(request);
@@ -226,8 +213,6 @@
         //ensure at least one chain is defined:
         resolver.getFilterChainManager().addToChain("/resource/book", "authcBasic");
 
-        expect(request.getAttribute(WebUtils.INCLUDE_CONTEXT_PATH_ATTRIBUTE)).andReturn(null).anyTimes();
-        expect(request.getContextPath()).andReturn("");
         expect(request.getServletPath()).andReturn("");
         expect(request.getPathInfo()).andReturn("/resource/book");
         replay(request);
@@ -249,8 +234,6 @@
         //ensure at least one chain is defined:
         resolver.getFilterChainManager().addToChain("/resource/book", "authcBasic");
 
-        expect(request.getAttribute(WebUtils.INCLUDE_CONTEXT_PATH_ATTRIBUTE)).andReturn(null).anyTimes();
-        expect(request.getContextPath()).andReturn("");
         expect(request.getServletPath()).andReturn("");
         expect(request.getPathInfo()).andReturn("/resource/book//");
         replay(request);