Author: markt Date: Sun May 16 21:47:26 2010 New Revision: 944920 URL: http://svn.apache.org/viewvc?rev=944920&view=rev Log: Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=49299 Implement the requirements of section 7.1.1 of the Servlet 3.0 spec that require the path parameter to change name as well. Remove the option to set the default via a system property for the sake of simplicity.
Added: tomcat/trunk/test/org/apache/catalina/connector/TestCoyoteAdaptor.java (with props) Modified: tomcat/trunk/java/org/apache/catalina/Globals.java tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java tomcat/trunk/java/org/apache/catalina/connector/LocalStrings.properties tomcat/trunk/java/org/apache/catalina/connector/Request.java tomcat/trunk/java/org/apache/catalina/connector/Response.java tomcat/trunk/java/org/apache/catalina/core/ApplicationSessionCookieConfig.java tomcat/trunk/java/org/apache/catalina/realm/RealmBase.java tomcat/trunk/webapps/docs/config/systemprops.xml Modified: tomcat/trunk/java/org/apache/catalina/Globals.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/Globals.java?rev=944920&r1=944919&r2=944920&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/catalina/Globals.java (original) +++ tomcat/trunk/java/org/apache/catalina/Globals.java Sun May 16 21:47:26 2010 @@ -265,22 +265,6 @@ public final class Globals { /** - * The name of the cookie used to pass the session identifier back - * and forth with the client. - */ - public static final String SESSION_COOKIE_NAME = "JSESSIONID"; - - - /** - * The name of the path parameter used to pass the session identifier - * back and forth with the client. - */ - public static final String SESSION_PARAMETER_NAME = - System.getProperty("org.apache.catalina.SESSION_PARAMETER_NAME", - "jsessionid"); - - - /** * The servlet context attribute under which we store a flag used * to mark this request as having been processed by the SSIServlet. * We do this because of the pathInfo mangling happening when using Modified: tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java?rev=944920&r1=944919&r2=944920&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java (original) +++ tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java Sun May 16 21:47:26 2010 @@ -103,13 +103,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 static final StringManager sm = @@ -503,55 +496,37 @@ public class CoyoteAdapter implements Ad req.serverName().setString(proxyName); } - // Parse session Id before decoding / removal of path params - parseSessionId(req, request); - - // URI decoding + // Copy the raw URI to the decodedURI 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()); - return false; - } - // Normalization - if (!normalize(req.decodedURI())) { - res.setStatus(400); - res.setMessage("Invalid URI"); - 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"); - 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()); + return false; + } + // Normalization + if (!normalize(req.decodedURI())) { + res.setStatus(400); + res.setMessage("Invalid URI"); + 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"); + return false; } // Set the remote principal @@ -621,8 +596,10 @@ public class CoyoteAdapter implements Ad if (request.isRequestedSessionIdFromURL()) { // This is not optimal, but as this is not very common, it // shouldn't matter - redirectPath = redirectPath + ";" + Globals.SESSION_PARAMETER_NAME + "=" - + request.getRequestedSessionId(); + redirectPath = redirectPath + ";" + + ApplicationSessionCookieConfig.getSessionUriParamName( + request.getContext()) + + "=" + request.getRequestedSessionId(); } if (query != null) { // This is not optimal, but as this is not very common, it @@ -634,15 +611,17 @@ public class CoyoteAdapter implements Ad } // Parse session Id - if (!request.getServletContext().getEffectiveSessionTrackingModes() + if (request.getServletContext().getEffectiveSessionTrackingModes() .contains(SessionTrackingMode.URL)) { - /* - * If we saw an ID in the URL but this is disabled - remove it - * Can't handle it when we parse the URL as we don't have the - * context at that point - */ - request.setRequestedSessionId(null); - request.setRequestedSessionURL(false); + + // Get the session ID if there was one + String sessionID = request.getPathParameter( + ApplicationSessionCookieConfig.getSessionUriParamName( + request.getContext())); + if (sessionID != null) { + request.setRequestedSessionId(sessionID); + request.setRequestedSessionURL(true); + } } parseSessionCookiesId(req, request); parseSessionSslId(request); @@ -651,6 +630,93 @@ public class CoyoteAdapter implements Ad /** + * 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 parsePathParameters(org.apache.coyote.Request req, + Request request) { + + // Process in bytes (this is default format so this is normally a NO-OP + req.decodedURI().toBytes(); + + ByteChunk uriBC = req.decodedURI().getByteChunk(); + int semicolon = uriBC.indexOf(';', 0); + String enc = null; + + while (semicolon > -1) { + if (enc == null) { + // What encoding to use? Some platforms, eg z/os, use a default + // encoding that doesn't give the expected result so be explicit + enc = connector.getURIEncoding(); + if (enc == null) { + enc = "ISO-8859-1"; + } + } + + // Parse path param, and extract it from the decoded request URI + int start = uriBC.getStart(); + int end = uriBC.getEnd(); + + int pathParamStart = semicolon + 1; + int pathParamEnd = ByteChunk.findChars(uriBC.getBuffer(), + uriBC.getStart() + pathParamStart, uriBC.getEnd(), + new byte[] {';', '/'}); + + String pv = null; + boolean warnedEncoding = false; + + 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; + } + } + // 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 (pv != null) { + int equals = pv.indexOf('='); + if (equals > -1) { + request.addPathParameter(pv.substring(0, equals), + pv.substring(equals + 1)); + } + } + + semicolon = uriBC.indexOf(';', semicolon); + } + } + + + /** * Look for SSL session ID if required. Only look for SSL Session ID if it * is the only tracking method enabled. */ @@ -672,62 +738,6 @@ public class CoyoteAdapter implements Ad /** * Parse session id in URL. */ - protected void parseSessionId(org.apache.coyote.Request req, Request request) { - - ByteChunk uriBC = req.requestURI().getByteChunk(); - int semicolon = uriBC.indexOf(match, 0, match.length(), 0); - - 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"; - } - - // Parse session ID, 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]; - } - 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 - request.setRequestedSessionId(null); - request.setRequestedSessionURL(false); - log.warn(sm.getString("coyoteAdapter.parseSession", enc), uee); - } - } else { - request.setRequestedSessionId(null); - request.setRequestedSessionURL(false); - } - - } - - - /** - * Parse session id in URL. - */ protected void parseSessionCookiesId(org.apache.coyote.Request req, Request request) { // If session tracking via cookies has been disabled for the current Modified: tomcat/trunk/java/org/apache/catalina/connector/LocalStrings.properties URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/connector/LocalStrings.properties?rev=944920&r1=944919&r2=944920&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/catalina/connector/LocalStrings.properties (original) +++ tomcat/trunk/java/org/apache/catalina/connector/LocalStrings.properties Sun May 16 21:47:26 2010 @@ -33,7 +33,7 @@ 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. # # CoyoteResponse Modified: tomcat/trunk/java/org/apache/catalina/connector/Request.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/connector/Request.java?rev=944920&r1=944919&r2=944920&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/catalina/connector/Request.java (original) +++ tomcat/trunk/java/org/apache/catalina/connector/Request.java Sun May 16 21:47:26 2010 @@ -418,10 +418,21 @@ public class Request protected Boolean asyncSupported = 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); + } public void setAsyncSupported(boolean asyncSupported) { this.asyncSupported = Boolean.valueOf(asyncSupported); @@ -506,6 +517,7 @@ public class Request if (asyncContext!=null) asyncContext.recycle(); asyncContext = null; + pathParameters.clear(); } protected boolean isProcessing() { Modified: tomcat/trunk/java/org/apache/catalina/connector/Response.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/connector/Response.java?rev=944920&r1=944919&r2=944920&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/catalina/connector/Response.java (original) +++ tomcat/trunk/java/org/apache/catalina/connector/Response.java Sun May 16 21:47:26 2010 @@ -45,6 +45,7 @@ import org.apache.catalina.Context; import org.apache.catalina.Globals; import org.apache.catalina.Session; import org.apache.catalina.Wrapper; +import org.apache.catalina.core.ApplicationSessionCookieConfig; import org.apache.catalina.security.SecurityUtil; import org.apache.catalina.util.CharsetMapper; import org.apache.catalina.util.DateTool; @@ -1517,7 +1518,10 @@ public class Response String file = url.getFile(); if ((file == null) || !file.startsWith(contextPath)) return (false); - String tok = ";" + Globals.SESSION_PARAMETER_NAME + "=" + session.getIdInternal(); + String tok = ";" + + ApplicationSessionCookieConfig.getSessionUriParamName( + request.getContext()) + + "=" + session.getIdInternal(); if( file.indexOf(tok, contextPath.length()) >= 0 ) return (false); } @@ -1653,7 +1657,8 @@ public class Response StringBuilder sb = new StringBuilder(path); if( sb.length() > 0 ) { // jsessionid can't be first. sb.append(";"); - sb.append(Globals.SESSION_PARAMETER_NAME); + sb.append(ApplicationSessionCookieConfig.getSessionUriParamName( + request.getContext())); sb.append("="); sb.append(sessionId); } Modified: tomcat/trunk/java/org/apache/catalina/core/ApplicationSessionCookieConfig.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/core/ApplicationSessionCookieConfig.java?rev=944920&r1=944919&r2=944920&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/catalina/core/ApplicationSessionCookieConfig.java (original) +++ tomcat/trunk/java/org/apache/catalina/core/ApplicationSessionCookieConfig.java Sun May 16 21:47:26 2010 @@ -21,10 +21,12 @@ import javax.servlet.SessionCookieConfig import javax.servlet.http.Cookie; import org.apache.catalina.Context; -import org.apache.catalina.Globals; public class ApplicationSessionCookieConfig implements SessionCookieConfig { + private static final String DEFAULT_SESSION_COOKIE_NAME = "JSESSIONID"; + private static final String DEFAULT_SESSION_PARAMETER_NAME = "jsessionid"; + private boolean httpOnly; private boolean secure; private int maxAge = -1; @@ -160,12 +162,7 @@ public class ApplicationSessionCookieCon } - /** - * Determine the name to use for the session cookie for the provided - * context. - * @param context - */ - public static String getSessionCookieName(Context context) { + private static String getConfiguredSessionCookieName(Context context) { // Priority is: // 1. Cookie name defined in context @@ -184,7 +181,40 @@ public class ApplicationSessionCookieCon return cookieName; } } + + return null; + } + + + /** + * Determine the name to use for the session cookie for the provided + * context. + * @param context + */ + public static String getSessionCookieName(Context context) { + + String result = getConfiguredSessionCookieName(context); + + if (result == null) { + result = DEFAULT_SESSION_COOKIE_NAME; + } + + return result; + } + + /** + * Determine the name to use for the session cookie for the provided + * context. + * @param context + */ + public static String getSessionUriParamName(Context context) { + + String result = getConfiguredSessionCookieName(context); + + if (result == null) { + result = DEFAULT_SESSION_PARAMETER_NAME; + } - return Globals.SESSION_COOKIE_NAME; + return result; } } Modified: tomcat/trunk/java/org/apache/catalina/realm/RealmBase.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/realm/RealmBase.java?rev=944920&r1=944919&r2=944920&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/catalina/realm/RealmBase.java (original) +++ tomcat/trunk/java/org/apache/catalina/realm/RealmBase.java Sun May 16 21:47:26 2010 @@ -34,7 +34,6 @@ import javax.servlet.http.HttpServletRes import org.apache.catalina.Container; import org.apache.catalina.Context; import org.apache.catalina.Engine; -import org.apache.catalina.Globals; import org.apache.catalina.Host; import org.apache.catalina.LifecycleException; import org.apache.catalina.LifecycleState; @@ -43,6 +42,7 @@ import org.apache.catalina.Server; import org.apache.catalina.Service; import org.apache.catalina.connector.Request; import org.apache.catalina.connector.Response; +import org.apache.catalina.core.ApplicationSessionCookieConfig; import org.apache.catalina.deploy.LoginConfig; import org.apache.catalina.deploy.SecurityConstraint; import org.apache.catalina.deploy.SecurityCollection; @@ -367,10 +367,11 @@ public abstract class RealmBase extends + " Server digest:" + serverDigest); } - if (serverDigest.equals(clientDigest)) + if (serverDigest.equals(clientDigest)) { return getPrincipal(username); - else - return null; + } + + return null; } @@ -754,11 +755,11 @@ public abstract class RealmBase extends status = false; // No listed roles means no access at all denyfromall = true; break; - } else { - if(log.isDebugEnabled()) - log.debug("Passing all access"); - status = true; } + + if(log.isDebugEnabled()) + log.debug("Passing all access"); + status = true; } else if (principal == null) { if (log.isDebugEnabled()) log.debug(" No user authenticated, cannot grant access"); @@ -923,7 +924,8 @@ public abstract class RealmBase extends if ((requestedSessionId != null) && request.isRequestedSessionIdFromURL()) { file.append(";"); - file.append(Globals.SESSION_PARAMETER_NAME); + file.append(ApplicationSessionCookieConfig.getSessionUriParamName( + request.getContext())); file.append("="); file.append(requestedSessionId); } Added: tomcat/trunk/test/org/apache/catalina/connector/TestCoyoteAdaptor.java URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/connector/TestCoyoteAdaptor.java?rev=944920&view=auto ============================================================================== --- tomcat/trunk/test/org/apache/catalina/connector/TestCoyoteAdaptor.java (added) +++ tomcat/trunk/test/org/apache/catalina/connector/TestCoyoteAdaptor.java Sun May 16 21:47:26 2010 @@ -0,0 +1,76 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.catalina.connector; + +import java.io.IOException; +import java.io.PrintWriter; + +import javax.servlet.ServletException; +import javax.servlet.http.HttpServlet; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +import org.apache.catalina.Context; +import org.apache.catalina.startup.Tomcat; +import org.apache.catalina.startup.TomcatBaseTest; +import org.apache.tomcat.util.buf.ByteChunk; + +public class TestCoyoteAdaptor extends TomcatBaseTest { + + public void testPathParams() throws Exception { + // Setup Tomcat instance + Tomcat tomcat = getTomcatInstance(); + + // Must have a real docBase - just use temp + Context ctx = + tomcat.addContext("/", System.getProperty("java.io.tmpdir")); + + Tomcat.addServlet(ctx, "servlet", new PathParamServlet()); + ctx.addServletMapping("/", "servlet"); + + tomcat.start(); + + testPath("/", "none"); + testPath("/;jsessionid=1234", "1234"); + testPath("/foo;jsessionid=1234", "1234"); + testPath("/foo;jsessionid=1234;dummy", "1234"); + testPath("/foo;jsessionid=1234;dummy=5678", "1234"); + testPath("/foo;jsessionid=1234;=5678", "1234"); + testPath("/foo;jsessionid=1234/bar", "1234"); + } + + private void testPath(String path, String expected) throws Exception { + ByteChunk res = getUrl("http://localhost:" + getPort() + path); + assertEquals(expected, res.toString()); + } + + private static class PathParamServlet extends HttpServlet { + private static final long serialVersionUID = 1L; + + @Override + protected void doGet(HttpServletRequest req, HttpServletResponse resp) + throws ServletException, IOException { + resp.setContentType("text/plain"); + PrintWriter pw = resp.getWriter(); + String sessionId = req.getRequestedSessionId(); + if (sessionId == null) { + sessionId = "none"; + } + pw.write(sessionId); + } + } +} Propchange: tomcat/trunk/test/org/apache/catalina/connector/TestCoyoteAdaptor.java ------------------------------------------------------------------------------ svn:eol-style = native Modified: tomcat/trunk/webapps/docs/config/systemprops.xml URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/config/systemprops.xml?rev=944920&r1=944919&r2=944920&view=diff ============================================================================== --- tomcat/trunk/webapps/docs/config/systemprops.xml (original) +++ tomcat/trunk/webapps/docs/config/systemprops.xml Sun May 16 21:47:26 2010 @@ -347,13 +347,6 @@ <properties> - <property name="org.apache.catalina.SESSION_PARAMETER_NAME"> - <p>An alternative name for the session path parameter. Defaults to - <code>jsessionid</code>. Note that the Servlet specification requires - this to be <code>jsessionid</code>. You should not rely on being able to - change this.</p> - </property> - <property name="org.apache.catalina.SSO_SESSION_COOKIE_NAME"> <p>An alternative name for the single sign on session cookie. Defaults to <code>JSESSIONIDSSO</code>.</p> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org