tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ma...@apache.org
Subject svn commit: r1149220 - in /tomcat/tc6.0.x/trunk: ./ java/org/apache/catalina/connector/ java/org/apache/tomcat/util/buf/ webapps/docs/
Date Thu, 21 Jul 2011 15:23:13 GMT
Author: markt
Date: Thu Jul 21 15:23:11 2011
New Revision: 1149220

URL: http://svn.apache.org/viewvc?rev=1149220&view=rev
Log:
Fix path parameter handling. Prevent the following URL failing with a 404: http://localhost:8080/examples/jsp/snp;x=y/snoop.jsp
Patch by kkolinko

Modified:
    tomcat/tc6.0.x/trunk/STATUS.txt
    tomcat/tc6.0.x/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java
    tomcat/tc6.0.x/trunk/java/org/apache/catalina/connector/LocalStrings.properties
    tomcat/tc6.0.x/trunk/java/org/apache/catalina/connector/Request.java
    tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/buf/ByteChunk.java
    tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml

Modified: tomcat/tc6.0.x/trunk/STATUS.txt
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/STATUS.txt?rev=1149220&r1=1149219&r2=1149220&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/STATUS.txt (original)
+++ tomcat/tc6.0.x/trunk/STATUS.txt Thu Jul 21 15:23:11 2011
@@ -58,19 +58,6 @@ PATCHES PROPOSED TO BACKPORT:
   +1: rjung, funkman, kkolinko
   -1:
 
-* Fix path parameter handling. Currently the following URL fails with a 404:
-  http://localhost:8080/examples/jsp/snp;x=y/snoop.jsp
-  http://people.apache.org/~kkolinko/patches/2010-11-17_tc6_path-params.patch
-  +1: kkolinko, markt, schultz
-  -1:
-
-   kkolinko: The old parseSessionId() method calls "clearRequestedSessionURL(request);"
-     when there is no sessionid in the path. The new code does not do that. Was that
-     call needed? It looks that it was not needed, but I might miss something.
-
-   kkolinko: Discussed in Re:r1005192 thread on dev@
-
-
 * Backport JSP unloading patch (BZ48358).
   The patch has substantially changed since the original version.
   Original revisions are: 937787, 1028377, 1028389, 1028396, 1028861, 1028862, 1028863,

Modified: tomcat/tc6.0.x/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java?rev=1149220&r1=1149219&r2=1149220&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java (original)
+++ tomcat/tc6.0.x/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java Thu Jul 21
15:23:11 2011
@@ -94,13 +94,6 @@ public class CoyoteAdapter implements Ad
 
 
     /**
-     * The match string for identifying a session ID parameter.
-     */
-    private static final String match =
-        ";" + Globals.SESSION_PARAMETER_NAME + "=";
-
-
-    /**
      * The string manager for this package.
      */
     protected StringManager sm =
@@ -423,61 +416,43 @@ public class CoyoteAdapter implements Ad
             req.serverName().setString(proxyName);
         }
 
-        // Parse session Id
-        parseSessionId(req, request);
-
-        // URI decoding
+        // Copy the raw URI to the decoded URI
         MessageBytes decodedURI = req.decodedURI();
         decodedURI.duplicate(req.requestURI());
 
