Updated Branches: refs/heads/master 4b881a77f -> aaf48b285
WICKET-4626 WicketFilter unify the filterPath Project: http://git-wip-us.apache.org/repos/asf/wicket/repo Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/aaf48b28 Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/aaf48b28 Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/aaf48b28 Branch: refs/heads/master Commit: aaf48b28586824277854b9e472c1f33e07f83676 Parents: 4b881a7 Author: Martin Tzvetanov Grigorov <mgrigo...@apache.org> Authored: Wed Aug 22 12:17:04 2012 +0300 Committer: Martin Tzvetanov Grigorov <mgrigo...@apache.org> Committed: Wed Aug 22 12:17:04 2012 +0300 ---------------------------------------------------------------------- .../apache/wicket/protocol/http/WicketFilter.java | 126 +++++++++++--- .../wicket/protocol/http/WicketFilterTest.java | 58 +++++++ 2 files changed, 157 insertions(+), 27 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/wicket/blob/aaf48b28/wicket-core/src/main/java/org/apache/wicket/protocol/http/WicketFilter.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/main/java/org/apache/wicket/protocol/http/WicketFilter.java b/wicket-core/src/main/java/org/apache/wicket/protocol/http/WicketFilter.java index 47cd246..c8f2750 100644 --- a/wicket-core/src/main/java/org/apache/wicket/protocol/http/WicketFilter.java +++ b/wicket-core/src/main/java/org/apache/wicket/protocol/http/WicketFilter.java @@ -343,22 +343,29 @@ public class WicketFilter implements Filter application.setWicketFilter(this); // Allow the filterPath to be preset via setFilterPath() - if (filterPath == null) + String configureFilterPath = getFilterPath(); + + if (configureFilterPath == null) + { + configureFilterPath = getFilterPathFromConfig(filterConfig); + } + + if (configureFilterPath == null) { - filterPath = getFilterPathFromConfig(filterConfig); + configureFilterPath = getFilterPathFromWebXml(isServlet, filterConfig); } - if (filterPath == null) + if (configureFilterPath == null) { - filterPath = getFilterPathFromWebXml(isServlet, filterConfig); + configureFilterPath = getFilterPathFromAnnotation(isServlet); } - if (filterPath == null) + if (configureFilterPath != null) { - filterPath = getFilterPathFromAnnotation(isServlet); + setFilterPath(configureFilterPath); } - if (filterPath == null) + if (getFilterPath() == null) { log.warn("Unable to determine filter path from filter init-parm, web.xml, " + "or servlet 3.0 annotations. Assuming user will set filter path " @@ -471,6 +478,15 @@ public class WicketFilter implements Filter } /** + * Provide a standard getter for filterPath. + * @return The configured filterPath. + */ + protected String getFilterPath() + { + return filterPath; + } + + /** * * @param filterConfig * @return filter path @@ -482,7 +498,7 @@ public class WicketFilter implements Filter { if (result.equals("/*")) { - filterPath = ""; + result = ""; } else if (!result.startsWith("/") || !result.endsWith("/*")) { @@ -492,10 +508,10 @@ public class WicketFilter implements Filter else { // remove leading "/" and trailing "*" - filterPath = result.substring(1, result.length() - 1); + result = result.substring(1, result.length() - 1); } } - return filterPath; + return result; } /** @@ -551,19 +567,6 @@ public class WicketFilter implements Filter uriLength = requestURI.length(); } - // We only need to determine it once. It'll not change. - if (filterPathLength == -1) - { - if (filterPath.endsWith("/")) - { - filterPathLength = filterPath.length() - 1; - } - else - { - filterPathLength = filterPath.length(); - } - } - // request.getContextPath() + "/" + filterPath. But without any trailing "/". int homePathLength = contextPath.length() + (filterPathLength > 0 ? 1 + filterPathLength : 0); @@ -578,7 +581,7 @@ public class WicketFilter implements Filter String uri = Strings.stripJSessionId(requestURI); // home page without trailing slash URI - String homePageUri = contextPath + "/" + filterPath; + String homePageUri = contextPath + '/' + getFilterPath(); if (homePageUri.endsWith("/")) { homePageUri = homePageUri.substring(0, homePageUri.length() - 1); @@ -603,14 +606,27 @@ public class WicketFilter implements Filter * * @param filterPath */ - public final void setFilterPath(final String filterPath) + public final void setFilterPath(String filterPath) { // see https://issues.apache.org/jira/browse/WICKET-701 if (this.filterPath != null) { throw new IllegalStateException( - "Filter path is write-once. You can not change it. Current value='" + filterPath + - "'"); + "Filter path is write-once. You can not change it. Current value='" + filterPath + '\''); + } + if (filterPath != null) + { + filterPath = canonicaliseFilterPath(filterPath); + + // We only need to determine it once. It'll not change. + if (filterPath.endsWith("/")) + { + filterPathLength = filterPath.length() - 1; + } + else + { + filterPathLength = filterPath.length(); + } } this.filterPath = filterPath; } @@ -642,6 +658,7 @@ public class WicketFilter implements Filter // We should always be under the rootPath, except // for the special case of someone landing on the // home page without a trailing slash. + String filterPath = getFilterPath(); if (!path.startsWith(filterPath)) { if (filterPath.equals(path + "/")) @@ -714,4 +731,59 @@ public class WicketFilter implements Filter } } } + + /** + * A filterPath should have all leading slashes removed and exactly one trailing slash. A + * wildcard asterisk character has no special meaning. If your intention is to mean the top + * level "/" then an empty string should be used instead. + * + * @param filterPath + * @return + */ + static String canonicaliseFilterPath(String filterPath) + { + if (Strings.isEmpty(filterPath)) + { + return filterPath; + } + + int beginIndex = 0; + int endIndex = filterPath.length(); + while (beginIndex < endIndex) + { + char c = filterPath.charAt(beginIndex); + if (c != '/') + { + break; + } + beginIndex++; + } + int o; + int i = o = beginIndex; + while (i < endIndex) + { + char c = filterPath.charAt(i); + i++; + if (c != '/') + { + o = i; + } + } + if (o < endIndex) + { + o++; // include exactly one trailing slash + filterPath = filterPath.substring(beginIndex, o); + } + else + { + // ensure to append trailing slash + filterPath = filterPath.substring(beginIndex) + '/'; + } + + if (filterPath.equals("/")) + { + return ""; + } + return filterPath; + } } http://git-wip-us.apache.org/repos/asf/wicket/blob/aaf48b28/wicket-core/src/test/java/org/apache/wicket/protocol/http/WicketFilterTest.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/test/java/org/apache/wicket/protocol/http/WicketFilterTest.java b/wicket-core/src/test/java/org/apache/wicket/protocol/http/WicketFilterTest.java index 3619254..3cd3c0d 100644 --- a/wicket-core/src/test/java/org/apache/wicket/protocol/http/WicketFilterTest.java +++ b/wicket-core/src/test/java/org/apache/wicket/protocol/http/WicketFilterTest.java @@ -404,4 +404,62 @@ public class WicketFilterTest extends Assert // the request is processed so the chain is not executed verify(chain, Mockito.times(3)).doFilter(request, response); } + + /** + * <a href="https://issues.apache.org/jira/browse/WICKET-4626">WICKET-4626</a> + * <p> + * Test method WicketFilter#canonicaliseFilterPath(String) + * </p> + */ + @Test + public void canonicaliseFilterPath() + { + String s; + + s = WicketFilter.canonicaliseFilterPath(""); + assertEquals("", s); + + s = WicketFilter.canonicaliseFilterPath("/"); + assertEquals("", s); + + s = WicketFilter.canonicaliseFilterPath("//"); + assertEquals("", s); + + s = WicketFilter.canonicaliseFilterPath("///"); + assertEquals("", s); + + s = WicketFilter.canonicaliseFilterPath("/wicket"); + assertEquals("wicket/", s); + + s = WicketFilter.canonicaliseFilterPath("/wicket/"); + assertEquals("wicket/", s); + + s = WicketFilter.canonicaliseFilterPath("wicket/"); + assertEquals("wicket/", s); + + s = WicketFilter.canonicaliseFilterPath("wicket"); + assertEquals("wicket/", s); + + s = WicketFilter.canonicaliseFilterPath("///wicket"); + assertEquals("wicket/", s); + + s = WicketFilter.canonicaliseFilterPath("///wicket///"); + assertEquals("wicket/", s); + + s = WicketFilter.canonicaliseFilterPath("wicket///"); + assertEquals("wicket/", s); + + s = WicketFilter.canonicaliseFilterPath("/wicket/foobar"); + assertEquals("wicket/foobar/", s); + + s = WicketFilter.canonicaliseFilterPath("/wicket/foobar/"); + assertEquals("wicket/foobar/", s); + + s = WicketFilter.canonicaliseFilterPath("wicket/foobar/"); + assertEquals("wicket/foobar/", s); + + s = WicketFilter.canonicaliseFilterPath("/wicket///foobar/"); + assertEquals("wicket///foobar/", s); // ok we're not perfect! + } + }