Author: markt Date: Fri Sep 26 10:58:45 2014 New Revision: 1627746 URL: http://svn.apache.org/r1627746 Log: Merge TestCookies[Allow|Disallow]Equals now that it is no longer necessary to use a system property to change this.
Move allowNameOnly cookies to a CookieProcessor property as this needs to be set to allow the new equals test to work. Fix an edge case and allow a cookie with a value that starts with = to be treated as a name only cookie if = is not allowed in values but name only cookies are permitted. Removed: tomcat/trunk/test/org/apache/tomcat/util/http/TestCookiesDisallowEquals.java Modified: tomcat/trunk/java/org/apache/tomcat/util/http/CookieSupport.java tomcat/trunk/java/org/apache/tomcat/util/http/LegacyCookieProcessor.java tomcat/trunk/test/org/apache/tomcat/util/http/TestCookiesAllowEquals.java tomcat/trunk/webapps/docs/config/cookie-processor.xml Modified: tomcat/trunk/java/org/apache/tomcat/util/http/CookieSupport.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/http/CookieSupport.java?rev=1627746&r1=1627745&r2=1627746&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/tomcat/util/http/CookieSupport.java (original) +++ tomcat/trunk/java/org/apache/tomcat/util/http/CookieSupport.java Fri Sep 26 10:58:45 2014 @@ -55,7 +55,10 @@ public final class CookieSupport { /** * If true, name only cookies will be permitted. + * + * @deprecated Will be removed in Tomcat 9. */ + @Deprecated public static final boolean ALLOW_NAME_ONLY; /** Modified: tomcat/trunk/java/org/apache/tomcat/util/http/LegacyCookieProcessor.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/http/LegacyCookieProcessor.java?rev=1627746&r1=1627745&r2=1627746&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/tomcat/util/http/LegacyCookieProcessor.java (original) +++ tomcat/trunk/java/org/apache/tomcat/util/http/LegacyCookieProcessor.java Fri Sep 26 10:58:45 2014 @@ -48,6 +48,30 @@ public final class LegacyCookieProcessor @SuppressWarnings("deprecation") // Default to false when deprecated code is removed private boolean allowEqualsInValue = CookieSupport.ALLOW_EQUALS_IN_VALUE; + @SuppressWarnings("deprecation") // Default to false when deprecated code is removed + private boolean allowNameOnly = CookieSupport.ALLOW_NAME_ONLY; + + + + public boolean getAllowEqualsInValue() { + return allowEqualsInValue; + } + + + public void setAllowEqualsInValue(boolean allowEqualsInValue) { + this.allowEqualsInValue = allowEqualsInValue; + } + + + public boolean getAllowNameOnly() { + return allowNameOnly; + } + + + public void setAllowNameOnly(boolean allowNameOnly) { + this.allowNameOnly = allowNameOnly; + } + @Override public Charset getCharset() { @@ -195,7 +219,7 @@ public final class LegacyCookieProcessor !CookieSupport.isV0Separator((char)bytes[pos]) && CookieSupport.ALLOW_HTTP_SEPARATORS_IN_V0 || !CookieSupport.isHttpSeparator((char)bytes[pos]) || - bytes[pos] == '=' && allowEqualsInValue) { + bytes[pos] == '=') { // Token valueStart = pos; // getToken returns the position at the delimiter @@ -203,6 +227,13 @@ public final class LegacyCookieProcessor valueEnd = getTokenEndPosition(bytes, valueStart, end, version, false); // We need pos to advance pos = valueEnd; + // Edge case. If value starts with '=' but this is not + // allowed in a value make sure we treat this as no + // value being present + if (valueStart == valueEnd) { + valueStart = -1; + valueEnd = -1; + } } else { // INVALID COOKIE, advance to next delimiter // The starting character of the cookie value was @@ -318,7 +349,7 @@ public final class LegacyCookieProcessor } } } else { // Normal Cookie - if (valueStart == -1 && !CookieSupport.ALLOW_NAME_ONLY) { + if (valueStart == -1 && !getAllowNameOnly()) { // Skip name only cookies if not supported continue; } @@ -359,7 +390,7 @@ public final class LegacyCookieProcessor CookieSupport.ALLOW_HTTP_SEPARATORS_IN_V0 && bytes[pos] != '=' && !CookieSupport.isV0Separator((char)bytes[pos]) || - !isName && bytes[pos] == '=' && allowEqualsInValue)) { + !isName && bytes[pos] == '=' && getAllowEqualsInValue())) { pos++; } @@ -461,14 +492,4 @@ public final class LegacyCookieProcessor } bc.setEnd(dest); } - - - public boolean getAllowEqualsInValue() { - return allowEqualsInValue; - } - - - public void setAllowEqualsInValue(boolean allowEqualsInValue) { - this.allowEqualsInValue = allowEqualsInValue; - } } Modified: tomcat/trunk/test/org/apache/tomcat/util/http/TestCookiesAllowEquals.java URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/tomcat/util/http/TestCookiesAllowEquals.java?rev=1627746&r1=1627745&r2=1627746&view=diff ============================================================================== --- tomcat/trunk/test/org/apache/tomcat/util/http/TestCookiesAllowEquals.java (original) +++ tomcat/trunk/test/org/apache/tomcat/util/http/TestCookiesAllowEquals.java Fri Sep 26 10:58:45 2014 @@ -40,21 +40,54 @@ public class TestCookiesAllowEquals exte private static final String COOKIE_WITH_EQUALS_3 = "name=equalsend="; @Test - public void testWithEquals() throws Exception { - System.setProperty( - "org.apache.tomcat.util.http.ServerCookie.ALLOW_EQUALS_IN_VALUE", - "true"); + public void testLegacyWithEquals() throws Exception { + TestCookieEqualsClient client = new TestCookieEqualsClient(true, false, false); + client.doRequest(); + } + + @Test + public void testLegacyWithoutEquals() throws Exception { + TestCookieEqualsClient client = new TestCookieEqualsClient(false, false, true); + client.doRequest(); + } - TestCookieEqualsClient client = new TestCookieEqualsClient(); + @Test + public void testRfc6265() throws Exception { + // Always allows equals + TestCookieEqualsClient client = new TestCookieEqualsClient(false, true, false); client.doRequest(); } private class TestCookieEqualsClient extends SimpleHttpClient { + private final boolean allowEquals; + private final boolean useRfc6265; + private final boolean expectTruncated; + + public TestCookieEqualsClient(boolean allowEquals, boolean useRfc6265, + boolean expectTruncated) { + this.allowEquals = allowEquals; + this.useRfc6265 = useRfc6265; + this.expectTruncated = expectTruncated; + } + private void doRequest() throws Exception { Tomcat tomcat = getTomcatInstance(); Context root = tomcat.addContext("", TEMP_DIR); + CookieProcessor cookieProcessor; + if (useRfc6265) { + cookieProcessor = new Rfc6265CookieProcessor(); + } else { + LegacyCookieProcessor legacyCookieProcessor = new LegacyCookieProcessor(); + legacyCookieProcessor.setAllowEqualsInValue(allowEquals); + // Need to allow name only cookies to handle equals at the start of + // the value + legacyCookieProcessor.setAllowNameOnly(true); + cookieProcessor = legacyCookieProcessor; + } + root.setCookieProcessor(cookieProcessor); + Tomcat.addServlet(root, "Simple", new SimpleServlet()); root.addServletMapping("/test", "Simple"); @@ -77,10 +110,24 @@ public class TestCookiesAllowEquals exte disconnect(); reset(); tomcat.stop(); - assertEquals(COOKIE_WITH_EQUALS_1 + COOKIE_WITH_EQUALS_2 + - COOKIE_WITH_EQUALS_3, response); + + StringBuilder expected = new StringBuilder(); + expected.append(truncate(COOKIE_WITH_EQUALS_1, expectTruncated)); + expected.append(truncate(COOKIE_WITH_EQUALS_2, expectTruncated)); + expected.append(truncate(COOKIE_WITH_EQUALS_3, expectTruncated)); + assertEquals(expected.toString(), response); } + private final String truncate(String input, boolean doIt) { + if (doIt) { + int end = input.indexOf('=', input.indexOf('=') + 1); + return input.substring(0, end); + } else { + return input; + } + } + + @Override public boolean isResponseBodyOK() { return true; Modified: tomcat/trunk/webapps/docs/config/cookie-processor.xml URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/config/cookie-processor.xml?rev=1627746&r1=1627745&r2=1627746&view=diff ============================================================================== --- tomcat/trunk/webapps/docs/config/cookie-processor.xml (original) +++ tomcat/trunk/webapps/docs/config/cookie-processor.xml Fri Sep 26 10:58:45 2014 @@ -96,7 +96,17 @@ the <code>org.apache.tomcat.util.http.ServerCookie.ALLOW_EQUALS_IN_VALUE</code> <a href="systemprops.html">system property</a>.</p> - + </attribute> + + <attribute name="allowNameOnly" required="false"> + <p>If this is <code>true</code> Tomcat will allow name only cookies + (with or without trailing '<code>=</code>') when parsing cookie headers. + If <code>false</code>, name only cookies will be dropped.</p> + <p>If not set the specification compliant default value of + <code>false</code> will be used. This default may be changed by setting + the + <code>org.apache.tomcat.util.http.ServerCookie.ALLOW_NAME_ONLY</code> + <a href="systemprops.html">system property</a>.</p> </attribute> </attributes> @@ -120,6 +130,7 @@ <ul> <li>The '<code>=</code>' is always permitted in a cookie value.</li> + <li>Name only cookies are always permitted.</li> </ul> <p>No additional attributes are supported by the <strong>RFC 6265 Cookie --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org