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-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
