Index: src/main/java/info/magnolia/cms/filters/ServletDispatchingFilter.java =================================================================== --- src/main/java/info/magnolia/cms/filters/ServletDispatchingFilter.java (revision 14418) +++ src/main/java/info/magnolia/cms/filters/ServletDispatchingFilter.java (working copy) @@ -51,6 +51,8 @@ import javax.servlet.ServletConfig; import javax.servlet.ServletContext; import javax.servlet.ServletException; +import javax.servlet.ServletRequest; +import javax.servlet.ServletRequestWrapper; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletRequestWrapper; import javax.servlet.http.HttpServletResponse; @@ -68,6 +70,8 @@ private static final Logger log = LoggerFactory.getLogger(ServletDispatchingFilter.class); + private static final String METACHARACTERS = "([\\^\\(\\)\\{\\}\\[\\]*$+])"; + private String servletName; private String servletClass; @@ -110,6 +114,17 @@ return determineMatchingEnd(uri) < 0 || super.bypasses(request); } + protected Matcher findMatcher(String uri) { + for (Iterator iter = mappings.iterator(); iter.hasNext();) { + final Matcher matcher = ((Pattern) iter.next()).matcher(uri); + + if (matcher.find()) { + return matcher; + } + } + + return null; + } /** * Determines the index of the first pathInfo character. If the uri does not match any mapping this method returns * -1. @@ -138,16 +153,9 @@ throws IOException, ServletException { log.debug("Dispatching to servlet " + getServletClass()); - servlet.service(new HttpServletRequestWrapper(request) { - - public String getPathInfo() { - final String uri = StringUtils.substringAfter(request.getRequestURI(), request.getContextPath()); - final String pathInfo = StringUtils.substring(uri, determineMatchingEnd(uri)); - - // according to the servlet spec the pathInfo should contain a leading slash - return (pathInfo.startsWith("/") ? pathInfo : "/" + pathInfo); - } - }, response); + servlet.service(new WrappedRequest(request, findMatcher( + StringUtils.substringAfter(request.getRequestURI(), request.getContextPath()))), + response); } public String getServletName() { @@ -175,21 +183,39 @@ } public void addMapping(String mapping) { - if (isPathMapping(mapping)) { - mapping = "^" + StringUtils.removeEnd(mapping, "*"); + if (isDefaultMapping(mapping)) { + // the mapping is exactly '/*', the servlet path should be + // an empty string and everything else should be the path info + mapping = "^()(/"+ SimpleUrlPattern.MULTIPLE_CHAR_PATTERN + ")"; + } else if (isPathMapping(mapping)) { + // the pattern ends with /*, escape out metacharacters for + // use in a regex, and replace the ending * with MULTIPLE_CHAR_PATTERN + mapping = "^(" + StringUtils.removeEnd(mapping, "/*") + .replaceAll(METACHARACTERS, "\\\\$1") + ")(/" + + SimpleUrlPattern.MULTIPLE_CHAR_PATTERN + ")"; } else if (isExtensionMapping(mapping)) { - // TODO - throw new UnsupportedOperationException("Extension mappings are currently not supported"); - } else if (isDefaultMapping(mapping)) { - mapping = "^" + mapping + "*"; + // something like '*.jsp', everything should be the servlet path + // and the path info should be null + mapping = "^(" + StringUtils.replace(mapping, "*.", + SimpleUrlPattern.MULTIPLE_CHAR_PATTERN + "\\.") + ")$"; + } else if (isRegexpMapping(mapping)) { + mapping = "^(" + mapping.replaceFirst("regex:", "") + ")(" + + SimpleUrlPattern.MULTIPLE_CHAR_PATTERN + ")"; } else { - mapping = "^" + mapping + "$"; + // just literal text, ensure metacharacters are escaped, and that only + // the exact string is matched. + mapping = "^(" + mapping.replaceAll(METACHARACTERS, "\\\\$1") + ")$"; } - final String encodedString = SimpleUrlPattern.getEncodedString(mapping); + log.info("Adding new mapping for " + mapping); - mappings.add(Pattern.compile(encodedString)); + mappings.add(Pattern.compile(mapping)); } + /** + * This is order specific, this method should not be called until + * after the isDefaultMapping() method else it will return true + * for a default mapping. + */ private boolean isPathMapping(String mapping) { return mapping.startsWith("/") && mapping.endsWith("/*"); } @@ -199,9 +225,13 @@ } private boolean isDefaultMapping(String mapping) { - return mapping.equals("/"); + return mapping.equals("/*"); } + private boolean isRegexpMapping(String mapping) { + return mapping.startsWith("regex:"); + } + public Map getParameters() { return parameters; } @@ -260,4 +290,50 @@ } } + + private class WrappedRequest extends HttpServletRequestWrapper { + + private Matcher matcher; + + /** + * This is set to true when the original request object passed + * in is changed by the setRequest() method. This can indicate + * that a forward occurred and the values pulled from the + * matcher should no longer be used. + */ + private boolean requestReplaced = false; + + public WrappedRequest(HttpServletRequest request, Matcher matcher) { + super(request); + this.matcher = matcher; + } + + public String getPathInfo() { + if (requestReplaced) { + return super.getPathInfo(); + } + if (matcher.groupCount() > 1) { + String pathInfo = matcher.group(2); + if (pathInfo.equals("")) { + return null; + } + // according to the servlet spec the pathInfo should contain a leading slash + return (pathInfo.startsWith("/") ? pathInfo : "/" + pathInfo); + } + return null; + } + + public String getServletPath() { + if (requestReplaced) { + return super.getServletPath(); + } + return matcher.group(1); + } + + public void setRequest(ServletRequest request) { + requestReplaced = true; + super.setRequest(request); + } + + } } Index: src/main/java/info/magnolia/cms/util/SimpleUrlPattern.java =================================================================== --- src/main/java/info/magnolia/cms/util/SimpleUrlPattern.java (revision 14418) +++ src/main/java/info/magnolia/cms/util/SimpleUrlPattern.java (working copy) @@ -53,15 +53,19 @@ */ private static final long serialVersionUID = 222L; + public static final String URL_CHAR_PATTERN = "[\\p{L}\\w!\"#$%&'*+,-./:; <=>?@`|~\\^\\(\\)\\{\\}\\[\\]]"; //$NON-NLS-1$ + /** * Regexp pattern used for the simple keyword * */ - private static final String MULTIPLE_CHAR_PATTERN = "[\\p{L}[a-z[A-Z[0-9[!\"#$%&'()*+,-./:; <=>?@\\^_`{|}~\\[\\]]]]]]*"; //$NON-NLS-1$ +// public static final String MULTIPLE_CHAR_PATTERN = "[\\p{L}[a-z[A-Z[0-9[!\"#$%&'()*+,-./:; <=>?@\\^_`{|}~\\[\\]]]]]]*"; //$NON-NLS-1$ + public static final String MULTIPLE_CHAR_PATTERN = URL_CHAR_PATTERN + "*"; //$NON-NLS-1$ /** * Regexp pattern used for the simple keyword ? */ - private static final String SINGLE_CHAR_PATTERN = "[\\p{L}[a-z[A-Z[0-9[!\"#$%&'()*+,-./:; <=>?@\\^_`{|}~\\[\\]]]]]]?"; //$NON-NLS-1$ +// public static final String SINGLE_CHAR_PATTERN = "[\\p{L}[a-z[A-Z[0-9[!\"#$%&'()*+,-./:; <=>?@\\^_`{|}~\\[\\]]]]]]?"; //$NON-NLS-1$ + public static final String SINGLE_CHAR_PATTERN = URL_CHAR_PATTERN + "?"; //$NON-NLS-1$ /** * Regexp pattern used in match(). @@ -102,16 +106,16 @@ while (i < chars.length) { char c = chars[i]; if (c == '*') { - stringBuffer.append('('); +// stringBuffer.append('('); stringBuffer.append(chars, last, i - last); - stringBuffer.append(')'); +// stringBuffer.append(')'); stringBuffer.append(MULTIPLE_CHAR_PATTERN); last = i + 1; } else if (c == '?') { - stringBuffer.append('('); +// stringBuffer.append('('); stringBuffer.append(chars, last, i - last); - stringBuffer.append(')'); +// stringBuffer.append(')'); stringBuffer.append(SINGLE_CHAR_PATTERN); last = i + 1; } Index: src/test/java/info/magnolia/cms/filters/ServletDispatchingFilterTest.java =================================================================== --- src/test/java/info/magnolia/cms/filters/ServletDispatchingFilterTest.java (revision 14418) +++ src/test/java/info/magnolia/cms/filters/ServletDispatchingFilterTest.java (working copy) @@ -54,35 +54,44 @@ */ public class ServletDispatchingFilterTest extends TestCase { public void testShouldNotBypassWhenDefaultMappingMatches() throws Exception { - doTestBypassAndPathInfo(false, null, "/dms/some-doc.pdf", "/"); + doTestBypassAndPathInfo(false, "/dms/some-doc.pdf", "", "/dms/some-doc.pdf", "/*"); } public void testShouldNotBypassWhenPathMappingMatches() throws Exception { - doTestBypassAndPathInfo(false, "/some-doc.pdf", "/dms/some-doc.pdf", "/dms/*"); - doTestBypassAndPathInfo(false, "/.html", "/nonSpecConformantMapping.html", "/nonSpecConformantMapping*"); + doTestBypassAndPathInfo(false, null, "/dms/some-doc.pdf", "/dms/some-doc.pdf", "*.pdf"); + doTestBypassAndPathInfo(false, "/", "/test", "/test/", "/test/*"); + doTestBypassAndPathInfo(false, "/some-doc.pdf", "/$d}^(m)+{s*", "/$d}^(m)+{s*/some-doc.pdf", "/$d}^(m)+{s*/*"); } public void testShouldNotBypassWhenExactMappingMatches() throws Exception { - doTestBypassAndPathInfo(false, null, "/exactMatch", "/exactMatch"); + doTestBypassAndPathInfo(false, null, null, "/exactMatch", "/exactMatch"); + doTestBypassAndPathInfo(false, null, "/somepath*", "/somepath*", "/somepath*"); } public void testShouldBypassWhenMappingDoesNotMatch() throws Exception { - doTestBypassAndPathInfo(true, null, "/bleh/foo.bar", "/dms/*"); - doTestBypassAndPathInfo(true, null, "/exactMatch/rightPathInfo", "/exactMatch"); + doTestBypassAndPathInfo(true, null, null, "/bleh/foo.bar", "/dms/*"); + doTestBypassAndPathInfo(true, null, null, "/exactMatch/rightPathInfo", "/exactMatch"); + doTestBypassAndPathInfo(true, null, null, "/nonSpecConformantMapping.html", "/nonSpecConformantMapping*"); + doTestBypassAndPathInfo(true, null, null, "/nonSpecConformantMapping*/somePage.html", "/nonSpecConformantMapping*"); } - + public void testShouldBypassWhenMappingDoesNotMatchMAGNOLIA1984() throws Exception { - doTestBypassAndPathInfo(true, null, "/modules/dms/managingdocs.html", "/dms/*"); + doTestBypassAndPathInfo(true, null, null, "/modules/dms/managingdocs.html", "/dms/*"); } - /* - public void testPathInfoShouldStateWhateverIsAfterTheMapping() throws Exception { - // extension mappings not supported - doTestBypassAndPathInfo(false, "/test", "/some-doc.pdf/test", "*.pdf"); + public void testPathInfoShouldAdhereToServletSpec() throws Exception { + doTestBypassAndPathInfo(false, "/pathInfo.html", "/servlet", "/servlet/pathInfo.html", "/servlet/*"); } - */ - private void doTestBypassAndPathInfo(final boolean expectedBypass, final String expectedPathInfo, final String requestPath, String mapping) throws Exception { + public void testPathInfoShouldStateWhateverIsAfterTheRegexMapping() throws Exception { + doTestBypassAndPathInfo(false, "/test", "/some-doc.pdf", "/some-doc.pdf/test", "regex:.*\\.pdf"); + } + + public void testShouldStatePathInfoCorrectly() throws Exception { + + } + + private void doTestBypassAndPathInfo(final boolean expectedBypass, final String expectedPathInfo, final String expectedServletPath, final String requestPath, String mapping) throws Exception { FactoryUtil.setInstance(Voting.class, new DefaultVoting()); final FilterChain chain = createNiceMock(FilterChain.class); @@ -94,12 +103,14 @@ final Servlet servlet = createStrictMock(Servlet.class); servlet.service(isA(HttpServletRequestWrapper.class), same(res)); - if (expectedPathInfo != null) { + if (expectedPathInfo != null || expectedServletPath != null) { expectLastCall().andAnswer(new IAnswer() { public Object answer() throws Throwable { final HttpServletRequestWrapper requestWrapper = (HttpServletRequestWrapper) getCurrentArguments()[0]; final String pathInfo = requestWrapper.getPathInfo(); + final String servletPath = requestWrapper.getServletPath(); assertEquals(expectedPathInfo, pathInfo); + assertEquals(expectedServletPath, servletPath); return null; } });