From 3708d7907016bf2fa12691dff6ff0def1249b8ce Mon Sep 17 00:00:00 2001 From: Brian Demers Date: Tue, 17 Mar 2020 16:56:36 -0400 Subject: [PATCH] Add tests for WebUtils --- .../shiro/guice/web/FilterConfigTest.java | 2 +- .../org/apache/shiro/web/util/WebUtils.java | 11 +++++- .../apache/shiro/web/util/WebUtilsTest.groovy | 34 +++++++++++++++++++ .../web/filter/PathMatchingFilterTest.java | 10 ++++++ .../PathMatchingFilterChainResolverTest.java | 24 ++++++++----- 5 files changed, 71 insertions(+), 10 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 c1b3c32295..75324adb47 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 @@ -87,7 +87,7 @@ private HttpServletRequest createMockRequest(String path) { expect(request.getAttribute(WebUtils.INCLUDE_CONTEXT_PATH_ATTRIBUTE)).andReturn(null).anyTimes(); expect(request.getContextPath()).andReturn(""); - expect(request.getRequestURI()).andReturn(path); + expect(request.getPathInfo()).andReturn(path); replay(request); return request; } 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 c61d2447d9..0700e675cf 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 @@ -136,11 +136,20 @@ public static String getPathWithinApplication(HttpServletRequest request) { public static String getRequestUri(HttpServletRequest request) { String uri = (String) request.getAttribute(INCLUDE_REQUEST_URI_ATTRIBUTE); if (uri == null) { - uri = request.getRequestURI(); + uri = valueOrEmpty(request.getContextPath()) + "/" + + valueOrEmpty(request.getServletPath()) + + valueOrEmpty(request.getPathInfo()); } return normalize(decodeAndCleanUriString(request, uri)); } + private static String valueOrEmpty(String input) { + if (input == null) { + return ""; + } + return input; + } + /** * Normalize a relative URI path that may have relative values ("/./", * "/../", and so on ) it it. WARNING - This method is 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 11d225f46d..26be858746 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 @@ -140,6 +140,26 @@ 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") + } @Test void testGetPathWithinApplication() { @@ -172,4 +192,18 @@ public class WebUtilsTest { assertEquals expectedValue, WebUtils.getPathWithinApplication(request) verify request } + + void dotTestGetPathWithinApplicationFromRequest(String contextPath, 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.getServletPath()).andReturn(servletPath) + expect(request.getContextPath()).andReturn(contextPath).times(2) + expect(request.getPathInfo()).andReturn(pathInfo) + expect(request.getCharacterEncoding()).andReturn("UTF-8").anyTimes() + replay request + assertEquals expectedValue, WebUtils.getPathWithinApplication(request) + verify request + } } diff --git a/web/src/test/java/org/apache/shiro/web/filter/PathMatchingFilterTest.java b/web/src/test/java/org/apache/shiro/web/filter/PathMatchingFilterTest.java index c6e81d4139..e1e29b022c 100644 --- a/web/src/test/java/org/apache/shiro/web/filter/PathMatchingFilterTest.java +++ b/web/src/test/java/org/apache/shiro/web/filter/PathMatchingFilterTest.java @@ -112,6 +112,8 @@ public void testEnabled() throws Exception { expect(request.getContextPath()).andReturn(CONTEXT_PATH).anyTimes(); expect(request.getRequestURI()).andReturn(ENABLED_PATH).anyTimes(); + expect(request.getServletPath()).andReturn("").anyTimes(); + expect(request.getPathInfo()).andReturn(ENABLED_PATH).anyTimes(); replay(request); boolean continueFilterChain = filter.preHandle(request, response); @@ -128,6 +130,8 @@ public void testEnabled() throws Exception { public void testPathMatchEqualUrlSeparatorEnabled() { expect(request.getContextPath()).andReturn(CONTEXT_PATH).anyTimes(); expect(request.getRequestURI()).andReturn("/").anyTimes(); + expect(request.getServletPath()).andReturn("").anyTimes(); + expect(request.getPathInfo()).andReturn("/").anyTimes(); replay(request); boolean matchEnabled = filter.pathsMatch("/", request); @@ -142,6 +146,8 @@ public void testPathMatchEqualUrlSeparatorEnabled() { public void testPathMatchEEnabled() { expect(request.getContextPath()).andReturn(CONTEXT_PATH).anyTimes(); expect(request.getRequestURI()).andReturn("/resource/book").anyTimes(); + expect(request.getServletPath()).andReturn("").anyTimes(); + expect(request.getPathInfo()).andReturn("/resource/book").anyTimes(); replay(request); boolean matchEnabled = filter.pathsMatch("/resource/book", request); @@ -156,6 +162,8 @@ public void testPathMatchEEnabled() { public void testPathMatchEndWithUrlSeparatorEnabled() { expect(request.getContextPath()).andReturn(CONTEXT_PATH).anyTimes(); expect(request.getRequestURI()).andReturn("/resource/book/").anyTimes(); + expect(request.getServletPath()).andReturn("").anyTimes(); + expect(request.getPathInfo()).andReturn("/resource/book/").anyTimes(); replay(request); boolean matchEnabled = filter.pathsMatch("/resource/book", request); @@ -170,6 +178,8 @@ public void testPathMatchEndWithUrlSeparatorEnabled() { public void testPathMatchEndWithMultiUrlSeparatorEnabled() { expect(request.getContextPath()).andReturn(CONTEXT_PATH).anyTimes(); expect(request.getRequestURI()).andReturn("/resource/book//").anyTimes(); + expect(request.getServletPath()).andReturn("").anyTimes(); + expect(request.getPathInfo()).andReturn("/resource/book//").anyTimes(); replay(request); boolean matchEnabled = filter.pathsMatch("/resource/book", 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 758be4d1f3..99bac0dccb 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 @@ -99,7 +99,8 @@ public void testGetChainsWithMatch() { expect(request.getAttribute(WebUtils.INCLUDE_CONTEXT_PATH_ATTRIBUTE)).andReturn(null).anyTimes(); expect(request.getContextPath()).andReturn(""); - expect(request.getRequestURI()).andReturn("/index.html"); + expect(request.getServletPath()).andReturn(""); + expect(request.getPathInfo()).andReturn("/index.html"); replay(request); FilterChain resolved = resolver.getChain(request, response, chain); @@ -118,7 +119,8 @@ public void testPathTraversalWithDot() { expect(request.getAttribute(WebUtils.INCLUDE_CONTEXT_PATH_ATTRIBUTE)).andReturn(null).anyTimes(); expect(request.getContextPath()).andReturn(""); - expect(request.getRequestURI()).andReturn("/./index.html"); + expect(request.getServletPath()).andReturn("/"); + expect(request.getPathInfo()).andReturn("./index.html"); replay(request); FilterChain resolved = resolver.getChain(request, response, chain); @@ -137,7 +139,8 @@ public void testPathTraversalWithDotDot() { expect(request.getAttribute(WebUtils.INCLUDE_CONTEXT_PATH_ATTRIBUTE)).andReturn(null).anyTimes(); expect(request.getContextPath()).andReturn(""); - expect(request.getRequestURI()).andReturn("/public/../index.html"); + expect(request.getServletPath()).andReturn("/public/"); + expect(request.getPathInfo()).andReturn("../index.html"); replay(request); FilterChain resolved = resolver.getChain(request, response, chain); @@ -156,7 +159,8 @@ public void testGetChainsWithoutMatch() { expect(request.getAttribute(WebUtils.INCLUDE_CONTEXT_PATH_ATTRIBUTE)).andReturn(null).anyTimes(); expect(request.getContextPath()).andReturn(""); - expect(request.getRequestURI()).andReturn("/"); + expect(request.getServletPath()).andReturn("/"); + expect(request.getPathInfo()).andReturn(null); replay(request); FilterChain resolved = resolver.getChain(request, response, chain); @@ -178,7 +182,8 @@ public void testGetChain() { expect(request.getAttribute(WebUtils.INCLUDE_CONTEXT_PATH_ATTRIBUTE)).andReturn(null).anyTimes(); expect(request.getContextPath()).andReturn(""); - expect(request.getRequestURI()).andReturn("/resource/book"); + expect(request.getServletPath()).andReturn(""); + expect(request.getPathInfo()).andReturn("/resource/book"); replay(request); FilterChain resolved = resolver.getChain(request, response, chain); @@ -200,7 +205,8 @@ public void testGetChainEqualUrlSeparator() { expect(request.getAttribute(WebUtils.INCLUDE_CONTEXT_PATH_ATTRIBUTE)).andReturn(null).anyTimes(); expect(request.getContextPath()).andReturn(""); - expect(request.getRequestURI()).andReturn("/"); + expect(request.getServletPath()).andReturn("/"); + expect(request.getPathInfo()).andReturn(null); replay(request); FilterChain resolved = resolver.getChain(request, response, chain); @@ -222,7 +228,8 @@ public void testGetChainEndWithUrlSeparator() { expect(request.getAttribute(WebUtils.INCLUDE_CONTEXT_PATH_ATTRIBUTE)).andReturn(null).anyTimes(); expect(request.getContextPath()).andReturn(""); - expect(request.getRequestURI()).andReturn("/resource/book/"); + expect(request.getServletPath()).andReturn(""); + expect(request.getPathInfo()).andReturn("/resource/book"); replay(request); FilterChain resolved = resolver.getChain(request, response, chain); @@ -244,7 +251,8 @@ public void testGetChainEndWithMultiUrlSeparator() { expect(request.getAttribute(WebUtils.INCLUDE_CONTEXT_PATH_ATTRIBUTE)).andReturn(null).anyTimes(); expect(request.getContextPath()).andReturn(""); - expect(request.getRequestURI()).andReturn("/resource/book//"); + expect(request.getServletPath()).andReturn(""); + expect(request.getPathInfo()).andReturn("/resource/book//"); replay(request); FilterChain resolved = resolver.getChain(request, response, chain);