On 08/11/2015 20:05, ma...@apache.org wrote: > Author: markt > Date: Sun Nov 8 20:05:27 2015 > New Revision: 1713285 > > URL: http://svn.apache.org/viewvc?rev=1713285&view=rev > Log: > Add the ability to validate client provided session IDs and implement basic > validation for the Standard session ID generator.
I've been thinking about this and I'm not convinced this is a good idea. There are multiple issues: - jvmRoute isn't validated - session IDs are only validated using the current context's settings Given that the whole point of re-using a client provided session is to have a common ID across multiple webapps, validating a session ID with the current context's rules doesn't make a whole lot of sense. Remy pointed me to a different solution he deployed in JBoss off-list. It looks much more promising. I intend to revert this and use Remy's approach. Mark > > Added: > > tomcat/trunk/test/org/apache/catalina/util/TestStandardSessionIdGenerator.java > (with props) > Modified: > tomcat/trunk/java/org/apache/catalina/SessionIdGenerator.java > tomcat/trunk/java/org/apache/catalina/session/ManagerBase.java > tomcat/trunk/java/org/apache/catalina/util/SessionIdGeneratorBase.java > tomcat/trunk/java/org/apache/catalina/util/StandardSessionIdGenerator.java > > Modified: tomcat/trunk/java/org/apache/catalina/SessionIdGenerator.java > URL: > http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/SessionIdGenerator.java?rev=1713285&r1=1713284&r2=1713285&view=diff > ============================================================================== > --- tomcat/trunk/java/org/apache/catalina/SessionIdGenerator.java (original) > +++ tomcat/trunk/java/org/apache/catalina/SessionIdGenerator.java Sun Nov 8 > 20:05:27 2015 > @@ -56,4 +56,17 @@ public interface SessionIdGenerator { > */ > public String generateSessionId(String route); > > + /** > + * Determine, based on implementation specific rules which may be as > strict > + * or as relaxed as the implementor wishes, if the provided session ID is > + * valid. This may be used when generating sessions with user provided > + * session IDs to ensure that they are suitable or if a new ID needs to > be > + * generated. > + * > + * @param sessionId The proposed session ID to test > + * > + * @return {@code true} if the proposed session ID is acceptable, > otherwise > + * {@code false} > + */ > + public boolean validateSessionId(String sessionId); > } > > Modified: tomcat/trunk/java/org/apache/catalina/session/ManagerBase.java > URL: > http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/session/ManagerBase.java?rev=1713285&r1=1713284&r2=1713285&view=diff > ============================================================================== > --- tomcat/trunk/java/org/apache/catalina/session/ManagerBase.java (original) > +++ tomcat/trunk/java/org/apache/catalina/session/ManagerBase.java Sun Nov 8 > 20:05:27 2015 > @@ -627,7 +627,7 @@ public abstract class ManagerBase extend > session.setCreationTime(System.currentTimeMillis()); > session.setMaxInactiveInterval(this.maxInactiveInterval); > String id = sessionId; > - if (id == null) { > + if (id == null || !sessionIdGenerator.validateSessionId(id)) { > id = generateSessionId(); > } > session.setId(id); > > Modified: > tomcat/trunk/java/org/apache/catalina/util/SessionIdGeneratorBase.java > URL: > http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/util/SessionIdGeneratorBase.java?rev=1713285&r1=1713284&r2=1713285&view=diff > ============================================================================== > --- tomcat/trunk/java/org/apache/catalina/util/SessionIdGeneratorBase.java > (original) > +++ tomcat/trunk/java/org/apache/catalina/util/SessionIdGeneratorBase.java > Sun Nov 8 20:05:27 2015 > @@ -273,6 +273,18 @@ public abstract class SessionIdGenerator > } > > > + /** > + * {@inheritDoc} > + * <p> > + * The base implementation performs no validation and treats all proposed > + * session IDs as valid. > + */ > + @Override > + public boolean validateSessionId(String sessionId) { > + return true; > + } > + > + > @Override > protected void initInternal() throws LifecycleException { > // NO-OP > > Modified: > tomcat/trunk/java/org/apache/catalina/util/StandardSessionIdGenerator.java > URL: > http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/util/StandardSessionIdGenerator.java?rev=1713285&r1=1713284&r2=1713285&view=diff > ============================================================================== > --- > tomcat/trunk/java/org/apache/catalina/util/StandardSessionIdGenerator.java > (original) > +++ > tomcat/trunk/java/org/apache/catalina/util/StandardSessionIdGenerator.java > Sun Nov 8 20:05:27 2015 > @@ -16,6 +16,8 @@ > */ > package org.apache.catalina.util; > > +import org.apache.tomcat.util.buf.HexUtils; > + > public class StandardSessionIdGenerator extends SessionIdGeneratorBase { > > @Override > @@ -61,4 +63,39 @@ public class StandardSessionIdGenerator > return buffer.toString(); > } > > + /** > + * {@inheritDoc} > + * <p> > + * This implementation performs the following checks: > + * <ul> > + * <li>The characters up to the first period (if any) are valid hex > + * digits</li> > + * <li>There are at least enough hex digits to represent the specified > + * session ID length</li> > + * <li>Anything after the first period is not validated since that is > + * assumed to be a JVM route and we can't easily determine valid > + * values</li> > + * </ul> > + */ > + @Override > + public boolean validateSessionId(String sessionId) { > + if (sessionId == null) { > + return false; > + } > + int len = sessionId.indexOf('.'); > + if (len == -1) { > + len = sessionId.length(); > + } > + // Session ID length is in bytes and 2 hex digits are required for > each > + // byte > + if (len < getSessionIdLength() * 2) { > + return false; > + } > + for (int i = 0; i < len; i++) { > + if (HexUtils.getDec(sessionId.charAt(i)) == -1) { > + return false; > + } > + } > + return true; > + } > } > > Added: > tomcat/trunk/test/org/apache/catalina/util/TestStandardSessionIdGenerator.java > URL: > http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/util/TestStandardSessionIdGenerator.java?rev=1713285&view=auto > ============================================================================== > --- > tomcat/trunk/test/org/apache/catalina/util/TestStandardSessionIdGenerator.java > (added) > +++ > tomcat/trunk/test/org/apache/catalina/util/TestStandardSessionIdGenerator.java > Sun Nov 8 20:05:27 2015 > @@ -0,0 +1,78 @@ > +/* > + * 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.util; > + > +import org.junit.Assert; > +import org.junit.Test; > + > +public class TestStandardSessionIdGenerator { > + > + // 100 character long valid session ID. This long to accomodate any > future > + // changes in defaut session ID length > + private static final String VALID = > "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA" > + + "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"; > + > + private StandardSessionIdGenerator generator = new > StandardSessionIdGenerator(); > + > + @Test > + public void testValidateNull() { > + Assert.assertFalse(generator.validateSessionId(null)); > + } > + > + @Test > + public void testValidateEmpty() { > + Assert.assertFalse(generator.validateSessionId("")); > + } > + > + @Test > + public void testValidateOneChar() { > + Assert.assertFalse(generator.validateSessionId("A")); > + } > + > + @Test > + public void testValidateShort() { > + Assert.assertFalse(generator.validateSessionId( > + VALID.substring(0, (generator.getSessionIdLength() * 2) > -1))); > + } > + > + @Test > + public void testValidateJustRight() { > + Assert.assertTrue(generator.validateSessionId( > + VALID.substring(0, (generator.getSessionIdLength() * 2)))); > + } > + > + @Test > + public void testValidateLong() { > + Assert.assertTrue(generator.validateSessionId(VALID)); > + } > + > + @Test > + public void testValidateInvalid() { > + Assert.assertFalse(generator.validateSessionId(VALID + "g")); > + } > + > + @Test > + public void testValidateWithJvmRoute() { > + Assert.assertTrue(generator.validateSessionId(VALID + ".g")); > + } > + > + @Test > + public void testValidateWithJvmRouteWithPerid() { > + Assert.assertTrue(generator.validateSessionId(VALID + ".g.h.i")); > + } > + > +} > > Propchange: > tomcat/trunk/test/org/apache/catalina/util/TestStandardSessionIdGenerator.java > ------------------------------------------------------------------------------ > svn:eol-style = native > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > For additional commands, e-mail: dev-h...@tomcat.apache.org > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
