Author: markt Date: Fri Dec 12 16:22:05 2014 New Revision: 1644962 URL: http://svn.apache.org/r1644962 Log: Fix a collection of RewriteVlave issues along with some refactoring and clean-up to support those fixes.
Added: tomcat/tc8.0.x/trunk/test/org/apache/catalina/valves/rewrite/ - copied from r1644730, tomcat/trunk/test/org/apache/catalina/valves/rewrite/ Modified: tomcat/tc8.0.x/trunk/ (props changed) tomcat/tc8.0.x/trunk/java/org/apache/catalina/Wrapper.java tomcat/tc8.0.x/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java tomcat/tc8.0.x/trunk/java/org/apache/catalina/core/StandardContext.java tomcat/tc8.0.x/trunk/java/org/apache/catalina/core/StandardWrapper.java tomcat/tc8.0.x/trunk/java/org/apache/catalina/mapper/Mapper.java tomcat/tc8.0.x/trunk/java/org/apache/catalina/servlets/DefaultServlet.java tomcat/tc8.0.x/trunk/java/org/apache/catalina/util/URLEncoder.java tomcat/tc8.0.x/trunk/java/org/apache/catalina/valves/rewrite/RewriteValve.java tomcat/tc8.0.x/trunk/java/org/apache/coyote/Adapter.java tomcat/tc8.0.x/trunk/test/org/apache/catalina/startup/TomcatBaseTest.java tomcat/tc8.0.x/trunk/test/org/apache/catalina/valves/rewrite/TestRewriteValve.java tomcat/tc8.0.x/trunk/webapps/docs/changelog.xml Propchange: tomcat/tc8.0.x/trunk/ ------------------------------------------------------------------------------ --- svn:mergeinfo (original) +++ svn:mergeinfo Fri Dec 12 16:22:05 2014 @@ -1 +1 @@ -/tomcat/trunk:1636524,1637156,1637176,1637188,1637331,1637684,1637695,1638720-1638725,1639653,1640010,1640083-1640084,1640088,1640275,1640322,1640347,1640361,1640365,1640403,1640410,1640652,1640655-1640658,1640688,1640700-1640883,1640903,1640976,1640978,1641000,1641026,1641038-1641039,1641051-1641052,1641058,1641064,1641300,1641369,1641374,1641380,1641486,1641634,1641656-1641692,1641704,1641707-1641718,1641720-1641722,1641735,1641981,1642233,1642280,1642554,1642564,1642595,1642606,1642668,1642679,1642697,1642699,1642766,1643002,1643045,1643054-1643055,1643066,1643121,1643128,1643206,1643209-1643210,1643216,1643249,1643270,1643283,1643309-1643310,1643323,1643365-1643366,1643370-1643371,1643465,1643474,1643570,1643634,1643649,1643651,1643654,1643675,1643731,1643733-1643734,1643761,1643766,1643814,1643937,1643963,1644017,1644169,1644201-1644203,1644321,1644323,1644516,1644523,1644529,1644535,1644815 +/tomcat/trunk:1636524,1637156,1637176,1637188,1637331,1637684,1637695,1638720-1638725,1639653,1640010,1640083-1640084,1640088,1640275,1640322,1640347,1640361,1640365,1640403,1640410,1640652,1640655-1640658,1640688,1640700-1640883,1640903,1640976,1640978,1641000,1641026,1641038-1641039,1641051-1641052,1641058,1641064,1641300,1641369,1641374,1641380,1641486,1641634,1641656-1641692,1641704,1641707-1641718,1641720-1641722,1641735,1641981,1642233,1642280,1642554,1642564,1642595,1642606,1642668,1642679,1642697,1642699,1642766,1643002,1643045,1643054-1643055,1643066,1643121,1643128,1643206,1643209-1643210,1643216,1643249,1643270,1643283,1643309-1643310,1643323,1643365-1643366,1643370-1643371,1643465,1643474,1643570,1643634,1643649,1643651,1643654,1643675,1643731,1643733-1643734,1643761,1643766,1643814,1643937,1643963,1644017,1644169,1644201-1644203,1644321,1644323,1644516,1644523,1644529,1644535,1644730,1644768,1644784-1644785,1644790,1644793,1644815,1644884,1644886,1644890,1644892,1644910 ,1644924,1644929-1644930,1644935 Modified: tomcat/tc8.0.x/trunk/java/org/apache/catalina/Wrapper.java URL: http://svn.apache.org/viewvc/tomcat/tc8.0.x/trunk/java/org/apache/catalina/Wrapper.java?rev=1644962&r1=1644961&r2=1644962&view=diff ============================================================================== --- tomcat/tc8.0.x/trunk/java/org/apache/catalina/Wrapper.java (original) +++ tomcat/tc8.0.x/trunk/java/org/apache/catalina/Wrapper.java Fri Dec 12 16:22:05 2014 @@ -134,7 +134,9 @@ public interface Wrapper extends Contain * servlet. * * @return Array of names of the methods supported by the underlying - * servlet + * servlet + * + * @throws ServletException If the target servlet can not be loaded */ public String[] getServletMethods() throws ServletException; Modified: tomcat/tc8.0.x/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java URL: http://svn.apache.org/viewvc/tomcat/tc8.0.x/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java?rev=1644962&r1=1644961&r2=1644962&view=diff ============================================================================== --- tomcat/tc8.0.x/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java (original) +++ tomcat/tc8.0.x/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java Fri Dec 12 16:22:05 2014 @@ -24,6 +24,7 @@ import java.util.concurrent.atomic.Atomi import javax.servlet.ReadListener; import javax.servlet.RequestDispatcher; +import javax.servlet.ServletException; import javax.servlet.SessionTrackingMode; import javax.servlet.WriteListener; import javax.servlet.http.HttpServletResponse; @@ -123,28 +124,6 @@ public class CoyoteAdapter implements Ad StringManager.getManager(Constants.Package); - /** - * Encoder for the Location URL in HTTP redirects. - */ - protected static final URLEncoder urlEncoder; - - - // ----------------------------------------------------- Static Initializer - - - /** - * The safe character set. - */ - static { - urlEncoder = new URLEncoder(); - urlEncoder.addSafeCharacter('-'); - urlEncoder.addSafeCharacter('_'); - urlEncoder.addSafeCharacter('.'); - urlEncoder.addSafeCharacter('*'); - urlEncoder.addSafeCharacter('/'); - } - - // -------------------------------------------------------- Adapter Methods @@ -610,6 +589,15 @@ public class CoyoteAdapter implements Ad } + @Override + public boolean prepare(org.apache.coyote.Request req, org.apache.coyote.Response res) + throws IOException, ServletException { + Request request = (Request) req.getNote(ADAPTER_NOTES); + Response response = (Response) res.getNote(ADAPTER_NOTES); + + return postParseRequest(req, request, res, response); + } + @Override public void errorDispatch(org.apache.coyote.Request req, @@ -741,19 +729,30 @@ public class CoyoteAdapter implements Ad // ------------------------------------------------------ Protected Methods - /** - * Parse additional request parameters. + * Perform the necessary processing after the HTTP headers have been parsed + * to enable the request/response pair to be passed to the start of the + * container pipeline for processing. + * + * @param req The coyote request object + * @param request The catalina request object + * @param res The coyote response object + * @param response The catalina response object + * + * @return <code>true</code> if the request should be passed on to the start + * of the container pipeline, otherwise <code>false</code> + * + * @throws IOException If there is insufficient space in a buffer while + * processing headers + * @throws ServletException If the supported methods of the target servlet + * can not be determined */ - protected boolean postParseRequest(org.apache.coyote.Request req, - Request request, - org.apache.coyote.Response res, - Response response) - throws Exception { - - // XXX the processor may have set a correct scheme and port prior to this point, - // in ajp13 protocols dont make sense to get the port from the connector... - // otherwise, use connector configuration + protected boolean postParseRequest(org.apache.coyote.Request req, Request request, + org.apache.coyote.Response res, Response response) throws IOException, ServletException { + + // If the processor has set the scheme (AJP will do this) use this to + // set the secure flag as well. If the processor hasn't set it, use the + // settings from the connector if (! req.scheme().isNull()) { // use processor specified scheme to determine secure state request.setSecure(req.scheme().equals("https")); @@ -764,9 +763,6 @@ public class CoyoteAdapter implements Ad request.setSecure(connector.getSecure()); } - // FIXME: the code below doesnt belongs to here, - // this is only have sense - // in Http11, not in ajp13.. // At this point the Host header has been processed. // Override if the proxyPort/proxyHost are set String proxyName = connector.getProxyName(); @@ -778,8 +774,10 @@ public class CoyoteAdapter implements Ad req.serverName().setString(proxyName); } + MessageBytes undecodedURI = req.requestURI(); + // Check for ping OPTIONS * request - if (req.requestURI().equals("*")) { + if (undecodedURI.equals("*")) { if (req.method().equalsIgnoreCase("OPTIONS")) { StringBuilder allow = new StringBuilder(); allow.append("GET, HEAD, POST, PUT, DELETE"); @@ -799,11 +797,12 @@ public class CoyoteAdapter implements Ad return false; } - // Copy the raw URI to the decodedURI MessageBytes decodedURI = req.decodedURI(); - decodedURI.duplicate(req.requestURI()); - if (decodedURI.getType() == MessageBytes.T_BYTES) { + if (undecodedURI.getType() == MessageBytes.T_BYTES) { + // Copy the raw URI to the decodedURI + decodedURI.duplicate(undecodedURI); + // Parse the path parameters. This will: // - strip out the path parameters // - convert the decodedURI to bytes @@ -839,9 +838,13 @@ public class CoyoteAdapter implements Ad return false; } } else { - // The URL is chars or String, and has been sent using an in-memory - // protocol handler, we have to assume the URL has been properly - // decoded already + /* The URI is chars or String, and has been sent using an in-memory + * protocol handler. The following assumptions are made: + * - req.requestURI() has been set to the 'original' non-decoded, + * non-normalized URI + * - req.decodedURI() has been set to the decoded, normalized form + * of req.requestURI() + */ decodedURI.toChars(); // Remove all path parameters; any needed path parameter should be set // using the request object rather than passing it in the URL @@ -876,11 +879,6 @@ public class CoyoteAdapter implements Ad } else { serverName = req.serverName(); } - if (request.isAsyncStarted()) { - //TODO SERVLET3 - async - //reset mapping data, should prolly be done elsewhere - request.getMappingData().recycle(); - } // Version for the second mapping loop and // Context that we expect to get for that version @@ -983,7 +981,7 @@ public class CoyoteAdapter implements Ad // Possible redirect MessageBytes redirectPathMB = request.getMappingData().redirectPath; if (!redirectPathMB.isNull()) { - String redirectPath = urlEncoder.encode(redirectPathMB.toString()); + String redirectPath = URLEncoder.DEFAULT.encode(redirectPathMB.toString()); String query = request.getQueryString(); if (request.isRequestedSessionIdFromURL()) { // This is not optimal, but as this is not very common, it @@ -1146,9 +1144,6 @@ public class CoyoteAdapter implements Ad SSL_ONLY.equals(request.getServletContext() .getEffectiveSessionTrackingModes()) && request.connector.secure) { - // TODO Is there a better way to map SSL sessions to our sesison ID? - // TODO The request.getAttribute() will cause a number of other SSL - // attribute to be populated. Is this a performance concern? request.setRequestedSessionId( request.getAttribute(SSLSupport.SESSION_ID_KEY).toString()); request.setRequestedSessionSSL(true); @@ -1213,8 +1208,7 @@ public class CoyoteAdapter implements Ad /** * Character conversion of the URI. */ - protected void convertURI(MessageBytes uri, Request request) - throws Exception { + protected void convertURI(MessageBytes uri, Request request) throws IOException { ByteChunk bc = uri.getByteChunk(); int length = bc.getLength(); @@ -1288,13 +1282,13 @@ public class CoyoteAdapter implements Ad /** - * Normalize URI. - * <p> - * This method normalizes "\", "//", "/./" and "/../". This method will - * return false when trying to go above the root, or if the URI contains - * a null byte. + * This method normalizes "\", "//", "/./" and "/../". * * @param uriMB URI to be normalized + * + * @return <code>false</false> if normalizing this URI would require going + * above the root, or if the URI contains a null byte, otherwise + * <code>true</code> */ public static boolean normalize(MessageBytes uriMB) { @@ -1405,13 +1399,14 @@ public class CoyoteAdapter implements Ad /** - * Check that the URI is normalized following character decoding. - * <p> - * This method checks for "\", 0, "//", "/./" and "/../". This method will - * return false if sequences that are supposed to be normalized are still - * present in the URI. + * Check that the URI is normalized following character decoding. This + * method checks for "\", 0, "//", "/./" and "/../". * * @param uriMB URI to be checked (should be chars) + * + * @return <code>false</code> if sequences that are supposed to be + * normalized are still present in the URI, otherwise + * <code>true</code> */ public static boolean checkNormalize(MessageBytes uriMB) { Modified: tomcat/tc8.0.x/trunk/java/org/apache/catalina/core/StandardContext.java URL: http://svn.apache.org/viewvc/tomcat/tc8.0.x/trunk/java/org/apache/catalina/core/StandardContext.java?rev=1644962&r1=1644961&r2=1644962&view=diff ============================================================================== --- tomcat/tc8.0.x/trunk/java/org/apache/catalina/core/StandardContext.java (original) +++ tomcat/tc8.0.x/trunk/java/org/apache/catalina/core/StandardContext.java Fri Dec 12 16:22:05 2014 @@ -168,32 +168,8 @@ public class StandardContext extends Con } - // ----------------------------------------------------- Class Variables - - - /** - * Array containing the safe characters set. - */ - protected static URLEncoder urlEncoder; - - - /** - * GMT timezone - all HTTP dates are on GMT - */ - static { - urlEncoder = new URLEncoder(); - urlEncoder.addSafeCharacter('~'); - urlEncoder.addSafeCharacter('-'); - urlEncoder.addSafeCharacter('_'); - urlEncoder.addSafeCharacter('.'); - urlEncoder.addSafeCharacter('*'); - urlEncoder.addSafeCharacter('/'); - } - - // ----------------------------------------------------- Instance Variables - /** * Allow multipart/form-data requests to be parsed even when the * target servlet doesn't specify @MultipartConfig or have a @@ -1977,7 +1953,7 @@ public class StandardContext extends Con log.warn(sm.getString( "standardContext.pathInvalid", path, this.path)); } - encodedPath = urlEncoder.encode(this.path); + encodedPath = URLEncoder.DEFAULT.encode(this.path); if (getName() == null) { setName(this.path); } Modified: tomcat/tc8.0.x/trunk/java/org/apache/catalina/core/StandardWrapper.java URL: http://svn.apache.org/viewvc/tomcat/tc8.0.x/trunk/java/org/apache/catalina/core/StandardWrapper.java?rev=1644962&r1=1644961&r2=1644962&view=diff ============================================================================== --- tomcat/tc8.0.x/trunk/java/org/apache/catalina/core/StandardWrapper.java (original) +++ tomcat/tc8.0.x/trunk/java/org/apache/catalina/core/StandardWrapper.java Fri Dec 12 16:22:05 2014 @@ -587,16 +587,6 @@ public class StandardWrapper extends Con } - /** - * Gets the names of the methods supported by the underlying servlet. - * - * This is the same set of methods included in the Allow response header - * in response to an OPTIONS request method processed by the underlying - * servlet. - * - * @return Array of names of the methods supported by the underlying - * servlet - */ @Override public String[] getServletMethods() throws ServletException { Modified: tomcat/tc8.0.x/trunk/java/org/apache/catalina/mapper/Mapper.java URL: http://svn.apache.org/viewvc/tomcat/tc8.0.x/trunk/java/org/apache/catalina/mapper/Mapper.java?rev=1644962&r1=1644961&r2=1644962&view=diff ============================================================================== --- tomcat/tc8.0.x/trunk/java/org/apache/catalina/mapper/Mapper.java (original) +++ tomcat/tc8.0.x/trunk/java/org/apache/catalina/mapper/Mapper.java Fri Dec 12 16:22:05 2014 @@ -16,6 +16,7 @@ */ package org.apache.catalina.mapper; +import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -672,10 +673,11 @@ public final class Mapper { * @param uri URI * @param mappingData This structure will contain the result of the mapping * operation + * @throws IOException if the buffers are too small to hold the results of + * the mapping. */ public void map(MessageBytes host, MessageBytes uri, String version, - MappingData mappingData) - throws Exception { + MappingData mappingData) throws IOException { if (host.isNull()) { host.getCharChunk().append(defaultHostName); @@ -696,9 +698,11 @@ public final class Mapper { * @param uri URI * @param mappingData This structure will contain the result of the mapping * operation + * @throws IOException if the buffers are too small to hold the results of + * the mapping. */ public void map(Context context, MessageBytes uri, - MappingData mappingData) throws Exception { + MappingData mappingData) throws IOException { ContextVersion contextVersion = contextObjectToContextVersionMap.get(context); @@ -715,9 +719,10 @@ public final class Mapper { /** * Map the specified URI. + * @throws IOException */ private final void internalMap(CharChunk host, CharChunk uri, - String version, MappingData mappingData) throws Exception { + String version, MappingData mappingData) throws IOException { if (mappingData.host != null) { // The legacy code (dating down at least to Tomcat 4.1) just @@ -822,11 +827,12 @@ public final class Mapper { /** * Wrapper mapping. + * @throws IOException if the buffers are too small to hold the results of + * the mapping. */ private final void internalMapWrapper(ContextVersion contextVersion, CharChunk path, - MappingData mappingData) - throws Exception { + MappingData mappingData) throws IOException { int pathOffset = path.getOffset(); int pathEnd = path.getEnd(); Modified: tomcat/tc8.0.x/trunk/java/org/apache/catalina/servlets/DefaultServlet.java URL: http://svn.apache.org/viewvc/tomcat/tc8.0.x/trunk/java/org/apache/catalina/servlets/DefaultServlet.java?rev=1644962&r1=1644961&r2=1644962&view=diff ============================================================================== --- tomcat/tc8.0.x/trunk/java/org/apache/catalina/servlets/DefaultServlet.java (original) +++ tomcat/tc8.0.x/trunk/java/org/apache/catalina/servlets/DefaultServlet.java Fri Dec 12 16:22:05 2014 @@ -129,11 +129,6 @@ public class DefaultServlet extends Http */ protected static final StringManager sm = StringManager.getManager(Constants.Package); - /** - * Array containing the safe characters set. - */ - protected static final URLEncoder urlEncoder; - private static final DocumentBuilderFactory factory; private static final SecureEntityResolver secureEntityResolver; @@ -162,13 +157,6 @@ public class DefaultServlet extends Http // ----------------------------------------------------- Static Initializer static { - urlEncoder = new URLEncoder(); - urlEncoder.addSafeCharacter('-'); - urlEncoder.addSafeCharacter('_'); - urlEncoder.addSafeCharacter('.'); - urlEncoder.addSafeCharacter('*'); - urlEncoder.addSafeCharacter('/'); - if (Globals.IS_SECURITY_ENABLED) { factory = DocumentBuilderFactory.newInstance(); factory.setNamespaceAware(true); @@ -678,7 +666,7 @@ public class DefaultServlet extends Http * @param path Path which has to be rewritten */ protected String rewriteUrl(String path) { - return urlEncoder.encode( path ); + return URLEncoder.DEFAULT.encode( path ); } Modified: tomcat/tc8.0.x/trunk/java/org/apache/catalina/util/URLEncoder.java URL: http://svn.apache.org/viewvc/tomcat/tc8.0.x/trunk/java/org/apache/catalina/util/URLEncoder.java?rev=1644962&r1=1644961&r2=1644962&view=diff ============================================================================== --- tomcat/tc8.0.x/trunk/java/org/apache/catalina/util/URLEncoder.java (original) +++ tomcat/tc8.0.x/trunk/java/org/apache/catalina/util/URLEncoder.java Fri Dec 12 16:22:05 2014 @@ -38,6 +38,16 @@ public class URLEncoder { {'0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'A', 'B', 'C', 'D', 'E', 'F'}; + public static final URLEncoder DEFAULT = new URLEncoder(); + static { + DEFAULT.addSafeCharacter('~'); + DEFAULT.addSafeCharacter('-'); + DEFAULT.addSafeCharacter('_'); + DEFAULT.addSafeCharacter('.'); + DEFAULT.addSafeCharacter('*'); + DEFAULT.addSafeCharacter('/'); + } + //Array containing the safe characters set. protected final BitSet safeCharacters = new BitSet(256); Modified: tomcat/tc8.0.x/trunk/java/org/apache/catalina/valves/rewrite/RewriteValve.java URL: http://svn.apache.org/viewvc/tomcat/tc8.0.x/trunk/java/org/apache/catalina/valves/rewrite/RewriteValve.java?rev=1644962&r1=1644961&r2=1644962&view=diff ============================================================================== --- tomcat/tc8.0.x/trunk/java/org/apache/catalina/valves/rewrite/RewriteValve.java (original) +++ tomcat/tc8.0.x/trunk/java/org/apache/catalina/valves/rewrite/RewriteValve.java Fri Dec 12 16:22:05 2014 @@ -41,12 +41,16 @@ import org.apache.catalina.Host; import org.apache.catalina.Lifecycle; import org.apache.catalina.LifecycleException; import org.apache.catalina.LifecycleListener; +import org.apache.catalina.Pipeline; +import org.apache.catalina.connector.Connector; import org.apache.catalina.connector.Request; import org.apache.catalina.connector.Response; import org.apache.catalina.util.LifecycleSupport; +import org.apache.catalina.util.URLEncoder; import org.apache.catalina.valves.ValveBase; import org.apache.tomcat.util.buf.CharChunk; import org.apache.tomcat.util.buf.MessageBytes; +import org.apache.tomcat.util.http.RequestUtil; import org.apache.tomcat.util.net.URL; public class RewriteValve extends ValveBase { @@ -443,7 +447,7 @@ public class RewriteValve extends ValveB queryString = urlString.substring(queryIndex+1); urlString = urlString.substring(0, queryIndex); } - // Set the new URL + // Set the new 'original' URI String contextPath = null; if (context) { contextPath = request.getContextPath(); @@ -454,8 +458,17 @@ public class RewriteValve extends ValveB if (context) { chunk.append(contextPath); } - chunk.append(urlString); + chunk.append(URLEncoder.DEFAULT.encode(urlString)); request.getCoyoteRequest().requestURI().toChars(); + // Decoded and normalized URI + request.getCoyoteRequest().decodedURI().setString(null); + chunk = request.getCoyoteRequest().decodedURI().getCharChunk(); + chunk.recycle(); + if (context) { + chunk.append(contextPath); + } + chunk.append(RequestUtil.normalize(urlString)); + request.getCoyoteRequest().decodedURI().toChars(); // Set the new Query if there is one if (queryString != null) { request.getCoyoteRequest().queryString().setString(null); @@ -475,8 +488,14 @@ public class RewriteValve extends ValveB request.getMappingData().recycle(); // Reinvoke the whole request recursively try { - request.getConnector().getProtocolHandler().getAdapter().service - (request.getCoyoteRequest(), response.getCoyoteResponse()); + Connector connector = request.getConnector(); + if (!connector.getProtocolHandler().getAdapter().prepare( + request.getCoyoteRequest(), response.getCoyoteResponse())) { + return; + } + Pipeline pipeline = connector.getService().getContainer().getPipeline(); + request.setAsyncSupported(pipeline.isAsyncSupported()); + pipeline.getFirst().invoke(request, response); } catch (Exception e) { // This doesn't actually happen in the Catalina adapter implementation } @@ -541,7 +560,9 @@ public class RewriteValve extends ValveB * Example: * RewriteCond %{REMOTE_HOST} ^host1.* [OR] * - * @param line + * @param line A line from the rewrite configuration + * + * @return The condition, rule or map resulting from parsing the line */ public static Object parse(String line) { StringTokenizer tokenizer = new StringTokenizer(line); Modified: tomcat/tc8.0.x/trunk/java/org/apache/coyote/Adapter.java URL: http://svn.apache.org/viewvc/tomcat/tc8.0.x/trunk/java/org/apache/coyote/Adapter.java?rev=1644962&r1=1644961&r2=1644962&view=diff ============================================================================== --- tomcat/tc8.0.x/trunk/java/org/apache/coyote/Adapter.java (original) +++ tomcat/tc8.0.x/trunk/java/org/apache/coyote/Adapter.java Fri Dec 12 16:22:05 2014 @@ -30,6 +30,9 @@ public interface Adapter { /** * Call the service method, and notify all listeners * + * @param req The request object + * @param res The response object + * * @exception Exception if an error happens during handling of * the request. Common errors are: * <ul><li>IOException if an input/output error occurs and we are @@ -42,8 +45,23 @@ public interface Adapter { * Tomcat should be able to handle and log any other exception ( including * runtime exceptions ) */ - public void service(Request req, Response res) - throws Exception; + public void service(Request req, Response res) throws Exception; + + /** + * Prepare the given request/response for processing. This method requires + * that the request object has been populated with the information available + * from the HTTP headers. + * + * @param req The request object + * @param res The response object + * + * @return <code>true</code> if processing can continue, otherwise + * <code>false</code> in which case an appropriate error will have + * been set on the response + * + * @throws Exception If the processing fails unexpectedly + */ + public boolean prepare(Request req, Response res) throws Exception; public boolean event(Request req, Response res, SocketStatus status) throws Exception; Modified: tomcat/tc8.0.x/trunk/test/org/apache/catalina/startup/TomcatBaseTest.java URL: http://svn.apache.org/viewvc/tomcat/tc8.0.x/trunk/test/org/apache/catalina/startup/TomcatBaseTest.java?rev=1644962&r1=1644961&r2=1644962&view=diff ============================================================================== --- tomcat/tc8.0.x/trunk/test/org/apache/catalina/startup/TomcatBaseTest.java (original) +++ tomcat/tc8.0.x/trunk/test/org/apache/catalina/startup/TomcatBaseTest.java Fri Dec 12 16:22:05 2014 @@ -231,6 +231,25 @@ public abstract class TomcatBaseTest ext } + /** + * Simple servlet that dumps request information. Tests using this should + * note that additional information may be added to in the future and should + * therefore test return values accordingly. + */ + public static final class SnoopServlet extends HttpServlet { + + private static final long serialVersionUID = 1L; + + @Override + protected void doGet(HttpServletRequest req, HttpServletResponse resp) + throws ServletException, IOException { + resp.setContentType("text/plain"); + PrintWriter out = resp.getWriter(); + out.println("00-RequestURI-" + req.getRequestURI()); + } + } + + /* * Wrapper for getting the response. */ Modified: tomcat/tc8.0.x/trunk/test/org/apache/catalina/valves/rewrite/TestRewriteValve.java URL: http://svn.apache.org/viewvc/tomcat/tc8.0.x/trunk/test/org/apache/catalina/valves/rewrite/TestRewriteValve.java?rev=1644962&r1=1644730&r2=1644962&view=diff ============================================================================== --- tomcat/tc8.0.x/trunk/test/org/apache/catalina/valves/rewrite/TestRewriteValve.java (original) +++ tomcat/tc8.0.x/trunk/test/org/apache/catalina/valves/rewrite/TestRewriteValve.java Fri Dec 12 16:22:05 2014 @@ -17,7 +17,6 @@ package org.apache.catalina.valves.rewrite; import org.junit.Assert; -import org.junit.Ignore; import org.junit.Test; import org.apache.catalina.Context; @@ -29,16 +28,25 @@ public class TestRewriteValve extends To @Test public void testNoRewrite() throws Exception { - doTestRewrite(""); + doTestRewrite("", "/a/%255A", "/a/%255A"); } @Test - @Ignore // getRequestURI is not encoded public void testNoopRewrite() throws Exception { - doTestRewrite("RewriteRule ^(.*) $1"); + doTestRewrite("RewriteRule ^(.*) $1", "/a/%255A", "/a/%255A"); } - private void doTestRewrite(String config) throws Exception { + @Test + public void testPathRewrite() throws Exception { + doTestRewrite("RewriteRule ^/b(.*) /a$1", "/b/%255A", "/a/%255A"); + } + + @Test + public void testNonNormalizedPathRewrite() throws Exception { + doTestRewrite("RewriteRule ^/b/(.*) /b/../a/$1", "/b/%255A", "/a/%255A"); + } + + private void doTestRewrite(String config, String request, String expectedURI) throws Exception { Tomcat tomcat = getTomcatInstance(); // No file system docBase required @@ -56,9 +64,9 @@ public class TestRewriteValve extends To tomcat.start(); - ByteChunk res = getUrl("http://localhost:" + getPort() + "/a/%255A"); + ByteChunk res = getUrl("http://localhost:" + getPort() + request); String body = res.toString(); - Assert.assertTrue(body, body.contains("/a/%255A")); + Assert.assertTrue(body, body.contains(expectedURI)); } } Modified: tomcat/tc8.0.x/trunk/webapps/docs/changelog.xml URL: http://svn.apache.org/viewvc/tomcat/tc8.0.x/trunk/webapps/docs/changelog.xml?rev=1644962&r1=1644961&r2=1644962&view=diff ============================================================================== --- tomcat/tc8.0.x/trunk/webapps/docs/changelog.xml (original) +++ tomcat/tc8.0.x/trunk/webapps/docs/changelog.xml Fri Dec 12 16:22:05 2014 @@ -151,6 +151,20 @@ <bug>57331</bug>: Allow ExpiresFilter to use "year" as synonym for "years" in its configuration. (kkolinko) </fix> + <fix> + Ensure that if the RewriteValve rewrites a request that subsequent calls + to <code>HttpServletRequest.getRequestURI()</code> return the undecoded + URI. (markt) + </fix> + <fix> + Ensure that if the RewriteValve rewrites a request to a non-normalized + URI that the URI is normalized before the URI is mapped to ensure that + the correct mapping is applied. (markt) + </fix> + <fix> + Prevent NPEs being logged during post-processing for requests that have + been re-written by the RewriteValve. (markt) + </fix> </changelog> </subsection> <subsection name="Coyote"> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org