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

Reply via email to