From a273b5f45cb46a273d06510a689fc314155a952d Mon Sep 17 00:00:00 2001 From: Mark Thomas Date: Thu, 19 Nov 2015 16:42:28 +0000 Subject: [PATCH] Move the functionality that provides redirects for context roots and directories where a trailing / is added from the Mapper to the DefaultServlet. This enables such requests to be processed by any configured Valves and Filters before the redirect is made. This behaviour is configurable via the mapperContextRootRedirectEnabled and mapperDirectoryRedirectEnabled attributes of the Context which may be used to restore the previous behaviour. git-svn-id: https://svn.apache.org/repos/asf/tomcat/tc7.0.x/trunk@1715213 13f79535-47bb-0310-9956-ffa450edef68 --- java/org/apache/catalina/Context.java | 40 ++++++++++++ .../apache/catalina/core/StandardContext.java | 44 ++++++++++++- .../catalina/core/mbeans-descriptors.xml | 8 +++ .../catalina/servlets/DefaultServlet.java | 11 ++++ .../catalina/startup/FailedContext.java | 18 +++++- .../tomcat/util/http/mapper/Mapper.java | 42 ++++++++++-- .../apache/catalina/core/TesterContext.java | 16 +++++ .../catalina/startup/TomcatBaseTest.java | 3 +- .../util/http/mapper/TestMapperWebapps.java | 64 +++++++++++++++++++ webapps/docs/changelog.xml | 10 +++ webapps/docs/config/context.xml | 16 +++++ 11 files changed, 262 insertions(+), 10 deletions(-) diff --git a/java/org/apache/catalina/Context.java b/java/org/apache/catalina/Context.java index c6fda8d6c311..5a16ce352183 100644 --- a/java/org/apache/catalina/Context.java +++ b/java/org/apache/catalina/Context.java @@ -1642,4 +1642,44 @@ Set addServletSecurity(ApplicationServletRegistration registration, * false} */ public boolean getValidateClientProvidedNewSessionId(); + + /** + * If enabled, requests for a web application context root will be + * redirected (adding a trailing slash) by the Mapper. This is more + * efficient but has the side effect of confirming that the context path is + * valid. + * + * @param mapperContextRootRedirectEnabled Should the redirects be enabled? + */ + public void setMapperContextRootRedirectEnabled(boolean mapperContextRootRedirectEnabled); + + /** + * Determines if requests for a web application context root will be + * redirected (adding a trailing slash) by the Mapper. This is more + * efficient but has the side effect of confirming that the context path is + * valid. + * + * @return {@code true} if the Mapper level redirect is enabled for this + * Context. + */ + public boolean getMapperContextRootRedirectEnabled(); + + /** + * If enabled, requests for a directory will be redirected (adding a + * trailing slash) by the Mapper. This is more efficient but has the + * side effect of confirming that the directory is valid. + * + * @param mapperDirectoryRedirectEnabled Should the redirects be enabled? + */ + public void setMapperDirectoryRedirectEnabled(boolean mapperDirectoryRedirectEnabled); + + /** + * Determines if requests for a directory will be redirected (adding a + * trailing slash) by the Mapper. This is more efficient but has the + * side effect of confirming that the directory is valid. + * + * @return {@code true} if the Mapper level redirect is enabled for this + * Context. + */ + public boolean getMapperDirectoryRedirectEnabled(); } diff --git a/java/org/apache/catalina/core/StandardContext.java b/java/org/apache/catalina/core/StandardContext.java index ab78c1637144..541fdc095750 100644 --- a/java/org/apache/catalina/core/StandardContext.java +++ b/java/org/apache/catalina/core/StandardContext.java @@ -911,15 +911,53 @@ public StandardContext() { private boolean validateClientProvidedNewSessionId = true; - + boolean mapperContextRootRedirectEnabled = false; + + boolean mapperDirectoryRedirectEnabled = false; + + // ----------------------------------------------------- Context Properties + @Override + public void setMapperContextRootRedirectEnabled(boolean mapperContextRootRedirectEnabled) { + this.mapperContextRootRedirectEnabled = mapperContextRootRedirectEnabled; + } + + + /** + * {@inheritDoc} + *

