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:08 UTC
[shiro] branch SHIRO-753 created (now b90f918)
This is an automated email from the ASF dual-hosted git repository.
bdemers pushed a change to branch SHIRO-753
in repository https://gitbox.apache.org/repos/asf/shiro.git.
at b90f918 The context path is no longer used when determining the path application path
This branch includes the following new commits:
new b90f918 The context path is no longer used when determining the path application path
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
[shiro] 01/01: The context path is no longer used when determining
the path application path
Posted by bd...@apache.org.
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);