-        if (decodedURI.getType() == MessageBytes.T_BYTES) {
-            // Remove any path parameters
-            ByteChunk uriBB = decodedURI.getByteChunk();
-            int semicolon = uriBB.indexOf(';', 0);
-            if (semicolon > 0) {
-                decodedURI.setBytes
-                    (uriBB.getBuffer(), uriBB.getStart(), semicolon);
-            }
-            // %xx decoding of the URL
-            try {
-                req.getURLDecoder().convert(decodedURI, false);
-            } catch (IOException ioe) {
-                res.setStatus(400);
-                res.setMessage("Invalid URI: " + ioe.getMessage());
-                connector.getService().getContainer().logAccess(
-                        request, response, 0, true);
-                return false;
-            }
-            // Normalization
-            if (!normalize(req.decodedURI())) {
-                res.setStatus(400);
-                res.setMessage("Invalid URI");
-                connector.getService().getContainer().logAccess(
-                        request, response, 0, true);
-                return false;
-            }
-            // Character decoding
-            convertURI(decodedURI, request);
-            // Check that the URI is still normalized
-            if (!checkNormalize(req.decodedURI())) {
-                res.setStatus(400);
-                res.setMessage("Invalid URI character encoding");
-                connector.getService().getContainer().logAccess(
-                        request, response, 0, true);
-                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
-            decodedURI.toChars();
-            // Remove any path parameters
-            CharChunk uriCC = decodedURI.getCharChunk();
-            int semicolon = uriCC.indexOf(';');
-            if (semicolon > 0) {
-                decodedURI.setChars
-                    (uriCC.getBuffer(), uriCC.getStart(), semicolon);
-            }
+        // Parse the path parameters. This will:
+        //   - strip out the path parameters
+        //   - convert the decodedURI to bytes
+        parsePathParameters(req, request);
+
+        // URI decoding
+        // %xx decoding of the URL
+        try {
+            req.getURLDecoder().convert(decodedURI, false);
+        } catch (IOException ioe) {
+            res.setStatus(400);
+            res.setMessage("Invalid URI: " + ioe.getMessage());
+            connector.getService().getContainer().logAccess(
+                    request, response, 0, true);
+            return false;
+        }
+        // Normalization
+        if (!normalize(req.decodedURI())) {
+            res.setStatus(400);
+            res.setMessage("Invalid URI");
+            connector.getService().getContainer().logAccess(
+                    request, response, 0, true);
+            return false;
+        }
+        // Character decoding
+        convertURI(decodedURI, request);
+        // Check that the URI is still normalized
+        if (!checkNormalize(req.decodedURI())) {
+            res.setStatus(400);
+            res.setMessage("Invalid URI character encoding");
+            connector.getService().getContainer().logAccess(
+                    request, response, 0, true);
+            return false;
         }
 
         // Set the remote principal
@@ -573,6 +548,12 @@ public class CoyoteAdapter implements Ad
         }
 
         // Parse session Id
+        String sessionID =
+            request.getPathParameter(Globals.SESSION_PARAMETER_NAME);
+        if (sessionID != null) {
+            request.setRequestedSessionId(sessionID);
+            request.setRequestedSessionURL(true);
+        }
         parseSessionCookiesId(req, request);
 
         return true;
@@ -586,53 +567,131 @@ public class CoyoteAdapter implements Ad
             return (false);
     }
 
+
     /**
-     * Parse session id in URL.
+     * Extract the path parameters from the request. This assumes parameters are
+     * of the form /path;name=value;name2=value2/ etc. Currently only really
+     * interested in the session ID that will be in this form. Other parameters
+     * can safely be ignored.
+     * 
+     * @param req
+     * @param request
      */
