You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@shiro.apache.org by bd...@apache.org on 2020/04/07 19:57:09 UTC
[shiro] 01/01: The context path is no longer used when determining
the path application path
This is an automated email from the ASF dual-hosted git repository.
bdemers pushed a commit to branch SHIRO-753
in repository https://gitbox.apache.org/repos/asf/shiro.git
commit b90f91875e5e18c4805013c2fa0567b1700f5a96
Author: Brian Demers <bd...@apache.org>
AuthorDate: Tue Apr 7 15:56:56 2020 -0400
The context path is no longer used when determining the path application path
Servlet-Path + Path-Info is used instead
NOTE: some servlet containers will decode the context-path (Tomcat) and others will not (Jetty). Per the Servlet spect the servlet-path and path-info are always decoded.
Shiro only makes decisions based on the application path, I.e. the request URI with the context-path removed, so we can just avoid dealing with it all together.
`WebUtils.getRequestURI()` has been deprecated to reflect this, and the previous behavior restored (as there was an issue with double decoding the URL in SHIRO-753)
Fixes: SHIRO-753
---
.../apache/shiro/guice/web/FilterConfigTest.java | 3 +-
.../apache/shiro/guice/web/ShiroWebModuleTest.java | 12 +--
.../guice/web/SimpleFilterChainResolverTest.java | 8 +-
.../java/org/apache/shiro/web/util/WebUtils.java | 31 ++++---
.../org/apache/shiro/web/util/WebUtilsTest.groovy | 100 +++++++++++++--------
.../mgt/PathMatchingFilterChainResolverTest.java | 17 ----
6 files changed, 91 insertions(+), 80 deletions(-)
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 @@ public class FilterConfigTest {
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 @@ public class ShiroWebModuleTest {
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 @@ public class SimpleFilterChainResolverTest {
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 @@ public class SimpleFilterChainResolverTest {
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 @@ public class WebUtils {
* @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 @@ public class WebUtils {
*
* @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 @@ public class WebUtils {
*/
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 @@ public class WebUtilsTest {
@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");
+ }
+ @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 contextPath, String requestUri, String expectedValue) {
+ 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 @@ public class PathMatchingFilterChainResolverTest extends WebTest {
//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 @@ public class PathMatchingFilterChainResolverTest extends WebTest {
//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 @@ public class PathMatchingFilterChainResolverTest extends WebTest {
//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 @@ public class PathMatchingFilterChainResolverTest extends WebTest {
//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 @@ public class PathMatchingFilterChainResolverTest extends WebTest {
//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 @@ public class PathMatchingFilterChainResolverTest extends WebTest {
//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 @@ public class PathMatchingFilterChainResolverTest extends WebTest {
//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 @@ public class PathMatchingFilterChainResolverTest extends WebTest {
//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);