+ * The default value for this implementation is {@code false}. + */ + @Override + public boolean getMapperContextRootRedirectEnabled() { + return mapperContextRootRedirectEnabled; + } + + + @Override + public void setMapperDirectoryRedirectEnabled(boolean mapperDirectoryRedirectEnabled) { + this.mapperDirectoryRedirectEnabled = mapperDirectoryRedirectEnabled; + } + + + /** + * {@inheritDoc} + *

+ * The default value for this implementation is {@code false}. + */ + @Override + public boolean getMapperDirectoryRedirectEnabled() { + return mapperDirectoryRedirectEnabled; + } + + @Override public void setValidateClientProvidedNewSessionId(boolean validateClientProvidedNewSessionId) { this.validateClientProvidedNewSessionId = validateClientProvidedNewSessionId; } - + /** * {@inheritDoc} *

@@ -930,7 +968,7 @@ public boolean getValidateClientProvidedNewSessionId() { return validateClientProvidedNewSessionId; } - + @Override public void setContainerSciFilter(String containerSciFilter) { this.containerSciFilter = containerSciFilter; diff --git a/java/org/apache/catalina/core/mbeans-descriptors.xml b/java/org/apache/catalina/core/mbeans-descriptors.xml index b4a58457c3ac..7f3b776f2b0e 100644 --- a/java/org/apache/catalina/core/mbeans-descriptors.xml +++ b/java/org/apache/catalina/core/mbeans-descriptors.xml @@ -221,6 +221,14 @@ description="The object used for mapping" type="java.lang.Object"/> + + + + diff --git a/java/org/apache/catalina/servlets/DefaultServlet.java b/java/org/apache/catalina/servlets/DefaultServlet.java index 3545f816a795..782160034791 100644 --- a/java/org/apache/catalina/servlets/DefaultServlet.java +++ b/java/org/apache/catalina/servlets/DefaultServlet.java @@ -862,6 +862,17 @@ protected void serveResource(HttpServletRequest request, if (cacheEntry.context != null) { + if (!path.endsWith("/")) { + StringBuilder location = new StringBuilder(request.getRequestURI()); + location.append('/'); + if (request.getQueryString() != null) { + location.append('?'); + location.append(request.getQueryString()); + } + response.sendRedirect(response.encodeRedirectURL(location.toString())); + return; + } + // Skip directory listings if we have been configured to // suppress them if (!listings) { diff --git a/java/org/apache/catalina/startup/FailedContext.java b/java/org/apache/catalina/startup/FailedContext.java index 1b32c3be215e..acb4ad153d9f 100644 --- a/java/org/apache/catalina/startup/FailedContext.java +++ b/java/org/apache/catalina/startup/FailedContext.java @@ -706,9 +706,25 @@ public void setContainerSciFilter(String containerSciFilter) { /* NO-OP */ } @Override public void setValidateClientProvidedNewSessionId(boolean validateClientProvidedNewSessionId) { - //NO-OP + // NO-OP } @Override public boolean getValidateClientProvidedNewSessionId() { return false; } + + @Override + public void setMapperContextRootRedirectEnabled(boolean mapperContextRootRedirectEnabled) { + // NO-OP + } + + @Override + public boolean getMapperContextRootRedirectEnabled() { return false; } + + @Override + public void setMapperDirectoryRedirectEnabled(boolean mapperDirectoryRedirectEnabled) { + // NO-OP + } + + @Override + public boolean getMapperDirectoryRedirectEnabled() { return false; } } \ No newline at end of file diff --git a/java/org/apache/tomcat/util/http/mapper/Mapper.java b/java/org/apache/tomcat/util/http/mapper/Mapper.java index ef6e0b16fdfe..90ca3a7f1788 100644 --- a/java/org/apache/tomcat/util/http/mapper/Mapper.java +++ b/java/org/apache/tomcat/util/http/mapper/Mapper.java @@ -248,7 +248,8 @@ public void setContext(String path, String[] welcomeResources, * @param context Context object * @param welcomeResources Welcome files defined for this context * @param resources Static resources of the context - * @deprecated Use {@link #addContextVersion(String, Object, String, String, Object, String[], javax.naming.Context, Collection)} + * @deprecated Use {@link #addContextVersion(String, Object, String, String, Object, String[], + * javax.naming.Context, Collection, boolean, boolean)} */ @Deprecated public void addContextVersion(String hostName, Object host, String path, @@ -258,6 +259,7 @@ public void addContextVersion(String hostName, Object host, String path, welcomeResources, resources, null); } + /** * Add a new Context to an existing Host. * @@ -269,10 +271,36 @@ public void addContextVersion(String hostName, Object host, String path, * @param welcomeResources Welcome files defined for this context * @param resources Static resources of the context * @param wrappers Information on wrapper mappings + * @deprecated Use {@link #addContextVersion(String, Object, String, String, Object, String[], + * javax.naming.Context, Collection, boolean, boolean)} */ + @Deprecated public void addContextVersion(String hostName, Object host, String path, String version, Object context, String[] welcomeResources, javax.naming.Context resources, Collection wrappers) { + addContextVersion(hostName, host, path, version, context, welcomeResources, resources, + wrappers, false, false); + } + + + /** + * Add a new Context to an existing Host. + * + * @param hostName Virtual host name this context belongs to + * @param host Host object + * @param path Context path + * @param version Context version + * @param context Context object + * @param welcomeResources Welcome files defined for this context + * @param resources Static resources of the context + * @param wrappers Information on wrapper mappings + * @param mapperContextRootRedirectEnabled Mapper does context root redirects + * @param mapperDirectoryRedirectEnabled Mapper does directory redirects + */ + public void addContextVersion(String hostName, Object host, String path, + String version, Object context, String[] welcomeResources, + javax.naming.Context resources, Collection wrappers, + boolean mapperContextRootRedirectEnabled, boolean mapperDirectoryRedirectEnabled) { Host mappedHost = exactFind(hosts, hostName); if (mappedHost == null) { @@ -294,6 +322,9 @@ public void addContextVersion(String hostName, Object host, String path, newContextVersion.slashCount = slashCount; newContextVersion.welcomeResources = welcomeResources; newContextVersion.resources = resources; + newContextVersion.mapperContextRootRedirectEnabled = mapperContextRootRedirectEnabled; + newContextVersion.mapperDirectoryRedirectEnabled = mapperDirectoryRedirectEnabled; + if (wrappers != null) { addWrappers(newContextVersion, wrappers); } @@ -904,7 +935,8 @@ private final void internalMapWrapper(ContextVersion contextVersion, } } - if(mappingData.wrapper == null && noServletPath) { + if(mappingData.wrapper == null && noServletPath && + contextVersion.mapperContextRootRedirectEnabled) { // The path is empty, redirect to "/" mappingData.redirectPath.setChars (path.getBuffer(), pathOffset, pathEnd-pathOffset); @@ -1032,7 +1064,8 @@ private final void internalMapWrapper(ContextVersion contextVersion, } catch(NamingException nex) { // Swallow, since someone else handles the 404 } - if (file != null && file instanceof DirContext) { + if (file != null && file instanceof DirContext && + contextVersion.mapperDirectoryRedirectEnabled) { // Note: this mutates the path: do not do any processing // after this (since we set the redirectPath, there // shouldn't be any) @@ -1049,7 +1082,6 @@ private final void internalMapWrapper(ContextVersion contextVersion, path.setOffset(pathOffset); path.setEnd(pathEnd); - } @@ -1684,6 +1716,8 @@ protected static final class ContextVersion extends MapElement { public Wrapper[] wildcardWrappers = new Wrapper[0]; public Wrapper[] extensionWrappers = new Wrapper[0]; public int nesting = 0; + public boolean mapperContextRootRedirectEnabled = false; + public boolean mapperDirectoryRedirectEnabled = false; private volatile boolean paused; public ContextVersion() { diff --git a/test/org/apache/catalina/core/TesterContext.java b/test/org/apache/catalina/core/TesterContext.java index 0898411e015d..950df58aa550 100644 --- a/test/org/apache/catalina/core/TesterContext.java +++ b/test/org/apache/catalina/core/TesterContext.java @@ -1227,4 +1227,20 @@ public void setValidateClientProvidedNewSessionId(boolean validateClientProvided @Override public boolean getValidateClientProvidedNewSessionId() { return false; } + + @Override + public void setMapperContextRootRedirectEnabled(boolean mapperContextRootRedirectEnabled) { + // NO-OP + } + + @Override + public boolean getMapperContextRootRedirectEnabled() { return false; } + + @Override + public void setMapperDirectoryRedirectEnabled(boolean mapperDirectoryRedirectEnabled) { + // NO-OP + } + + @Override + public boolean getMapperDirectoryRedirectEnabled() { return false; } } diff --git a/test/org/apache/catalina/startup/TomcatBaseTest.java b/test/org/apache/catalina/startup/TomcatBaseTest.java index 2da2e19b326e..4e8c24eefd67 100644 --- a/test/org/apache/catalina/startup/TomcatBaseTest.java +++ b/test/org/apache/catalina/startup/TomcatBaseTest.java @@ -631,8 +631,7 @@ public static int methodUrl(String path, ByteChunk out, int readTimeout, String method) throws IOException { URL url = new URL(path); - HttpURLConnection connection = - (HttpURLConnection) url.openConnection(); + HttpURLConnection connection = (HttpURLConnection) url.openConnection(); connection.setUseCaches(false); connection.setReadTimeout(readTimeout); connection.setRequestMethod(method); diff --git a/test/org/apache/tomcat/util/http/mapper/TestMapperWebapps.java b/test/org/apache/tomcat/util/http/mapper/TestMapperWebapps.java index 2061d1b92f72..e0749c7bcf63 100644 --- a/test/org/apache/tomcat/util/http/mapper/TestMapperWebapps.java +++ b/test/org/apache/tomcat/util/http/mapper/TestMapperWebapps.java @@ -18,6 +18,7 @@ import java.io.File; import java.io.IOException; +import java.net.HttpURLConnection; import java.util.HashMap; import java.util.List; @@ -31,8 +32,11 @@ import org.apache.catalina.Context; import org.apache.catalina.core.StandardContext; +import org.apache.catalina.deploy.SecurityCollection; +import org.apache.catalina.deploy.SecurityConstraint; import org.apache.catalina.startup.Tomcat; import org.apache.catalina.startup.TomcatBaseTest; +import org.apache.catalina.valves.RemoteAddrValve; import org.apache.tomcat.util.buf.ByteChunk; /** @@ -223,6 +227,66 @@ public void testWelcomeFileStrict() throws Exception { Assert.assertEquals(HttpServletResponse.SC_NOT_FOUND, rc); } + @Test + public void testRedirect() throws Exception { + // Disable the following of redirects for this test only + boolean originalValue = HttpURLConnection.getFollowRedirects(); + HttpURLConnection.setFollowRedirects(false); + try { + Tomcat tomcat = getTomcatInstance(); + + // Use standard test webapp as ROOT + File rootDir = new File("test/webapp-3.0"); + org.apache.catalina.Context root = + tomcat.addWebapp(null, "", rootDir.getAbsolutePath()); + + // Add a security constraint + SecurityConstraint constraint = new SecurityConstraint(); + SecurityCollection collection = new SecurityCollection(); + collection.addPattern("/welcome-files/*"); + collection.addPattern("/welcome-files"); + constraint.addCollection(collection); + constraint.addAuthRole("foo"); + root.addConstraint(constraint); + + // Also make examples available + File examplesDir = new File(getBuildDirectory(), "webapps/examples"); + org.apache.catalina.Context examples = tomcat.addWebapp( + null, "/examples", examplesDir.getAbsolutePath()); + // Then block access to the examples to test redirection + RemoteAddrValve rav = new RemoteAddrValve(); + rav.setDeny(".*"); + rav.setDenyStatus(404); + examples.getPipeline().addValve(rav); + + tomcat.start(); + + // Redirects within a web application + doRedirectTest("/welcome-files", 401); + doRedirectTest("/welcome-files/", 401); + + doRedirectTest("/jsp", 302); + doRedirectTest("/jsp/", 404); + + doRedirectTest("/WEB-INF", 404); + doRedirectTest("/WEB-INF/", 404); + + // Redirects between web applications + doRedirectTest("/examples", 404); + doRedirectTest("/examples/", 404); + } finally { + HttpURLConnection.setFollowRedirects(originalValue); + } + } + + + private void doRedirectTest(String path, int expected) throws IOException { + ByteChunk bc = new ByteChunk(); + int rc = getUrl("http://localhost:" + getPort() + path, bc, null); + Assert.assertEquals(expected, rc); + } + + /** * Prepare a string to search in messages that contain a timestamp, when it * is known that the timestamp was printed between {@code timeA} and diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index ae7581e3cf7c..d0c5bc41527c 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -171,6 +171,16 @@ Ensure that in an embedded Tomcat the logging configuration is not lost during garbage collection. (violetagg) + + Move the functionality that provides redirects for context roots and + directories where a trailing / is added from the Mapper to + the DefaultServlet. This enables such requests to be + processed by any configured Valves and Filters before the redirect is + made. This behaviour is configurable via the + mapperContextRootRedirectEnabled and + mapperDirectoryRedirectEnabled attributes of the Context + which may be used to restore the previous behaviour. (markt) + diff --git a/webapps/docs/config/context.xml b/webapps/docs/config/context.xml index 3996b613b245..60094fad780c 100644 --- a/webapps/docs/config/context.xml +++ b/webapps/docs/config/context.xml @@ -360,6 +360,22 @@ default value of false is used.

+ +

If enabled, requests for a web application context root will be + redirected (adding a trailing slash) if necessary by the Mapper rather + than the default Servlet. This is more efficient but has the side effect + of confirming that the context path exists. If not specified, the + default value of false is used.

+ + + +

If enabled, requests for a web application directory will be + redirected (adding a trailing slash) if necessary by the Mapper rather + than the default Servlet. This is more efficient but has the side effect + of confirming that the directory is exists. If not specified, the + default value of false is used.

+ +

Set to true to ignore any settings in both the global or Host default contexts. By default, settings