-    protected void parseSessionId(org.apache.coyote.Request req, Request request) {
+    protected void parsePathParameters(org.apache.coyote.Request req,
+            Request request) {
 
-        ByteChunk uriBC = req.requestURI().getByteChunk();
-        int semicolon = uriBC.indexOf(match, 0, match.length(), 0);
+        // Process in bytes (this is default format so this is normally a NO-OP
+        req.decodedURI().toBytes();
 
-        if (semicolon > 0) {
-            // What encoding to use? Some platforms, eg z/os, use a default
-            // encoding that doesn't give the expected result so be explicit 
-            String enc = connector.getURIEncoding();
-            if (enc == null) {
-                enc = "ISO-8859-1";
-            }
+        ByteChunk uriBC = req.decodedURI().getByteChunk();
+        int semicolon = uriBC.indexOf(';', 0);
 
-            // Parse session ID, and extract it from the decoded request URI
+        // What encoding to use? Some platforms, eg z/os, use a default
+        // encoding that doesn't give the expected result so be explicit
+        String enc = connector.getURIEncoding();
+        if (enc == null) {
+            enc = "ISO-8859-1";
+        }
+
+        if (log.isDebugEnabled()) {
+            log.debug(sm.getString("coyoteAdapter.debug", "uriBC",
+                    uriBC.toString()));
+            log.debug(sm.getString("coyoteAdapter.debug", "semicolon",
+                    String.valueOf(semicolon)));
+            log.debug(sm.getString("coyoteAdapter.debug", "enc", enc));
+        }
+
+        boolean warnedEncoding = false;
+
+        while (semicolon > -1) {
+            // Parse path param, and extract it from the decoded request URI
             int start = uriBC.getStart();
             int end = uriBC.getEnd();
 
-            int sessionIdStart = semicolon + match.length();
-            int semicolon2 = uriBC.indexOf(';', sessionIdStart);
-            try {
-                if (semicolon2 >= 0) {
-                    request.setRequestedSessionId
-                        (new String(uriBC.getBuffer(), start + sessionIdStart,
-                                semicolon2 - sessionIdStart, enc));
-                    // Extract session ID from request URI
-                    byte[] buf = uriBC.getBuffer();
-                    for (int i = 0; i < end - start - semicolon2; i++) {
-                        buf[start + semicolon + i] 
-                            = buf[start + i + semicolon2];
+            int pathParamStart = semicolon + 1;
+            int pathParamEnd = ByteChunk.findBytes(uriBC.getBuffer(),
+                    start + pathParamStart, end,
+                    new byte[] {';', '/'});
+
+            String pv = null;
+
+            if (pathParamEnd >= 0) {
+                try {
+                    pv = (new String(uriBC.getBuffer(), start + pathParamStart,
+                                pathParamEnd - pathParamStart, enc));
+                } catch (UnsupportedEncodingException e) {
+                    if (!warnedEncoding) {
+                        log.warn(sm.getString("coyoteAdapter.parsePathParam",
+                                enc));
+                        warnedEncoding = true;
                     }
-                    uriBC.setBytes(buf, start,
-                            end - start - semicolon2 + semicolon);
-                } else {
-                    request.setRequestedSessionId
-                        (new String(uriBC.getBuffer(), start + sessionIdStart, 
-                                (end - start) - sessionIdStart, enc));
-                    uriBC.setEnd(start + semicolon);
-                }
-                request.setRequestedSessionURL(true);
-            } catch (UnsupportedEncodingException uee) {
-                // Make sure no session ID is returned
-                clearRequestedSessionURL(request);
-                log.warn(sm.getString("coyoteAdapter.parseSession", enc), uee);
+                }
+                // Extract path param from decoded request URI
+                byte[] buf = uriBC.getBuffer();
+                for (int i = 0; i < end - start - pathParamEnd; i++) {
+                    buf[start + semicolon + i] 
+                        = buf[start + i + pathParamEnd];
+                }
+                uriBC.setBytes(buf, start,
+                        end - start - pathParamEnd + semicolon);
+            } else {
+                try {
+                    pv = (new String(uriBC.getBuffer(), start + pathParamStart, 
+                                (end - start) - pathParamStart, enc));
+                } catch (UnsupportedEncodingException e) {
+                    if (!warnedEncoding) {
+                        log.warn(sm.getString("coyoteAdapter.parsePathParam",
+                                enc));
+                        warnedEncoding = true;
+                    }
+                }
+                uriBC.setEnd(start + semicolon);
+            }
+
+            if (log.isDebugEnabled()) {
+                log.debug(sm.getString("coyoteAdapter.debug", "pathParamStart",
+                        String.valueOf(pathParamStart)));
+                log.debug(sm.getString("coyoteAdapter.debug", "pathParamEnd",
+                        String.valueOf(pathParamEnd)));
+                log.debug(sm.getString("coyoteAdapter.debug", "pv", pv));
+            }
+
+            if (pv != null) {
+                int equals = pv.indexOf('=');
+                if (equals > -1) {
+                    String name = pv.substring(0, equals);
+                    String value = pv.substring(equals + 1); 
+                    request.addPathParameter(name, value);
+                    if (log.isDebugEnabled()) {
+                        log.debug(sm.getString("coyoteAdapter.debug", "equals",
+                                String.valueOf(equals)));
+                        log.debug(sm.getString("coyoteAdapter.debug", "name",
+                                name));
+                        log.debug(sm.getString("coyoteAdapter.debug", "value",
+                                value));
+                    }
+                }
             }
+
+            semicolon = uriBC.indexOf(';', semicolon);
+        }
+    }
+
+
+    /**
+     * Parse session id in URL.
+     * @deprecated Not used since 6.0.30
+     */
+    @Deprecated
+    protected void parseSessionId(org.apache.coyote.Request req, Request request) {
+
+        parsePathParameters(req, request);
+
+        String sessionID =
+            request.getPathParameter(Globals.SESSION_PARAMETER_NAME);
+        if (sessionID != null) {
+            request.setRequestedSessionId(sessionID);
+            request.setRequestedSessionURL(true);
         } else {
             clearRequestedSessionURL(request);
         }

Modified: tomcat/tc6.0.x/trunk/java/org/apache/catalina/connector/LocalStrings.properties
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/catalina/connector/LocalStrings.properties?rev=1149220&r1=1149219&r2=1149220&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/java/org/apache/catalina/connector/LocalStrings.properties (original)
+++ tomcat/tc6.0.x/trunk/java/org/apache/catalina/connector/LocalStrings.properties Thu Jul
21 15:23:11 2011
@@ -37,6 +37,8 @@ coyoteConnector.protocolUnregistrationFa
 coyoteAdapter.service=An exception or error occurred in the container during the request
processing
 coyoteAdapter.read=The servlet did not read all available bytes during the processing of
the read event
 coyoteAdapter.parseSession=Unable to parse the session ID using encoding [{0}]. The session
ID in the URL will be ignored.
+coyoteAdapter.parsePathParam=Unable to parse the path parameters using encoding [{0}]. The
path parameters in the URL will be ignored.
+coyoteAdapter.debug=The variable [{0}] has value [{1}]
 
 #
 # CoyoteResponse

Modified: tomcat/tc6.0.x/trunk/java/org/apache/catalina/connector/Request.java
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/catalina/connector/Request.java?rev=1149220&r1=1149219&r2=1149220&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/java/org/apache/catalina/connector/Request.java (original)
+++ tomcat/tc6.0.x/trunk/java/org/apache/catalina/connector/Request.java Thu Jul 21 15:23:11
2011
@@ -388,9 +388,23 @@ public class Request
     protected String localName = null;
 
 
+    /**
+     * Path parameters
+     */
+    protected Map<String,String> pathParameters = new HashMap<String, String>();
+
+
     // --------------------------------------------------------- Public Methods
 
 
+    protected void addPathParameter(String name, String value) {
+        pathParameters.put(name, value);
+    }
+
+    protected String getPathParameter(String name) {
+        return pathParameters.get(name);
+    }
+
     /**
      * Release all object references, and initialize instance variables, in
      * preparation for reuse of this object.
@@ -470,6 +484,7 @@ public class Request
             }
         }
 
+        pathParameters.clear();
     }
 
 

Modified: tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/buf/ByteChunk.java
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/buf/ByteChunk.java?rev=1149220&r1=1149219&r2=1149220&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/buf/ByteChunk.java (original)
+++ tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/buf/ByteChunk.java Thu Jul 21 15:23:11
2011
@@ -762,6 +762,30 @@ public final class ByteChunk implements 
         return -1;
     }
 
+    /**
+     * Returns the first instance of any of the given bytes in the byte array
+     * between the specified start and end.
+     * 
+     * @param bytes The byte array to search
+     * @param start The point to start searching from in the byte array
+     * @param end   The point to stop searching in the byte array
+     * @param b     The array of bytes to search for 
+     * @return      The position of the first instance of the byte or -1 if the
+     *                  byte is not found.
+     */
+    public static int findBytes(byte bytes[], int start, int end, byte b[]) {
+        int blen = b.length;
+        int offset = start;
+        while (offset < end) {
+            for (int i = 0;  i < blen; i++) 
+                if (bytes[offset] == b[i]) {
+                    return offset;
+                }
+            offset++;
+        }
+        return -1;
+    }
+
     /** Find a character, no side effects.
      *  @return index of char if found, -1 if not
      */

Modified: tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml?rev=1149220&r1=1149219&r2=1149220&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml Thu Jul 21 15:23:11 2011
@@ -163,6 +163,10 @@
         Log a failure if access log file cannot be opened. Improve i18n
         of messages. (kkolinko)
       </fix>
+      <fix>
+        Improve handling of URLs with path parameters and prevent incorrect 404
+        responses that could occur when path parameters were present. (kkolinko)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Coyote">



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Mime
View raw message